I made a quick search but could not find any documented policy on when it is appropriate for a function (in std or elsewhere) to panic. Is there such a policy? If not, should one be written?
I was astounded to see such broad support among the responders for std::time::Duration::as_millis() to panic. I’ve always regarded panicking as a last resort, when there’s no other way out. The natural solutions for as_millis would of course be to return a bigger type or a Result. Any given input, as a validly constructed Duration, is also an expected value, and a valid one.
Are there other places in std or in other officially hosted or supported libraries where the question of panicking or not need to be discussed or determined?
Towards a policy
I’m going to jot down a few ideas that come to mind. Please help out in specifying a proper policy:
A valid input to a fn should not, in it self, be an acceptable cause for panicking. Now you may ask, what exactly is meant by “valid”, and that’s a fair objection that needs some examination.
A fn may only panic when there’s no other reasonable way to deal with the task, or recover from a failure.
If a non-panicking fn can reasonably be designed then that is a good first-hand choice. Panicking behaviour does not draw upon the strengths of the type system. In fact, it’s kind of a cop out in many cases and should not be encouraged.
Even though panicking is an efficient, and easy-to-use means of contract enforcement it should not be used as an excuse to not choose the proper contract. Perhaps it’s easier to just panic if the Duration cannot fit inside a u64, and most of the time it would fit, but that’s a bad reason. The semantics of the values represented is more important than some easy-to-conceive use cases.
A function should panic only if the vast majority of client code would either a) almost never trigger the panic, or b) almost always choose to panic themselves even if the error condition was exposed to them via Result.
Arguably this overlaps with your second bullet point, but I think this formulation is important too.
To me a panic means the programmer has broken some function’s contract and that’s a bug in the program (e.g. bad input should have been caught and eliminated earlier).
In case of Duration it was probably just a pragmatic choice — overflow of u64 is very very unlikely to happen in normal usage, so it either doesn’t matter, or it is a sign of a bug.
I probably go even a bit further in this approach, or simpler one.
Basically:
Did it panic? There’s a bug somewhere.
In that regard, I even wonder if the default of panic = "unwind" was a good choice.
Now, the question is if the bug is inside or outside of the function. If a function from external crate panics on me while I didn’t do anything the documentation says I shouldn’t (or something that just make no sense at all, like if multiplying matrices of incompatible sizes and the function doesn’t produce a Result, but Matrix), I open a bug report against that library. It might resolve in not panicking, in explaining why my code was bad (and updating the docs) or stating this bug can’t be fixed (but still being a bug).
If the milliseconds of a Duration cannot be computed because the value is greater than what is normal - but still a valid Duration in its own right - then panicking feels very wrong to me. Sure you can document that not all Durations are applicable but I regard that solution as juggling with knives.
What I would like to see is better use of the type system:
Accept only ShortDurations.
Return a u128 that is big enough to store the result.
Return a Result.
These alternatives can be generalised to many situations where one might consider using panics for contract enforcement:
I'd say that "almost never" isn't good enough. A function in safe rust shouldn't panic on any input value that is a valid value of it's type. If someone wants a more efficient but not completely correct option there should be an unsafe version of it where the caller explicitly affirms that they fulfill the functions contract.
That would basically mean that panic is forbidden. Values of a type that are no valid values of that type should never happen and are worse than a panic, they are undefined behavior.
I never considered whether these types of functions could panic. Some as_* functions do some minor work to return a borrowed type so I am not sure that “never panic” is realistic.
To me Duration itself seems "wrong" for many uses — it's overengineered, where a simple integer count would normally do fine (e.g. 2^64 ns is 585 years, which easily covers many of its uses — for others, a date type would be appropriate).
Like it or not, trade-offs are commonly required in any type of engineering. Integer overflow panics — this is a very similar case to Duration::to_millis not fitting in a u64, in that both indicate a code bug or inappropriate usage of the functionality.
I don't share that view. Integers do not carry any meaning - that's their weakness. I do like to use types that can only be instantiated if they are also valid, like in DDD. This gives you the best static guarantees and robustness.
I completely agree with this sentence. We should aim to control this though.
@kyrias This would basically exclude one of the most important use cases of panics: bounds checks.
Regarding the safe/unsafe distinction I’d even say that it’s the other way round. safe functions must panic, if the input arguments don’t fulfill the input contract. OTOH unsafe functions can just document the limitations and place the burden of checking on the caller.
How so? If a function doesn't support the full input type range it should perform bounds checking and return Result<T>.
And the input contract is the types. If things valid under that contract are impossible to handle for the function, that should be explicitly checked and handled, not just throw a panic around when it wasn't necessary, especially so in safe code. And this is the exact reason I said to make the panicking version unsafe, then the caller is obliged to enforce the input contract that the type system isn't able to enforce.
That's one possibility but not always the best one.
That's why there are 3 different variants to get an element of a slice:
index, i.e. normal indexing which panics on OOB
get, which returns a result
get_unchecked which does no bounds checking at all and is unsafe.
I don't think it makes sense to create a policy that contradicts the current practice.
That's one (strong) part of the contract. But there are many contracts that are not expressible with Rust's type system, and in that case panic! is the correct answer.
Of course it would be nice if every contract could be expressed as a type, but you'd need a much much more expressible type system.
What I’d like to see someday is a light-weight effect system by which you could impose constraints on all callees. One of the most important constraints would be nopanic. Neither the callee, nor any transient callees, would be allowed to panic if used:
#[effect(nopanic)]
fn foo() {
... // must not panic directly or, more importantly, indirectly!
}
The above should be uncontroversial, I hope.
A completely separate (and somewhat weird) idea, that I do not endorse btw, is some sort of panic elimination for all callees, direct and indirect. I’ve not given this any thoughts but perhaps this touches on the safe/unsafe distinction in Rust? If you, by means of this hypothetical feature, promise that you adhere to all contracts, and state that you are willing to take the consequences of all errors, then this could be a way out for you in places where you really need the last drop of performance. This means eliminating edge-case detection and resulting panicking. This is probably too weird to support of course, so I’d leave this aside.
By annotating a fn as non-panicking you are really specifying its contract explicitly.
