rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.58k stars 12.74k forks source link

Tracking issue for RFC 2345, "Allow panicking in constants" (const_panic) #51999

Closed Centril closed 3 years ago

Centril commented 6 years ago

This is a tracking issue for the RFC "Allow panicking in constants" (rust-lang/rfcs#2345).

Steps:

Unresolved questions:

Blockers:

clarfonthey commented 6 years ago

Don't forget expect and all the other unwrap-like methods. :P

vi commented 6 years ago

I suppose unimplemented!() and unreachable!() should also be included?

SimonSapin commented 6 years ago

I think they would be "automatically", since macros don’t have a separation similar to fn v.s. const fn.

oli-obk commented 6 years ago

I'll add some tests to #52011, but yes, it just works.

Centril commented 6 years ago

Triage: @oli-obk so this is implemented now, right?

oli-obk commented 6 years ago

Yes

IsaacWoods commented 6 years ago

Now this is implemented, is there anything stopping Option::unwrap and friends from becoming const fns (I'm assuming with a rustc_const_unstable(feature = "??"))?

Centril commented 6 years ago

@IsaacWoods to my knowledge there shouldn't be anything in the way of that.

IsaacWoods commented 6 years ago

@Centril great! I'll start work on a PR in that case

clarfonthey commented 6 years ago

The thing in the way of that is running fmt::Debug::fmt in const context.

…oh, right, I was thinking in terms of Result::unwrap and Result::expect. Option should be fine.

IsaacWoods commented 6 years ago

On second thoughts, even Option is still blocked I think - I don't think we can use match in const contexts yet either

oli-obk commented 6 years ago

Yea, we need conditions in constants first

Centril commented 6 years ago

Right; bummer.

@oli-obk Any thoughts about the first unresolved question?

oli-obk commented 6 years ago

I'm for basically reporting it verbatim, but having the same prefix (error[0080]:) as all other errors.

Centril commented 6 years ago

@oli-obk I like that; (and it's a diagnostics issue anyways, not part of the spec, so it is up to T-compiler to decide this...).

Centril commented 6 years ago

@oli-obk I think sufficient time has passed for us to stabilize this... how do you feel about doing that? If you feel alright with that maybe you could write up a stabilization report?

Nemo157 commented 6 years ago

@Centril has this been used in nightly anywhere yet? As far as I'm aware all usecases are blocked on needing control flow, otherwise all this allows is writing un-compilable code (I guess that could be generated by macros/proc-macros, but those already have compile_error! to use in the same places).

Centril commented 6 years ago

@Nemo157 this basically permits panic!("foo") at the moment and not much more.

oli-obk commented 6 years ago

Yea, without control flow this feature is useless except for unimplemented!() working in const functions

DoumanAsh commented 5 years ago

Small question. Would it allow to have functons like:

const fn static_assert(condition) {
    match condition {
        true => (),
        false => panic!("Assertion failed"),
     }
}
oli-obk commented 5 years ago

No, match is handled in https://github.com/rust-lang/rust/issues/49146

You can only do const fn foo() { unimplemented!() } as of right now.

Swoorup commented 5 years ago

Would this imply that const fn is not pure function?

clarfonthey commented 5 years ago

Panicking doesn't cause impurity. The function will still always act the same for any given input.

Swoorup commented 5 years ago

I haven't seen any write-ups mentioning const fn would never evolve to be an impure function, so I am assuming you can't rely on this for purity?

clarfonthey commented 5 years ago

I'm fairly certain that constants are by definition pure. What would it mean to have an impure constant?

mark-i-m commented 5 years ago

Imagine a const fn rand_number_gen() or something. All const means in my mind is that it runs at compile time. (That said, it's not clear to me how useful non-purity would be).

clarfonthey commented 5 years ago

I feel like impure constants would be a bad idea, as it breaks determinism in builds. That said, I guess that this discussion isn't best had here.

porky11 commented 5 years ago

@clarcharr It may be useful to run iterators at compile time. This would also require impure const fn.

clarfonthey commented 5 years ago

@porky11 I think you're misunderstanding what impure means. Impure simply means that you get different values for the same input. Mutation isn't impure, as long as it's explicit. If you're passing a mutable reference as an argument (in this case, &mut self) you can think of it as treating the iterator state as both an input and an output. The same iterator state before will result in the same state after. Therefore, this is still pure.

Panicking is pure because it essentially ends the program when you reach a panic, and the circumstances leading up to it will always be the same. Catching panics could lead to impurity, but that's not mentioned at all here. I don't think that will ever be allowed.

elichai commented 5 years ago

Would be awesome to see compile time assert!(condition) :)

clarfonthey commented 5 years ago

I believe that this will be possible with the effects traits RFC? On my phone right now but someone can probably share a link.

elichai commented 5 years ago

I actually found out(through people in the IRC) that it's already kind of possible if you cast the bool to usize and use it in indexing &[()][1 - (condition as usize)]

i.e. https://play.rust-lang.org/?gist=962e9178f07a0a534229b6d6b0c37d22

mark-i-m commented 5 years ago

~Yes, but IIUC it produces inefficient code.~

EDIT: oops, misread the comment... ignore me :man_facepalming:

clarfonthey commented 5 years ago

Yeah, there are ways people do compile time assertions but the main issue is that there's no compile-time Debug.

elichai commented 5 years ago

No code to produce in compile time operations that result in ZST :) https://godbolt.org/z/1sxuS-

RalfJung commented 4 years ago

Is there any forwards compatibility concern with eventually being able to catch panics in const fn?

oli-obk commented 4 years ago

I think only in the sense that users may expect panics in const eval to cause an error. So if you pass your function f to another function g to be called there, if f is called and panics, you may expect compilation to fail and it now isn't guaranteed, because g may catch the panic.

oli-obk commented 4 years ago

And we'd have this problem already for index out of bound panics, so there's nothing new

RalfJung commented 4 years ago

Okay. Our current implementation immediately aborts execution on a panic, we'd have to do something more clever if the panic could be caught... but I assume that should not be impossible.

Ixrec commented 4 years ago

Are there plans to allow catching panics in CTFE? I thought that feature was only useful/intended for keeping panics from unwinding into FFI, and the RFC doesn't even mention it.

RalfJung commented 4 years ago

Well I don't see why we'd not eventually permit catching panics in CTFE, given that we plan to allow pretty much everything that doesn't break determinism. ;)

golddranks commented 4 years ago

If everything goes well, if and match in constants are going to be released in 1.45 (https://github.com/rust-lang/rust/issues/49146). Is there any significant movement around this issue? I see if/match and panics in constants as a valuable combination: together they bring in the ability for libraries to define const asserts and thus have better story around compile-time guarantees and library-level compiler errors.

oli-obk commented 4 years ago

Well, there are two unresolved questions and we need to document this feature. I don't see any actual problems with it though.

golddranks commented 4 years ago

My two cents about the unresolved questions. Hopefully this stirs some discussion and gets this moving:

Should there be some additional message in the error about this being a panic turned error? Or do we just produce the exact message the panic would produce?

Currently, the error message is like this ( https://play.rust-lang.org/?version=nightly&mode=debug&edition=2015&gist=1083e1ce78d0ca89e95fb5c3fc733cf5 ):

error[E0080]: evaluation of constant value failed
 --> src/main.rs:5:5
  |
5 |     panic!("panic while evaluating panic_in_const");
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program panicked at 'panic while evaluating panic_in_const', src/main.rs:5:5
  |
  = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: any use of this value will cause an error
 --> src/main.rs:8:15
  |
8 | const C: () = panic_in_const();
  | --------------^^^^^^^^^^^^^^^^-
  |               |
  |               referenced constant has errors
  |
  = note: `#[deny(const_err)]` on by default

error: aborting due to 2 previous errors

I think the error message could be better:

This change becomes really useful if Result::unwrap and Option::unwrap become const fn, doing both in one go might be a good idea.

  • If and match are going to be stabilized, so these APIs are getting unblocked. I think we should go and stabilize their constness as a part of this push.
RalfJung commented 4 years ago

Note that so far only panics with a constant string are even implemented. That suffices for Option::unwrap, but Result::unwrap also formats the error value, and thus is far out of reach of CTFE.

golddranks commented 4 years ago

Maybe Result::unwrap should be left out of the scope then. I half-expected expect not to require formatting, but forgot that it prints both the expect message and the error value, so no luck there. It should still be possible to unwrap Results using custom functions, so the capability to do so stays valuable even if unwrap can't be stabilized right away.

golddranks commented 4 years ago

About the first unsolved question: "Or do we just produce the exact message the panic would produce?" I wonder if there's some intention there that panics could be used for bona fide custom-made error messages, and that's why there would be motivation to produce "just the exact message"? As for that, I don't think we should mix and use messages that might have originally meant for run-time error messages, in a different, compile-time context without an explanation about that context. What I mean is that the compiler should clearly say for the reason for the error something like "a constant expression panicked" and then tell the panic error message.

I think that there is need and demand for proper custom-made compiler error messages/warnings in libraries, if the library is able to detect that its API is misused etc. However, the mechanism for that shouldn't be the same than for what is normally meant for run-time panics. It might be that a panicking mechanism is fit for even that, but in that case, the object associated with the panic should be some structured object with an API designed for showing and formatting great compile-time error messages. For purposes of this feature, it's definitely out of scope.

And thus: I think that the answer to the question is: yes, there should be an error message or an explanation that "frames" the error as a panic that happened in a constant. But preferably something more lightweight than what the current message looks like.

golddranks commented 4 years ago

As for the documentation: I noticed that mentions about const fns are entirely missing from the Reference, which seems to be the relevant place of documentation for feature like this. Have the earlier stabilization PR's been neglecting updating it, or maybe there is so much churn around const fn feature that it makes sense to update it in one go?

memoryruins commented 4 years ago

mentions about const fns are entirely missing from the Reference

const fn is documented in the reference https://doc.rust-lang.org/nightly/reference/items/functions.html#const-functions, but the section is slightly out of date https://github.com/rust-lang/reference/issues/800

golddranks commented 4 years ago

@memoryruins: Thanks, I missed it.

More about the const_err: In this example using TEST inside another constant counts as "use". https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=928d36c50cceec31d97999d5ef05447a

This explains the behaviour I called earlier "seemingly buggy". Using an "erroneous" constant even inside another constant is an error, even though I expected the error propagation logic to work transitively. However, as this example that doesn't use panicking at all shows, the problem isn't directly related to allowing panicking in constants: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=702befa4f5e87e6f997307c8419cbd62

Rather, it's about the propagation and presentation of "erroneous" constants. But with allowing panicking making erroneous constants becomes more normal and expected than before, so it may need to be considered as an UX problem.