Make mir borrowck's use of opaque types independent of the typeck query's result #87287

oli-obk · 2021-07-19T16:51:58Z

we used to use the typeck results only to generate an obligation for the mir borrowck type to be equal to the typeck result.

When i removed the fixup_opaque_types function in #87200, I exposed a bug that showed that mir borrowck can't doesn't get enough information from typeck in order to build the correct lifetime mapping from opaque type usage to the actual concrete type. We therefor now fully compute the information within mir borrowck (we already did that, but we only used it to verify the typeck result) and stop using the typeck information.

We will likely be able to remove most opaque type information from the borrowck results in the future and just have all current callers use the mir borrowck result instead.

r? @spastorino

bors · 2021-07-20T08:46:58Z

☔ The latest upstream changes (presumably #87141) made this pull request unmergeable. Please resolve the merge conflicts.

b-naber

Not sure whether I'm qualified enough to review this, I might be missing something, but there were two parts I didn't understand.

b-naber · 2021-07-20T11:16:33Z

compiler/rustc_mir/src/borrow_check/type_check/mod.rs

-            cx.opaque_type_values
+            let mut opaque_type_values = cx.opaque_type_values;
+
+            for (_, revealed_ty) in &mut opaque_type_values {


Why does opaque_type_values contain elements at the start of the loop? It seems to me that the only place where elements were inserted into this field was in the code you deleted below?

yes, but the function where I deleted code is called multiple times. The opaque types are collected over time.

compiler/rustc_mir/src/borrow_check/type_check/mod.rs

spastorino · 2021-07-20T15:39:59Z

Looks good to me, bors r=me once tests are 👍

oli-obk · 2021-07-20T16:03:54Z

@bors r=spastorino

bors · 2021-07-20T16:03:55Z

📌 Commit 713044c has been approved by spastorino

spastorino · 2021-07-20T19:16:25Z

@bors retry

spastorino · 2021-07-20T19:30:08Z

@bors retry

oli-obk · 2021-07-20T23:15:46Z

@bors r=spastorino

bors · 2021-07-20T23:15:47Z

💡 This pull request was already approved, no need to approve it again.

There's another pull request that is currently being tested, blocking this pull request: [beta] Remove unsound TrustedRandomAccess implementations #87136

bors · 2021-07-20T23:15:48Z

📌 Commit 713044c has been approved by spastorino

…pes, r=spastorino Make mir borrowck's use of opaque types independent of the typeck query's result fixes rust-lang#87218 fixes rust-lang#86465 we used to use the typeck results only to generate an obligation for the mir borrowck type to be equal to the typeck result. When i removed the `fixup_opaque_types` function in rust-lang#87200, I exposed a bug that showed that mir borrowck can't doesn't get enough information from typeck in order to build the correct lifetime mapping from opaque type usage to the actual concrete type. We therefor now fully compute the information within mir borrowck (we already did that, but we only used it to verify the typeck result) and stop using the typeck information. We will likely be able to remove most opaque type information from the borrowck results in the future and just have all current callers use the mir borrowck result instead. r? `@spastorino`

…pes, r=spastorino Make mir borrowck's use of opaque types independent of the typeck query's result fixes rust-lang#87218 fixes rust-lang#86465 we used to use the typeck results only to generate an obligation for the mir borrowck type to be equal to the typeck result. When i removed the `fixup_opaque_types` function in rust-lang#87200, I exposed a bug that showed that mir borrowck can't doesn't get enough information from typeck in order to build the correct lifetime mapping from opaque type usage to the actual concrete type. We therefor now fully compute the information within mir borrowck (we already did that, but we only used it to verify the typeck result) and stop using the typeck information. We will likely be able to remove most opaque type information from the borrowck results in the future and just have all current callers use the mir borrowck result instead. r? ``@spastorino``

It used to allow you to mutate the key, even though that can invalidate the map by creating duplicate keys.

…ry's result

I attempted that with the previous code, but I misunderstdood how `shallow_resolve` works.

oli-obk · 2021-07-22T11:27:08Z

@bors r=spastorino

bors · 2021-07-22T11:27:09Z

📌 Commit d103852 has been approved by spastorino

bors · 2021-07-23T03:40:30Z

⌛ Testing commit d103852 with merge b2b7c85...

bors · 2021-07-23T06:21:14Z

☀️ Test successful - checks-actions
Approved by: spastorino
Pushing b2b7c85 to master...

…i-obk VecMap::get_value_matching should return just one element r? `@nikomatsakis` Related to rust-lang#86465 and rust-lang#87287

Mir borrowck does not generate lifetime variables for 'static lifetimes during opaque type resolution Fixes rust-lang#87455 This situation was unreachable before rust-lang#87287 as we used to just grab the resolved opaque type from typeck and replaced all regions with new inference vars. After rust-lang#87287 we let the `InferCx` in mir borrowck figure out the opaque type all by itself (which it already did before, but it only used the result to sanity check with the typeck result).

Concrete regions can show up in mir borrowck if the originated from there We used to not encounter them here, because we took regions from typeck's opaque type resolution by renumbering them. We don't do that anymore. Instead mir borrock does all the logic, and it can handle concrete regions just fine, as long as it created them itself. fixes rust-lang#83190 which was introduced by rust-lang#87287 r? `@spastorino`

rust-highfive assigned spastorino Jul 19, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 19, 2021

This comment has been minimized.

Sign in to view

oli-obk force-pushed the fixup_fixup_fixup_opaque_types branch from 2db308c to 75d9ed7 Compare July 20, 2021 10:11

b-naber reviewed Jul 20, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

oli-obk force-pushed the fixup_fixup_fixup_opaque_types branch from 3d70dee to 5156aab Compare July 20, 2021 15:34

spastorino approved these changes Jul 20, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

oli-obk force-pushed the fixup_fixup_fixup_opaque_types branch from 5156aab to 713044c Compare July 20, 2021 16:03

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 Jul 20, 2021

This comment has been minimized.

Sign in to view

spastorino closed this Jul 20, 2021

spastorino reopened this Jul 20, 2021

Dylan-DPC-zz mentioned this pull request Jul 21, 2021

Rollup of 11 pull requests #87333

Closed

Dylan-DPC-zz mentioned this pull request Jul 21, 2021

Rollup of 12 pull requests #87344

Closed

oli-obk added 7 commits July 22, 2021 11:20

Fix VecMap::iter_mut

d693a98

It used to allow you to mutate the key, even though that can invalidate the map by creating duplicate keys.

Make mir borrowck's use of opaque types independent of the typeck que…

6d76002

…ry's result

Use instrument debugging for more readable logs

bdc20e3

Remove an unnecessary variable

a8551ab

Resolve nested inference variables.

9f09a5e

I attempted that with the previous code, but I misunderstdood how `shallow_resolve` works.

Add a regression test

f2d2fd3

bless nll tests

d103852

oli-obk force-pushed the fixup_fixup_fixup_opaque_types branch from 713044c to d103852 Compare July 22, 2021 11:26

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 Jul 22, 2021

spastorino mentioned this pull request Jul 22, 2021

VecMap::get_value_matching should return just one element #86410

Merged

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 23, 2021

bors merged commit b2b7c85 into rust-lang:master Jul 23, 2021

rustbot added this to the 1.55.0 milestone Jul 23, 2021

bors mentioned this pull request Jul 23, 2021

Improve get_by_key_enumerated more #86429

Merged

Alexendoo mentioned this pull request Jul 24, 2021

ICE(broken MIR and no errors despite delay_span_bug) when existential type alias is used as closure return value causing type error #74280

Closed

ehuss mentioned this pull request Jul 25, 2021

ICE: unexpected concrete region in borrowck: ReStatic #87455

Closed

oli-obk mentioned this pull request Jul 26, 2021

Mir borrowck does not generate lifetime variables for 'static lifetimes during opaque type resolution #87483

Merged

oli-obk mentioned this pull request Aug 31, 2021

Concrete regions can show up in mir borrowck if the originated from there #88533

Merged

oli-obk deleted the fixup_fixup_fixup_opaque_types branch November 9, 2022 09:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make mir borrowck's use of opaque types independent of the typeck query's result #87287

Make mir borrowck's use of opaque types independent of the typeck query's result #87287

oli-obk commented Jul 19, 2021 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented Jul 20, 2021

b-naber left a comment

b-naber Jul 20, 2021

oli-obk Jul 20, 2021 •

edited

Loading

This comment has been minimized.

spastorino commented Jul 20, 2021

This comment has been minimized.

oli-obk commented Jul 20, 2021

bors commented Jul 20, 2021

This comment has been minimized.

spastorino commented Jul 20, 2021

spastorino commented Jul 20, 2021

oli-obk commented Jul 20, 2021

bors commented Jul 20, 2021

bors commented Jul 20, 2021

oli-obk commented Jul 22, 2021

bors commented Jul 22, 2021

bors commented Jul 23, 2021

bors commented Jul 23, 2021

Make mir borrowck's use of opaque types independent of the typeck query's result #87287

Make mir borrowck's use of opaque types independent of the typeck query's result #87287

Conversation

oli-obk commented Jul 19, 2021 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented Jul 20, 2021

b-naber left a comment

Choose a reason for hiding this comment

b-naber Jul 20, 2021

Choose a reason for hiding this comment

oli-obk Jul 20, 2021 • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

spastorino commented Jul 20, 2021

This comment has been minimized.

oli-obk commented Jul 20, 2021

bors commented Jul 20, 2021

This comment has been minimized.

spastorino commented Jul 20, 2021

spastorino commented Jul 20, 2021

oli-obk commented Jul 20, 2021

bors commented Jul 20, 2021

bors commented Jul 20, 2021

oli-obk commented Jul 22, 2021

bors commented Jul 22, 2021

bors commented Jul 23, 2021

bors commented Jul 23, 2021

oli-obk commented Jul 19, 2021 •

edited

Loading

oli-obk Jul 20, 2021 •

edited

Loading