const fn's should allow (and eval) assert! and debug_assert! statements

[pnkfelix]

Right now we have ad hoc checks for certain known bad scenarios in our const evaluator ... so for example:

#![feature(const_fn)]

fn add1(x: u32) -> u32 {
    // For non-const, I can inject input checks
    assert!(x < ::std::u32::MAX);
    x + 1
}

#[cfg(cannot_build_today)]
const fn const_add1(x: u32) -> u32 {
    // but current rules for `const fn`
    // disallow this variation
    assert!(x < ::std::u32::MAX,
            "I want this assertion to fire.");
    x + 1
}

// So one is forced to instead write this:
const fn const_add1(x: u32) -> u32 {
    x + 1
}

fn main() {
    let x = -1i32 as u32;
    println!("foo(0): {}", add1(0));
    println!("bar(-1 as u32): {}",
             const_add1(x));
    println!("foo(-1 as u32): {}",
             add1(x));
}

the above code today evaluates like so (playpen) in debug mode:

foo(0): 1
thread '<main>' panicked at 'arithmetic operation overflowed', <anon>:20
playpen: application terminated with error code 101

and in release mode:

foo(0): 1
bar(-1 as u32): 0
thread '<main>' panicked at 'assertion failed: x < ::std::u32::MAX', <anon>:5
playpen: application terminated with error code 101

(i.e., there is no way to emulate the foo behavior within bar while compiling in release mode.)

Another example: since we made the NonZero constructor a const function, we cannot add a debug_assert! to its body that checks that the input is actually non-zero. (If it were not a const function, then it is straight-forward to extend the Zeroable trait with an fn is_zero(&self) -> bool predicate to use as the basis for such a `debug_assert!).

I don't feel like writing up a formal RFC yet for addressing this, but I think it would be nice to extend the const fn sublanguage to allow assert! and debug_assert! statements where the input text expression is restricted to a const expression, and then have rustc actually fail the compilation if it discovers an assertion violation at compile-time.

[oli-obk]

This is pretty hard, since assert! is expanded and then looks somewhat like this:

    if !(4 < 5) {
        ::std::rt::begin_unwind("assertion failed: 4 < 5",
                                {
                                    static _FILE_LINE: (&'static str, u32)
                                           =
                                        ("bla.rs", 2u32);
                                    &_FILE_LINE
                                })
    };

1. We could detect function calls to diverging functions and error on that. The user experience will be bad though. ("something something diverging in expansion of x in expansion of y");
2. To special case the assert-macros, we'd probably need to check every expression and figure out if it was desugared from an assert!. Is the AST still available after the HIR was created?
3. We could specifically detect function calls to ::std::rt::begin_unwind and error with the first argument as the error message.

[pnkfelix]

@oli-obk yes I was assuming we would have to special-case the two macros (or revise their expansions to call a new special cased macro)

[petrochenkov]

@oli-obk
4. Report an error when something not const-evaluatable, like begin_unwind actually needs to be evaluated and not just presents in the function, i.e. only when !(4 < 5). This is what C++ constexpr does.

Error messages from gcc and clang look pretty nice for this case. Our assert! should probably also be expanded into some wrapper function around begin_unwind with word "assert" in it for better error messages.

#include <cassert>

constexpr auto f(int a) -> int {
    assert(a == 10);
    return a;
}

auto main() -> int {
    constexpr auto b = f(11);
}

// gcc
In file included from /usr/local/include/c++/5.2.0/cassert:43:0,
                 from main.cpp:3:
main.cpp: In function 'int main()':
main.cpp:11:25:   in constexpr expansion of 'f(11)'
main.cpp:6:5: error: call to non-constexpr function 'void __assert_fail(const char*, const char*, unsigned int, const char*)'
     assert(a == 10);
     ^

// Clang
main.cpp:11:20: error: constexpr variable 'b' must be initialized by a constant expression
    constexpr auto b = f(11);
                   ^   ~~~~~
main.cpp:6:5: note: non-constexpr function '__assert_fail' cannot be used in a constant expression
    assert(a == 10);
    ^
/usr/include/assert.h:94:6: note: expanded from macro 'assert'
   : __assert_fail (__STRING(expr), __FILE__, __LINE__, __ASSERT_FUNCTION))
     ^
main.cpp:11:24: note: in call to 'f(11)'
    constexpr auto b = f(11);
                       ^
/usr/include/assert.h:71:13: note: declared here
extern void __assert_fail (__const char *__assertion, __const char *__file,
            ^

[oli-obk]

The problem with that approach is that const fn functions should be checkable on their own. So that error would occur even if you don't ever call the function. Think about it like generic functions. They can be checked on their own and when you call them you know they won't give a compile-time error.

So if we want to make this correct, we need to specify ranges and similar for all arguments, and then check the const fn on its own. A simplified version with pre-conditions:

#[pre(b != 0 && (a != i32::MIN || b != -1))]
const fn div(a: i32, b: i32) -> i32 {
    a / b
}

During the check_const pass, it would be checked whether the function will never error as long as its precondition is satisfied.
So when you call the function, an assert is inserted. But if you call it in a const environment, the precondition is simply checked and an error is given if the pre-condition doesn't hold.

Of course it's incredibly hard to check whether a function will never fail for arbitrary preconditions. I'm sure we can come up with a practical system.

[pnkfelix]

The problem with that approach is that const fn functions should be checkable on their own. So that error would occur even if you don't ever call the function.

I don't think this is a reasonable goal.

I would be satisfied with errors in these cases being delayed until there is actually a call-site acting as a witness to the erroneous condition. (In which case one would just express pre-conditions via assert! directly, assuming we were to move forward with the ideas outlined in the description of this issue.)

[petrochenkov]

@oli-obk
Hm, I assumed no such additional checking is needed.
I agree, that const fn should be ill-formed when it calls something not const-evaluatable unconditionally, but conditional compilation failure should probably be ok for the sake of practicality, it just should be mentioned in the function documentation. But this is a tricky philosophical question, yeah :)

[oli-obk]

I was under the impression that const functions should be checked if they contain errors even if they are not called. I can't find the comment I'm referring to and I'm not sure how far it was meant. It might just be for errors that occur unconditionally with any input.

I don't think this is a reasonable goal.

It was a goal for generics. But generic functions are used everywhere. The question is whether we want to take a different path than other languages or if all those specifications would be too much hazzle considering that const fn might be used rarely even once stabilized.

[petrochenkov]

#[pre(b != 0 && (a != i32::MIN || b != -1))]

Such preconditions may be closely related to bounds on integer type paremeters (if they are ever considered to be a way to go).

[pnkfelix]

I don't think this is a reasonable goal.

It was a goal for generics

Yes, I agree with that. There is reasonably well-established foundation (update: and precedent) for using the type-system as a theorem-prover for statically checking generics, and so we make use of that.

I don't think we want to impose the same set of restrictions on const fn, especially if it entails coming up with a sublanguage of preconditons that must be added to const fn's that do various types of arithmetic. (I'm being deliberately vague here, since I am not sure if your intention was to reject even things like arithmetic overflow, or your attention was restriction solely to behavior like division-by-zero...)

---

(But then again, who knows what will come in the future with respect to Rust's attitude towards dependently-typed programming... maybe my mental model is stuck in the dark ages...)

[oli-obk]

I'm being deliberately vague here, since I am not sure if your intention was to reject even things like arithmetic overflow, or your attention was restriction solely to behavior like division-by-zero...

Yes, my intention was to catch every const eval failure.

So, how about an attribute for non-const functions that makes them usable in const functions until they are evaluated? The attribute could take a string or an ident as the argument. In case of an ident, the content of the function's argument with that name will be printed as the error. Then we can mark std::rt::begin_unwind as such a function.

[Kimundi]

We could also just make begin_unwind a const fn lang item that calls out to the runtime unwinding stuff in non-const contexts, and causes the compiler to emit an error otherwise.

[pnkfelix]

@Kimundi well, your use of the word "just" may be a little misleading, since there are other changes that would need to be made to const fn for any of this to work.

(namely, we'd need to loosen the current restriction that "blocks in constant functions are limited to items and tail expressions [E0016]")

[Kimundi]

Oh, I didn't mean to imply it would be easy, just that as far as language changes are concerned adding another lang item seemed simpler than some of the other proposals here. :)

[eddyb]

I am with @Kimundi on this one - this came up a while ago, and I even suggested interpreting format arguments at compile-time, although that is overkill here.

If we rely on rvalue promotion, we can do the following:

if !(4 < 5) {
    ::std::rt::begin_unwind("assertion failed: 4 < 5", &("bla.rs", 2u32));
}

That requires implementing if in constants and having begin_unwind treated as a special const fn (even though its body is most definitely not a const fn).
But the message and the file/line information is already constant and can readily be taken apart by rustc (could maybe even produce a nice (String, Span) pair out of it).

[oli-obk]

I implemented this, but there's a small issue... panics end up expanding to

if !(4 < 5) {
    static _FILE_LINE: (&'static str, u32) = ("bla.rs", 2u32);
    ::std::rt::begin_unwind("assertion failed: 4 < 5", &_FILE_LINE);
}

Putting statics into const fns is allowed, but referencing them is not. Playpen

[eddyb]

@oli-obk This is miri territory now, I don't see any way forward that doesn't involve ripping const_eval from the compiler, so any progress there will probably be wasted (other than better compile-time checks).
Can miri do this right now? If not, what's the blocker there? cc @tsion

[oli-obk]

It's not an implementation issue, constants are not allowed to access statics in Rust. To lift that restriction we need an rfc, right? I don't even know if it makes sense to lift the restriction.

[eddyb]

@oli-obk There isn't even an access there, it's just taking a reference. Completely harmless in my model, which miri is roughly following. Even reading a static should be fine, mutations are the fundamental issue.

[oli-obk]

The last thing I got told about constants referencing statics: rust-lang/rust#25570 (comment) ;)

[eddyb]

@oli-obk The context is that in the current system, only trans could complain about &STATIC eventually ending up in accessing the underlying memory, if it could tell at all, so there are conservative limits that must be enforced.
I suppose that if the type doesn't contain UnsafeCell at all and doesn't require dropping, it could be trivially allowed (since it's effectively a const with an identity).
However, I believe that &STATIC would evaluate to a constant global with the same value, but not the same address as STATIC, because trans::consts has no concept of lvalues.

[solson]

Can miri do this right now? If not, what's the blocker there? cc @tsion

@eddyb The blocker is Lvalue::Static, which miri doesn't currently attempt to handle:

    tmp1 = &r::_FILE_LINE
thread '<main>' panicked at 'not yet implemented', src/interpreter.rs:795

See line 795 which is the Static case in eval_lvalue.

[eddyb]

@tsion Would it be possible to have static implemented with allocations marked as read-only or "static so error on mutation"?

[solson]

@eddyb Yes, that seems doable.

By the way, I think the reason I left Static unhandled for now was because rustc only has HIR for statics, not MIR, and there didn't appear to be an accessible way to cause MIR generation from miri.

[Centril]

Closing in favor of rust-lang/rust#51999.