Skip to content

Move check only relevant in error case out of critical path #82738

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Mar 7, 2021
Merged

Move check only relevant in error case out of critical path #82738

merged 1 commit into from
Mar 7, 2021

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Mar 4, 2021

Move the check for potentially forgotten return in a tail expression
of arbitrary expressions into the coercion error branch to avoid
computing unncessary coercion checks on successful code.

Follow up to #81458.

@estebank
Move check only relevant in error case out of critical path
c6c243a 
Move the check for potentially forgotten `return` in a tail expression
of arbitrary expressions into the coercion error branch to avoid
computing unncessary coercion checks on successful code.

Follow up to rust-lang#81458.
@rust-highfive
Copy link
Contributor

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 4, 2021
@estebank estebank mentioned this pull request Mar 4, 2021
Merged
@estebank
Copy link
Contributor Author

estebank commented Mar 4, 2021

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 4, 2021
@bors
Copy link
Collaborator

bors commented Mar 4, 2021

⌛ Trying commit c6c243a with merge 75dfc563af3058ba1fd662a6c5c9e256e96338af...

@bors
Copy link
Collaborator

bors commented Mar 4, 2021

☀️ Try build successful - checks-actions
Build commit: 75dfc563af3058ba1fd662a6c5c9e256e96338af (75dfc563af3058ba1fd662a6c5c9e256e96338af)

@rust-timer
Copy link
Collaborator

Queued 75dfc563af3058ba1fd662a6c5c9e256e96338af with parent 1c77a1f, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (75dfc563af3058ba1fd662a6c5c9e256e96338af): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 4, 2021
@oli-obk
Copy link
Contributor

oli-obk commented Mar 4, 2021

@bors r+ rollup=never

I compared the perf results with #81458 and it looks to me like it fully reverts the regression

@bors
Copy link
Collaborator

bors commented Mar 4, 2021

📌 Commit c6c243a has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 4, 2021
@bors
Copy link
Collaborator

bors commented Mar 5, 2021

⌛ Testing commit c6c243a with merge 36aedfbed8fbd3df10ae768cd2fb7b8358420ed4...

@bors
Copy link
Collaborator

bors commented Mar 5, 2021

💥 Test timed out

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 5, 2021
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@JohnTitor
Copy link
Member

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 5, 2021
@bors
Copy link
Collaborator

bors commented Mar 6, 2021

⌛ Testing commit c6c243a with merge dfe519b...

@bors
Copy link
Collaborator

bors commented Mar 7, 2021

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing dfe519b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 7, 2021
@bors bors merged commit dfe519b into rust-lang:master Mar 7, 2021
@rustbot rustbot added this to the 1.52.0 milestone Mar 7, 2021
@bors bors mentioned this pull request Mar 7, 2021
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees

@davidtwco davidtwco

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.52.0
Development

Successfully merging this pull request may close these issues.
9 participants
@estebank @rust-highfive @rust-timer @bors @oli-obk @rust-log-analyzer @JohnTitor @davidtwco @rustbot