Skip to content

On-demand const verification may be a backwards compatibility hazard #19265

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

Closed
huonw opened this issue Nov 24, 2014 · 5 comments
Closed

On-demand const verification may be a backwards compatibility hazard #19265

huonw opened this issue Nov 24, 2014 · 5 comments
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) C-future-incompatibility Category: Future-incompatibility lints P-low Low priority T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@huonw
Copy link
Member

huonw commented Nov 24, 2014

E.g. #19244 describes how the following is valid, but uncommenting the line causes the error given below.

struct Wrapper(usize);
const BAD_CONST: usize = Wrapper(42).0;

// type X = [u8; BAD_CONST];

fn main() {}
3 | const BAD_CONST: usize = Wrapper(42).0;
  |                          ^^^^^^^^^^^ unimplemented constant expression: tuple struct constructors

(NB. that specific instance may be fixed in future, but that does not necessarily mean this bug is fixed.)

That is, the const-suitability of the value is checked when used as a constant expression, not at the definition. This could possibly may lead to surprises if rules are tweaked (at least, it could lead to surprises that are not detected). In any case, it is certainly confusing for a const to apparently be an unsupported constant expression.

It seems that we could check the definitions, not just checking them lazily when they are used as const expressions.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Nov 27, 2014
With this PR, the following code works:

```
#![feature(tuple_indexing)]
struct MyStruct { field1: uint }

const S: MyStruct = MyStruct { field1: 42u };
const T: (uint,) = (42u,);

struct ConstCheck {
    array1: [int, ..S.field1],
    array2: [int, ..T.0],
}
```

Closes rust-lang#19244
Related rust-lang#19265
@brson brson removed the I-nominated label Dec 11, 2014
@brson
Copy link
Contributor

brson commented Dec 11, 2014

P-low, not 1.0

@nikomatsakis
Copy link
Contributor

I am not sure how bad this is. There are constants that we cannot evaluate at type-checking time but only at trans time. Eventually I think we can (and should) make that distinction very small or even non-existant, but I don't think it means that those constants can't exist in the first place (which seems to be what @huonw is suggesting).

@steveklabnik steveklabnik added I-slow Issue: Problems and improvements with respect to performance of generated code. P-low Low priority and removed I-slow Issue: Problems and improvements with respect to performance of generated code. labels Jan 29, 2015
@oli-obk
Copy link
Contributor

oli-obk commented Nov 30, 2015

Related issue: #3170 (comment)

const ARR: [u32; 0] = []; const X: u32 = ARR[5];

The above doesn't error until you do let x = X; somewhere.

bors added a commit that referenced this issue Apr 4, 2016
check constants even if they are not used in the current crate

For now this is just a `warn`-by-default lint. I suggest to make it a `deny`-by-default lint in the next release cycle (so no dependencies break), and then in another release cycle move to an error.

cc #19265
cc #3170
@eddyb
Copy link
Member

eddyb commented Apr 15, 2017

I've updated the issue description so that it matches what's happening today.
However, the only reason that example still behaves the way it does is this whitelist - which is somewhat necessary as that expression is quite valid and will work when put through the trans evaluator (or miri).

@oli-obk's example is more interesting and I'm not sure how that one is getting whitelisted.
It's feature-gated (const_indexing) and that's whitelisted as IndexOpFeatureGated, resulting in:

const X: usize = [][5];
// Uncomment to get an indexing feature-gating constant evaluation error.
// type Foo = [u8; X];

@Mark-Simulacrum Mark-Simulacrum added A-const-fn C-future-incompatibility Category: Future-incompatibility lints T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Jul 22, 2017
@oli-obk
Copy link
Contributor

oli-obk commented Aug 2, 2018

This has been solved. All exported constants are checked even if unused

@oli-obk oli-obk closed this as completed Aug 2, 2018
@RalfJung RalfJung added the A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) label Dec 1, 2024
lnicola added a commit to lnicola/rust that referenced this issue Mar 10, 2025
…gling-dyn-diagnostic

feat: Add diagnostic for dangling dyn and impl
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, ...) C-future-incompatibility Category: Future-incompatibility lints P-low Low priority T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants