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_mir: fix miri substitution/"universe" discipline. #63497

Open
wants to merge 12 commits into
base: master
from

Conversation

@eddyb
Copy link
Member

commented Aug 12, 2019

Based on #63495, relative diff

Alternative to #61041, based on @RalfJung's own attempt at it.
I haven't done a full audit, but I believe everything is fixed now.

Closes #61336, as a drive-by fix (for a subset of #43408, that is already special-cased).

r? @oli-obk / @RalfJung cc @varkor @yodaldevoid

@eddyb

This comment has been minimized.

Copy link
Member Author

commented Aug 12, 2019

I expect some tests to fail due to #61336 (comment). EDIT: fixed now.

Some(frame) => self.tcx.normalize_erasing_regions(
self.param_env,
value.subst(*self.tcx, frame.instance.substs),
),

This comment has been minimized.

Copy link
@RalfJung

RalfJung Aug 12, 2019

Member

This could use tcx.subst_and_normalize_erasing_regions, right?

This comment has been minimized.

Copy link
@eddyb

eddyb Aug 12, 2019

Author Member

I intentionally inlined that function to avoid its assert.

This comment has been minimized.

Copy link
@RalfJung

This comment has been minimized.

Copy link
@eddyb

eddyb Aug 12, 2019

Author Member

I must've gotten confused, looks like it's normalize_erasing_late_bound_regions that's the problematic one.

@@ -541,7 +544,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Early-return cases.
match val.val {

This comment has been minimized.

Copy link
@RalfJung

RalfJung Aug 12, 2019

Member

Could you add a doc comment to this function that say something similar to resolve, namely that val (and layout) is expected to be already in the InterpCx "universe"?

@RalfJung

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

I can confirm that this fixes the test-case in rust-lang/miri#837 :)

@bors

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2019

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

@RalfJung

This comment has been minimized.

Copy link
Member

commented Aug 17, 2019

@eddyb the base PR landed, so after a rebase this should be ready for review. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.