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

rustc_infer: remove InferCtxt::closure_sig as the FnSig is always shallowly known. #70089

Merged
merged 1 commit into from
Mar 21, 2020

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Mar 18, 2020

That is, ClosureSubsts is always created (in rustc_typeck::check::closure) with a FnSig, as the number of inputs is known, even if they might all have inference types.
The only useful thing InferCtxt::closure_sig was doing is resolving an inference variable used just to get the ty::FnPtr containing that FnSig into ClosureSubsts.

The ideal way to solve this would be to add a constructor for ClosureSubsts, that combines the parent Substs, the closure kind, the signature, and capture types together, but for now I've went with resolving the inference types just after unifying them with the real types.

r? @nikomatsakis

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 18, 2020
@eddyb
Copy link
Member Author

eddyb commented Mar 18, 2020

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Mar 18, 2020

⌛ Trying commit 63811bc with merge 2056ca8947d090a687667e68d89cceab943a3928...

@bors
Copy link
Contributor

bors commented Mar 18, 2020

☀️ Try build successful - checks-azure
Build commit: 2056ca8947d090a687667e68d89cceab943a3928 (2056ca8947d090a687667e68d89cceab943a3928)

@rust-timer
Copy link
Collaborator

Queued 2056ca8947d090a687667e68d89cceab943a3928 with parent 7ceebd9, future comparison URL.

@eddyb
Copy link
Member Author

eddyb commented Mar 18, 2020

Perf looks neutral, probably makes sense outside of explicit closure stress tests.

@@ -155,6 +156,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);
}

// HACK(eddyb) this forces the types equated above into `substs` but
// it should rely on `ClosureSubsts` providing a constructor, instead.
let substs = self.resolve_vars_if_possible(&substs);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the purpose of this code is to ensure that have a fn type, and not an inference variable that's been equated with a fn type?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yupp, pretty much.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Mar 19, 2020

📌 Commit 63811bc has been approved by nikomatsakis

@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 19, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 21, 2020
Rollup of 8 pull requests

Successful merges:

 - rust-lang#67888 (Prefetch some queries used by the metadata encoder)
 - rust-lang#69934 (Update the mir inline costs)
 - rust-lang#69965 (Refactorings to get rid of rustc_codegen_utils)
 - rust-lang#70054 (Build dist-android with --enable-profiler)
 - rust-lang#70089 (rustc_infer: remove InferCtxt::closure_sig as the FnSig is always shallowly known.)
 - rust-lang#70092 (hir: replace "items" terminology with "nodes" where appropriate.)
 - rust-lang#70138 (do not 'return' in 'throw_' macros)
 - rust-lang#70151 (Update stdarch submodule)

Failed merges:

 - rust-lang#70074 (Expand: nix all fatal errors)

r? @ghost
@bors bors merged commit a6d0c35 into rust-lang:master Mar 21, 2020
@eddyb eddyb deleted the closure-sig-infer branch March 21, 2020 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants