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

const-eval: perform validation before interning #122398

Closed
RalfJung opened this issue Mar 12, 2024 · 0 comments · Fixed by #122684
Closed

const-eval: perform validation before interning #122398

RalfJung opened this issue Mar 12, 2024 · 0 comments · Fixed by #122684
Assignees
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) WG-const-eval Working group: Const evaluation

Comments

@RalfJung
Copy link
Member

Currently, after const-eval is done, we first intern the result into tcx and then validate that it matches the type. This is unfortunate because interning can go wrong (when there is a mutable pointer somewhere we did not expect), and in most cases that issue would also be detected by validation -- and validation shows a much nicer error, indicating where in the value the issue arises.

However, interning currently also does the job of adjusting the allocation's mutability to its final value. This has to be done before validation. So fixing this would end up with a final order of operations like:

  • Adjust mutability
  • Validation
  • Interning

All that said, I'm not entirely sure whether this is worth it since triggering the interning error should be pretty rare. Our static const checks should largely prevent them.

Cc @rust-lang/wg-const-eval

@RalfJung RalfJung added the A-const-eval Area: Constant evaluation (MIR interpretation) label Mar 12, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 12, 2024
@jieyouxu jieyouxu added WG-const-eval Working group: Const evaluation and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 12, 2024
@oli-obk oli-obk self-assigned this Mar 13, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 18, 2024
…=<try>

Validate before interning

based on rust-lang#122397

fixes rust-lang#122398

r? `@RalfJung`

There are more cleanups that can be done afterwards, but I think they may be unnecessary to make this PR useful on its own
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Apr 17, 2024
…after_validaiton, r=RalfJung

Delay interning errors to after validation

fixes rust-lang#122398
fixes rust-lang#122548

This improves diagnostics since validation errors are usually more helpful compared with interning errors that just make broad statements about the entire constant

r? `@RalfJung`
@bors bors closed this as completed in 5260893 Apr 18, 2024
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Apr 19, 2024
…aiton, r=RalfJung

Delay interning errors to after validation

fixes rust-lang/rust#122398
fixes #122548

This improves diagnostics since validation errors are usually more helpful compared with interning errors that just make broad statements about the entire constant

r? `@RalfJung`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) WG-const-eval Working group: Const evaluation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants