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

Use correct location for type tests in promoted constants #102187

Merged
merged 2 commits into from
Oct 12, 2022

Conversation

b-naber
Copy link
Contributor

@b-naber b-naber commented Sep 23, 2022

Previously we forgot to remap the location in a type test collected when visiting the body of a promoted constant back to the usage location, causing an ICE when trying to get span information for that type test.

Fixes #102117

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 23, 2022
@rust-highfive
Copy link
Collaborator

r? @eholk

(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 Sep 23, 2022
@@ -622,6 +624,13 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
swap_constraints(self);

let locations = location.to_locations();

// Use location of promoted const in collected constraints
for type_test in type_tests.iter() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to do this by iterating over self.cx.borrowck_context.constraints.type_tests directly and modifying in place? Or are the type tests shared with something else and the in place modification would break something else, or something like that?

Copy link
Contributor Author

@b-naber b-naber Sep 26, 2022

Choose a reason for hiding this comment

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

Or are the type tests shared with something else and the in place modification would break something else, or something like that?

Yes the type tests are shared with those of the body that contains the promoted constant, so we need some way to only collect the type tests of the promoted constant body when we type check that. swap_constraints is currently used for this. I'm not sure what the problem is with spawning an entirely new borrow checking context when type checking promoteds. Presumably this would cause slight performance problems when we only really care about the constraints being different? Not sure.

@eholk
Copy link
Contributor

eholk commented Oct 11, 2022

Yikes! I just realized I forgot to come back to this after you replied. Thanks for the explanation. This looks good to me.

@bors r+

@bors
Copy link
Contributor

bors commented Oct 11, 2022

📌 Commit 29cc36f has been approved by eholk

It is now in the queue for this repository.

@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 Oct 11, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 12, 2022
Rollup of 8 pull requests

Successful merges:

 - rust-lang#102110 (Migrate rustc_passes diagnostics)
 - rust-lang#102187 (Use correct location for type tests in promoted constants)
 - rust-lang#102239 (Move style guide to rust-lang/rust)
 - rust-lang#102578 (Panic for invalid arguments of `{integer primitive}::ilog{,2,10}` in all modes)
 - rust-lang#102811 (Use memset to initialize readbuf)
 - rust-lang#102890 (Check representability in adt_sized_constraint)
 - rust-lang#102913 (unify `IsPattern` and `IsImport` enum in `show_candidates`)
 - rust-lang#102924 (rustdoc: remove unused classes from sidebar links)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5e04567 into rust-lang:master Oct 12, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 12, 2022
@b-naber b-naber deleted the inline-const-source-info branch October 12, 2022 12:00
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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ICE with inline_const and TypeId
5 participants