Removing const-eval duplication of labor between librustc and librustc_trans

internals
1

Spawned off of arithmetic-overflow checks during const-eval by pnkfelix · Pull Request #23863 · rust-lang/rust · GitHub

@pnkfelix said:

There is some duplication of labor between rustc::middle::const_eval and rustc_trans::trans::consts. It might be good to explore ways to try to factor out the common structure to the two passes (by abstracting over the particular value-representation used in the compile-time interpreter).

@eddyb responded:

As for the checks in trans: I did something else for div/rem: check_const invokes const_eval for those binops on integers and errors if evaluation fails.

Sadly, that doesn't work well in the presence of const fn and associated constants.

@nikomatsakis I wonder if recording "obligations" on constant evaluation would make sense - like where clauses, but implicit and working on values (either arguments of const fns or associated constants on type parameters).

and later @eddyb added:

I personally prefer checking somewhere higher than trans, but in the end, what I am not particularly fond of is extracting values from LLVM for checking.

2

So I want to revive this, since I'm working on it.

  1. implemented ConstVal to llvm-ValueRef translation (already exists partially on master, used in MIR: https://github.com/rust-lang/rust/blob/master/src/librustc_trans/trans/mir/constant.rs#L23-L79)
  2. upgraded the Struct and Tuple variants of ConstVal to actually contain the translated version of their fields
  3. added more variants to the ConstVal enum, until it can translate anything that trans/consts can
  • already implemented
    • Ref is a &'static T that points to a const
    • StaticRef is a &'static T that points to a static
    • RValue is a dereferenced StaticRef
    • Ptr is an integer that was cast to a *const T or a *mut T
    • AddrOf is a *const T or a *mut T that came from casting a StaticRef
    • Slice is a [T]
  • TODO
    • Variant is an enum variant. Probably will have a sub-enum to denote the different variant kinds
    • probably forgot something
  1. The next step is to replace the translation of the expression of all const definitions in trans/consts with a call into const_eval and a translation of the evaluated value
  2. and then once all that works nicely think about how to translate statics. Shouldn't be too hard, since statics can't refer to other statics by value.

Now... to the problems

Step 2. is a breaking change.

I'm not sure how bad it is, but it's definitely a breaking change, because now all fields of a struct constructor are evaluated, whether they are used or not. Currently only used struct constructor fields are evaluated, allowing the other fields to be completely bogus. I reported something similar in lazy const evaluation incompatible with const fn · Issue #29928 · rust-lang/rust · GitHub . The example I found is actually a run-pass test that checks for this behavior:

struct S<T>(T) where [T; (||{}, 1).1]: Copy;                                    
fn main() {}

But we don't support closures. In fact, you can write arbitrary expression that typecheck and they will compile:

unsafe fn bla() -> i32 { 5 }
struct S<T>(T) where [T; (unsafe { bla() }, 1).1]: Copy;                                    
fn main() {}

The dead_code lint knows what's going on: Rust Playground

The moment we compute all struct and tuple fields eagerly, const_eval will try to evalutate that bla() function call which will fail for various reasons.

check_const improvements are breaking changes

I tried to start checking things early in check_const. But check_const doesn't care whether a const is ever used or not. Just adding const X: usize = 1 << 99; to an arbitrary program and not using X will emit a compile-time error, due to @eddyb's check in check_const. But const X: usize = 4 - 5; will only error if X is used, since @eddyb's check only checks the rhs of the bitshift operation.

#How to proceed

I had a similar problem when I improved the const evaluator and we decided on creating a const_err lint that gets emitted when it's not a true constant, but still a const evaluable situation (like let x = 5 + 6;).

I don't think it's just unused constants, but also pub constants. As long as noone uses them, they don't error, but the crate still compiles. If we improve check_const, it will also check pub constants, even if they are not used in the crate itself.

3

Please break it. Bugs in dead code can indicate that the programmer is making an invalid assumption (leading to a bug in live code).

1 Like
4

I think we should break code that abuses constants this way.

5

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.