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

Treat const fn like fn for promotion. #75586

Closed
RalfJung opened this issue Aug 16, 2020 · 3 comments · Fixed by #76411
Closed

Treat const fn like fn for promotion. #75586

RalfJung opened this issue Aug 16, 2020 · 3 comments · Fixed by #76411
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

rust-lang/const-eval#19 gives many arguments for why we should not promote const fn calls. However, we currently do promote const fn calls in const fn, even though that has all the same problems. This fails to compile, but really we shouldn't make this a hard error:

#![allow(unconditional_panic, const_err)]

const fn foo() { [()][42] }
pub const fn do_something(b: bool) -> i32 {
    if b {
        let _x = &foo();
    }
    13
}

This does not just apply to const fn calls; all promotion inside const fn should be treated like inside fn.

Cc @rust-lang/wg-const-eval

@RalfJung
Copy link
Member Author

#75502 is a crater experiment to stop doing this (at least for the case of &call() inside const fn).

@LeSeulArtichaut LeSeulArtichaut added A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-const-fn T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 16, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Aug 17, 2020

One data point I want to bring up is rust-lang/rfcs#1229 (comment)

It's probably worth adding to the RFC a definition of "constant evaluation" vs "runtime evaluation" contexts. I think that the language has a pretty clear distinction, though:

  • the initializer of a constant const foo: ty = EXPR or static foo: ty = EXPR
  • the size of an array [T; EXPR]

The body of a const fn is sort of interesting. I think my preference is that the body is treated as a constant evaluation context, and hence an error is reported, even if the const fn is never called from a constant initializer -- so long as the overflow can be observed without knowing the values for these arguments. Implementing this, of course, will require a more sophisticated handling of constants (which we need anyhow). Since const fn is feature-gated we have time to play with this. Similar concerns will apply to generic and associated constants of course. I think from the POV of this RFC we can leave this as an "unresolved question", or at least as an open bug that will not be impl'd immediately.

From that reading I'm getting the vibes that @nikomatsakis would be mostly fine with hard erroring on promotion or even const propagation of failing code in const fn. But since we did not address this during the stabilization of const fn... this train probably left?

@RalfJung
Copy link
Member Author

The reason promotion in fn is subtle is that it needs to be codegen'd, and we cannot say if the code we are generating will ever evaluate or not. There might be things like

fn foo<T: Trait>() -> &'static i32 {
  if T::ASSOC != 0 { &(14/T::ASSOC) } else { panic!() }
}

which would never cause div-by-zero at runtime, but will have a failing CTFE query during codegen if T::ASSOC is 0.

And all of this applies equally to const fn! They are still fn, subject to codegen, and thus have all the same problems. I don't see how them being a "const context" is at all related. Even in a "const context" I would not expect code that properly guards against div-by-zero to cause a compile-time error.

RalfJung added a commit to RalfJung/rust that referenced this issue Sep 19, 2020
…n-const-fn, r=RalfJung

Use implicit (not explicit) rules for promotability by default in `const fn`

For crater run. See rust-lang/const-eval#54 (comment).

cc rust-lang#75586
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Sep 19, 2020
…tatic-morse

Some promotion cleanup

Based on top of both rust-lang#75502 and rust-lang#75585, this does some cleanup of the promotion code. The last 2 commits are new.

* Remove the remaining cases where `const fn` is treated different from `fn`. This means no longer promoting ptr-to-int casts, raw ptr operations, and union field accesses in `const fn` -- or anywhere, for that matter. These are all unstable in const-context so this should not break any stable code. Fixes rust-lang#75586.
* ~~Promote references to statics even outside statics (i.e., in functions) for consistency.~~
* Promote `&mut []` everywhere, not just in non-`const` functions, for consistency.
* Explain why we do not promote deref's of statics outside statics. ~~(This is the only remaining direct user of `const_kind`.)~~

This can only land once the other two PRs land; I am mostly putting this up already because I couldn't wait ;) and to get some feedback from @rust-lang/wg-const-eval .
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Sep 19, 2020
…tatic-morse

Some promotion cleanup

Based on top of both rust-lang#75502 and rust-lang#75585, this does some cleanup of the promotion code. The last 2 commits are new.

* Remove the remaining cases where `const fn` is treated different from `fn`. This means no longer promoting ptr-to-int casts, raw ptr operations, and union field accesses in `const fn` -- or anywhere, for that matter. These are all unstable in const-context so this should not break any stable code. Fixes rust-lang#75586.
* ~~Promote references to statics even outside statics (i.e., in functions) for consistency.~~
* Promote `&mut []` everywhere, not just in non-`const` functions, for consistency.
* Explain why we do not promote deref's of statics outside statics. ~~(This is the only remaining direct user of `const_kind`.)~~

This can only land once the other two PRs land; I am mostly putting this up already because I couldn't wait ;) and to get some feedback from @rust-lang/wg-const-eval .
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Sep 19, 2020
…tatic-morse

Some promotion cleanup

Based on top of both rust-lang#75502 and rust-lang#75585, this does some cleanup of the promotion code. The last 2 commits are new.

* Remove the remaining cases where `const fn` is treated different from `fn`. This means no longer promoting ptr-to-int casts, raw ptr operations, and union field accesses in `const fn` -- or anywhere, for that matter. These are all unstable in const-context so this should not break any stable code. Fixes rust-lang#75586.
* ~~Promote references to statics even outside statics (i.e., in functions) for consistency.~~
* Promote `&mut []` everywhere, not just in non-`const` functions, for consistency.
* Explain why we do not promote deref's of statics outside statics. ~~(This is the only remaining direct user of `const_kind`.)~~

This can only land once the other two PRs land; I am mostly putting this up already because I couldn't wait ;) and to get some feedback from @rust-lang/wg-const-eval .
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Sep 20, 2020
…tatic-morse

Some promotion cleanup

Based on top of both rust-lang#75502 and rust-lang#75585, this does some cleanup of the promotion code. The last 2 commits are new.

* Remove the remaining cases where `const fn` is treated different from `fn`. This means no longer promoting ptr-to-int casts, raw ptr operations, and union field accesses in `const fn` -- or anywhere, for that matter. These are all unstable in const-context so this should not break any stable code. Fixes rust-lang#75586.
* ~~Promote references to statics even outside statics (i.e., in functions) for consistency.~~
* Promote `&mut []` everywhere, not just in non-`const` functions, for consistency.
* Explain why we do not promote deref's of statics outside statics. ~~(This is the only remaining direct user of `const_kind`.)~~

This can only land once the other two PRs land; I am mostly putting this up already because I couldn't wait ;) and to get some feedback from @rust-lang/wg-const-eval .
@bors bors closed this as completed in 10b3595 Sep 20, 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, ...) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants