Constant evaluation failure reports an error on each use-site

[retep998]

In the past few days winapi was hit with a deluge of this warning (266 of them to be precise): warning: constant evaluation error: attempted to subtract with overflow. This will become a HARD ERROR in the future, #[warn(const_err)] on by default

I can't say I'm too thrilled as winapi makes use of this pattern quite a bit in defining constants: pub const LVN_DELETEALLITEMS: ::UINT = LVN_FIRST - 4;

The workaround is kinda ugly: pub const LVN_DELETEALLITEMS: ::UINT = (LVN_FIRST as ::INT - 4) as ::UINT;

[nikomatsakis]

cc me, @oli-obk, @rust-lang/lang

[nikomatsakis]

@retep998 presumably LVN_FIRST is 0 here?

[retep998]

pub const LVN_FIRST: ::UINT = 0 - 100;

Which means that only LVN_FIRST is actually an overflow, and all the other LVN_ constants like LVN_DELETEALLITEMS are technically fine but the lint is triggering for them as well.

[nikomatsakis]

@retep998 points out on IRC that we really ought to report exactly one overflow here -- for LVN_FIRST. I tend to agree. More importantly, it ought to be that @retep998 only needs to change that one definition, not all of them.

@oli-obk, do you agree? How hard do you think it would be to achieve that?

[retep998]

By swapping out pub const LVN_FIRST: ::UINT = 0 - 100; for pub const LVN_FIRST: ::UINT = -100i32 as ::UINT; it does eliminate the warnings on the constants that depend on LVN_FIRST.
There are only 44 hits for 0 - so the actual work done to fix it on my end is somewhat less than 266.
Do note that one of those constants on which a lot of other constants subtract from is pub const NM_FIRST: ::UINT = 0; so I still have to do the really ugly workaround for those constants.

[arielb1]

@retep998

Of course, the attempted wraparound subtraction is, of course, erroneous and should not compile. We should not emit errors on use-sites, however.

The reason you did not see the errors on stable was because we used to emit errors only on use-sites - if you actually tried to use the aforementioned constants, you should get the same error.

[arielb1]

Example:

const C: usize = 1/0;
fn main() {
    let _ = C;
    let _ = C;
    let _ = C;
    let _ = C;
}

This gives 4 duplicate errors on all versions:

<anon>:1:18: 1:21 warning: attempted to divide by zero, #[warn(const_err)] on by default
<anon>:1 const C: usize = 1/0;
                          ^~~
<anon>:1:18: 1:21 error: attempted to divide by zero
<anon>:1 const C: usize = 1/0;
                          ^~~
<anon>:1:18: 1:21 error: attempted to divide by zero
<anon>:1 const C: usize = 1/0;
                          ^~~
<anon>:1:18: 1:21 error: attempted to divide by zero
<anon>:1 const C: usize = 1/0;
                          ^~~
<anon>:1:18: 1:21 error: attempted to divide by zero
<anon>:1 const C: usize = 1/0;

[oli-obk]

These are two distinct issues. One is the new lint and easy to fix. I'll have a PR next week. The other is in the old trans code, and impossible to fix without breaking safety requires a global cache for constants until the lint is a hard error. Or we just wait for MIR to surpass the old trans

[nikomatsakis]

@oli-obk sorry, which is the issue that is impossible to fix specifically?

[oli-obk]

It's not impossible to fix, it just can't be fixed without a cache for errors and/or constants.

The issue I meant is that an error is reported additionally at a use site, when using a broken constant in a normal expression. This occurs, because trans/consts doesn't cache constants, but instead evaluates them through llvm at every use site (in contrast to statics, which are only evaluated once). Note that this error reporting happens at trans time, so if the const_err lint is set to deny, we never get there.

[nikomatsakis]

@oli-obk by error do you mean lint warning?