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.

Sign up for GitHub

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

Jump to bottom

Cleanup CTFE error reporting code & centralize promoted error reporting logic #75461

Closed
2 tasks
RalfJung opened this issue Aug 12, 2020 · 1 comment · Fixed by #104317
Closed
2 tasks

Cleanup CTFE error reporting code & centralize promoted error reporting logic #75461

RalfJung opened this issue Aug 12, 2020 · 1 comment · Fixed by #104317
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) C-cleanup Category: PRs that clean code up or issues documenting cleanup.

Comments

@RalfJung
Copy link
Member

CTFE error reporting code is quite complex, and some of the logic is repeated in many places -- e.g. the fact that we do not report hard errors for evaluation failures in promoteds. We should try to centralize that logic (and with rust-lang/const-eval#53 we can hopefully get rid of some of it eventually).

Cases I know of:

  • This here might or might not still be needed but at least the "promoteds only get lints" logic" can hopefully be deduplicated and all the error-reporting logic also moved to librustc_mir::const_eval.
  • Codegen also does CTFE error reporting (with yet another copy of the "promoted" logic) that should be handled through librustc_mir::const_eval::error.

Cc @rust-lang/wg-const-eval
@RalfJung RalfJung added C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-const-eval Area: Constant evaluation (MIR interpretation) labels Aug 12, 2020
This was referenced Aug 12, 2020
Open
Closed
@RalfJung
Copy link
Member Author

With #80579 having landed, promoteds in functions cannot fail to evaluate, so a lot of this can be simplified by quite a bit. :)

@RalfJung RalfJung mentioned this issue Nov 12, 2022
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 16, 2022
@matthiaskrgr
Rollup merge of rust-lang#104317 - RalfJung:ctfe-error-reporting, r=o…
2f93cf5 
…li-obk

cleanup and dedupe CTFE and Miri error reporting

It looks like most of the time, this error raised from const_prop_lint is just redundant -- it duplicates the error reported when evaluating the const-eval query. This lets us make `ConstEvalErr` private to the const_eval module which I think is a good step.

The Miri change mostly replaces a `match` by `if let`, and dedupes the "this error is impossible in Miri" checks.

r? `@oli-obk`
Fixes rust-lang#75461
@bors bors closed this as completed in 353b915 Nov 16, 2022
RalfJung pushed a commit to RalfJung/miri that referenced this issue Nov 17, 2022
@matthiaskrgr
Rollup merge of #104317 - RalfJung:ctfe-error-reporting, r=oli-obk
353a75d 
cleanup and dedupe CTFE and Miri error reporting

It looks like most of the time, this error raised from const_prop_lint is just redundant -- it duplicates the error reported when evaluating the const-eval query. This lets us make `ConstEvalErr` private to the const_eval module which I think is a good step.

The Miri change mostly replaces a `match` by `if let`, and dedupes the "this error is impossible in Miri" checks.

r? ``@oli-obk``
Fixes rust-lang/rust#75461
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) C-cleanup Category: PRs that clean code up or issues documenting cleanup.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

cleanup and dedupe CTFE and Miri error reporting RalfJung/rust
2 participants
@RalfJung and others