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

Allow monomorphization time const eval failures if the cause is a type layout issue #124516

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Apr 29, 2024

r? @RalfJung

fixes #124348

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 29, 2024
@rustbot
Copy link
Collaborator

rustbot commented Apr 29, 2024

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 29, 2024
@oli-obk oli-obk changed the title Allow const eval failures if the cause is a type layout issue Allow monomorphization time const eval failures if the cause is a type layout issue Apr 30, 2024
Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

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

r=me with the comment nit -- but I think this does not entirely solve the problem; I just don't have the time to try and find an example right now

if !reported.is_tainted_by_errors() {
// Looks like the const is not captued by `required_consts`, that's bad.
span_bug!(span, "interpret const eval failure of {val:?} which is not in required_consts");
}
Copy link
Member

Choose a reason for hiding this comment

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

I guess we don't need a delay_span_bug in the else since we in fact have an ErrorGuaranteed handle here?

Suggested change
}
} else {
// const-eval will return "tainted" errors if e.g. the layout cannot
// be computed as the type references non-existing names.
// See <https://github.com/rust-lang/rust/issues/124348>.
}

err_inval!(Layout(LayoutError::ReferencesError(guar))) => {
ErrorHandled::Reported(guar.into(), span.unwrap_or(DUMMY_SP))
}
err_inval!(Layout(LayoutError::ReferencesError(guar))) => ErrorHandled::Reported(
Copy link
Member

Choose a reason for hiding this comment

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

What if there is aSizeOverflow in a promoted?

This PR seems like a good step so I am okay with landing it, but it seems to me there's still a potential for ICEs here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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: interpret const eval failure of Unevaluated(UnevaluatedConst ... which is not in required_consts
3 participants