fix universe map in ifcx.instantiate_canonical_*

[aliemjay]

Previously, infcx.instantiate_canonical_* maps the root universe in canonical into ty::UniverseIndex::Root, I think because it assumes it works with a fresh infcx but this is not true for the use cases in mir typeck. Now the root universe is mapped into infcx.universe().

I catched this accidentally while reviewing the code. I'm not sure if this is the right fix or if it is really a bug!

[rust-highfive]

r? @petrochenkov

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

[aliemjay]

r? @rust-lang/types

[jackh726]

Any chance there's some test we can add?

[b-naber]

Currently reading about Universes/Placeholders and am curious, why can we do that? Why can't we encounter canonicalized variables with UniverseIndex::Root inside instantiate_canonical_vars?

[jackh726]

I think this looks fine, but I'd like @nikomatsakis's eyes on this

[nikomatsakis]

I'm not sure what a test would look like for this, but I see the logic in the proposed behavior.

[jackh726]

@aliemjay any idea if you can come up with a test for this?

[aliemjay]

@jackh726 I don't think a test is possible because the only place this method is used on a non-fresh InferCtxt is when instantiating UserTypeAnnotation in MIR typeck, but type annotations are currently only used in root universe AFAIK. For example if we're type-checking closures in a different way (using universes), the test would be like:

fn test() { // U0
    for<'a> |s: &'a str| { //U1
        let _: &'_ str = s; // `'_` lives in a higher universe
    };
}

[jackh726]

Could this be done with rust-lang/rfcs#3216?

[apiraino]

hello, checking progress. What's the current status? Based on the previous comment, I'd switch to waiting for a feedback from @aliemjay (hope I am reading the status correctly). Feel free to request a review with @rustbot ready, thanks!

@rustbot author

[jackh726]

Going to go ahead and approve this. The new code looks more right and writing a test for this is hard.

@bors r+ rollup=never

(no rollup so it's easier to bisect if it is indeed wrong somehow)

[bors]

📌 Commit 70200ac has been approved by jackh726

It is now in the queue for this repository.

[bors]

⌛ Testing commit 70200ac with merge 90711a8...

[bors]

☀️ Test successful - checks-actions
Approved by: jackh726
Pushing 90711a8 to master...

[rust-timer]

Finished benchmarking commit (90711a8): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

This benchmark run did not return any relevant results for this metric.