Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

typeck: simplify the handling of `diverges` #68422

Merged
merged 3 commits into from Jan 22, 2020
Merged

Conversation

@Centril
Copy link
Member

Centril commented Jan 21, 2020

Some drive-by cleanup while working on hir::ExprKind::Let.
Ostensibly, this has some perf benefits due to reduced allocation and whatnot as well.

r? @eddyb

let scrut_diverges = self.diverges.get();
self.diverges.set(Diverges::Maybe);
// Otherwise, we have to union together the types that the arms produce and so forth.
let scrut_diverges = self.diverges.replace(Diverges::Maybe);

This comment has been minimized.

Copy link
@eddyb

eddyb Jan 21, 2020

Member

Ohh, we might have more similar places where we use get and set, predating Cell::replace.
I found some in these two files, if someone wants to take them:

  • src/librustc/ty/print/pretty.rs
  • src/librustc/infer/mod.rs

(there's others, but they use the result of get() to compute the value to set, and I don't think we have a nice abstraction for that)

// #55810: Type check patterns first so we get types for all bindings.
for arm in arms {
self.check_pat_top(&arm.pat, scrut_ty, Some(scrut.span), true);
}

This comment has been minimized.

Copy link
@eddyb

eddyb Jan 21, 2020

Member

I assume "they're now subsumed by unreachable_patterns warnings" in the deleted comment is why you can do this change now?

This comment has been minimized.

Copy link
@Centril

Centril Jan 21, 2020

Author Member

Filed #68429.

(To recap what I said in private: check/pat.rs does not mention diverges, except indirectly by type checking expressions, which only occur in literals and range patterns, but those cannot be typed at a diverging type, so removing this code has no effect.)

@@ -200,16 +183,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
arms: &'tcx [hir::Arm<'tcx>],
source: hir::MatchSource,
) {
if self.diverges.get().is_always() {

This comment has been minimized.

Copy link
@eddyb

eddyb Jan 21, 2020

Member

I think this was a performance microopt?

@eddyb
eddyb approved these changes Jan 21, 2020
@eddyb

This comment has been minimized.

Copy link
Member

eddyb commented Jan 21, 2020

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 21, 2020

📌 Commit 7dceff9 has been approved by eddyb

Centril added a commit to Centril/rust that referenced this pull request Jan 21, 2020
typeck: simplify the handling of `diverges`

Some drive-by cleanup while working on `hir::ExprKind::Let`.
Ostensibly, this has some perf benefits due to reduced allocation and whatnot as well.

r? @eddyb
bors added a commit that referenced this pull request Jan 21, 2020
Rollup of 4 pull requests

Successful merges:

 - #68253 (add bare metal ARM Cortex-A targets to rustc)
 - #68361 (Unbreak linking with lld 9 on FreeBSD 13.0-CURRENT i386)
 - #68421 (Update cargo, books)
 - #68422 (typeck: simplify the handling of `diverges`)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Jan 22, 2020
typeck: simplify the handling of `diverges`

Some drive-by cleanup while working on `hir::ExprKind::Let`.
Ostensibly, this has some perf benefits due to reduced allocation and whatnot as well.

r? @eddyb
bors added a commit that referenced this pull request Jan 22, 2020
Rollup of 3 pull requests

Successful merges:

 - #68421 (Update cargo, books)
 - #68422 (typeck: simplify the handling of `diverges`)
 - #68439 (Update Clippy)

Failed merges:

r? @ghost
@bors bors merged commit 7dceff9 into rust-lang:master Jan 22, 2020
4 checks passed
4 checks passed
pr Build #20200121.51 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-7) Linux x86_64-gnu-llvm-7 succeeded
Details
pr (Linux x86_64-gnu-tools) Linux x86_64-gnu-tools succeeded
Details
@Centril Centril deleted the Centril:diverges-simplify branch Jan 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.