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

[tracking issue] warnings on long running constant evaluations #49980

Closed
oli-obk opened this issue Apr 15, 2018 · 5 comments
Closed

[tracking issue] warnings on long running constant evaluations #49980

oli-obk opened this issue Apr 15, 2018 · 5 comments
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-diagnostics Area: Messages for errors, warnings, and lints C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Apr 15, 2018

#49947 introduces a warning (removing the previous hard error).

This warning is currently unsilenceable and should probably become a lint. For simplicity (#49947 needs to be backported to beta) this has not been done.

"When" the warning appears is deterministic, but depends on MIR, so it might change between versions of rustc.

@oli-obk oli-obk added A-diagnostics Area: Messages for errors, warnings, and lints A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) labels Apr 15, 2018
@rcoh
Copy link
Contributor

rcoh commented Apr 17, 2018

Seems like this will also require a small amount of refactoring? I don't think the evaluator where the warning is currently emitted knows the NodeId that is needed when emitting the lint.

@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 18, 2018

We should be reporting the lint on the constant that is being evaluated, not the place that we are currently at, because that place might change between rustc versions

@oli-obk
Copy link
Contributor Author

oli-obk commented May 18, 2018

Note that just turning this into a lint is probably not enough. We need to also check whether the lint is deny in the constant's scope. If it is, we need to abort the interpretation. Otherwise we'd just keep getting these lints as errors without ever stopping compilation.

ecstatic-morse added a commit to ecstatic-morse/rust that referenced this issue Jul 4, 2018
This test relies on the fact that restrictions on expressions in `const
fn` do not apply when computing array lengths. It is more difficult to
statically analyze than the simple `loop{}` mentioned in rust-lang#50637.

This test should be updated to ignore the warning after rust-lang#49980 is resolved.
@XAMPPRocky XAMPPRocky added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Aug 27, 2018
Centril added a commit to Centril/rust that referenced this issue Mar 23, 2020
…op-detector, r=RalfJung

Remove const eval loop detector

Now that there is a configurable instruction limit for CTFE (see rust-lang#67260), we can replace the loop detector with something much simpler. See rust-lang#66946 for more discussion about this. Although the instruction limit is nightly-only, the only practical way to reach the default limit uses nightly-only features as well (although CTFE will still execute code using such features inside an array initializer on stable).

This will at the very least require a crater run, since it will result in an error wherever the "long running const eval" warning appeared before. We may need to increase the default for `const_eval_limit` to work around this.

Resolves rust-lang#54384 cc rust-lang#49980
r? @oli-obk cc @RalfJung
Centril added a commit to Centril/rust that referenced this issue Mar 23, 2020
…op-detector, r=RalfJung

Remove const eval loop detector

Now that there is a configurable instruction limit for CTFE (see rust-lang#67260), we can replace the loop detector with something much simpler. See rust-lang#66946 for more discussion about this. Although the instruction limit is nightly-only, the only practical way to reach the default limit uses nightly-only features as well (although CTFE will still execute code using such features inside an array initializer on stable).

This will at the very least require a crater run, since it will result in an error wherever the "long running const eval" warning appeared before. We may need to increase the default for `const_eval_limit` to work around this.

Resolves rust-lang#54384 cc rust-lang#49980
r? @oli-obk cc @RalfJung
@steveklabnik
Copy link
Member

Triage: I'm not aware of any changes here. I'm also not sure what to do to reproduce this issue, to be honest.

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 15, 2020

static FOO: () = loop {}; gives (on beta)

error[E0080]: could not evaluate static initializer
 --> src/lib.rs:1:18
  |
1 | static FOO: () = loop {};
  |                  ^^^^^^^ exceeded interpreter step limit (see `#[const_eval_limit]`)

So I think we're good here

@oli-obk oli-obk closed this as completed Aug 15, 2020
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, covers all const contexts (static, const fn, ...) A-diagnostics Area: Messages for errors, warnings, and lints C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants