On 07/10/13 14:49, Gavin Lambert wrote:
As for being encountered as part of UB, that is *only* if you decide to declare the constructor as having a non-null-parameter precondition by assumption instead of actually validating it. Which is not the case in the position that I am arguing for, so that is irrelevant.
The point is still that validating at runtime by throwing an exception is not good enough. If the programmer wants to use a non-null smart pointer, he wants to only ever pass in a non-null pointer. Detecting a null-pointer inside the construction of the smart pointer is already too late; the programmer thought it was non-null, so in essence, he has no idea what the code is doing... Whatever it *is* doing may not be undefined, but it is most certainly wrong. How do you recover at runtime from a program that does not do what you thought it did?
Can you please restate why you think it should be a precondition? I've reread the thread and all I can find is "Initializing the object by passing a null pointer should be UB since it is a clear violation of the constructor's assumed precondition (don't pass a null pointer)" which is a statement with an assumption, not an explanation. (You go on to quite some lengths about why throwing is bad for preconditions, but not why you think it should be a precondition in the first place.)
My argument is that it should be an explicitly-validated parameter, not an assumed-valid-via-precondition one.
If the programmer wants to use a non-null smart ptr, presumably it is because he expects his pointer to never be null. If his pointer is null, he has behaviour in his program that is not expected. You can't recover from that, unless you expect the software to rewrite and recompile itself into something that is correct and then try again.
The exception avoids continuing on (in the case of a release build, where the assert is skipped) to construct an object with an invalid invariant and then execute further code that expects that invariant to be valid, which I hope we can all agree is well into unrecoverable UB territory.
The point is that at that instant all of the program's state, both global and local, is still undamaged. (It may not be *happy* with having a null pointer, but until it tries to access it nothing undefined has yet happened, bar a quibble in documentation.)
No, the point is that the state probably is damaged; if a programmer who expects a pointer to be non-null finds himself at runtime with a null-pointer, either global state is already damaged, or there is a bug in the program and very little can be said about the current state of the data. Neither of which are particularly recoverable. The argument is this: { try { throw_if_null(p) auto p2 = shared_ptr_nonnull(p); doSomething(p2); } catch(null_pointer_exception const& e) { doSomethingElse(); } } vs { if(p) { auto p2 = shared_ptr_nonnull(p) doSomething(p2) } else { doSomethingElse(); } } Would you write the first one or the second? If the first, then do it, but many other people would prefer the second variant. Don't burden the other users of the shared_ptr with the throwing behaviour, it has nothing to do with a nonnull shared_ptr. What if the user would prefer some other exception type to be thown? Or assert? You gonna turn that into a policy? The debate is that it's nicer to let the user of your smart pointer decide how to handle the conditions that would result in indefined behaviour than to decide for them, perhaps have a factory function: auto p2 = throw_or_make_shared_ptr_nonnull(p);
There is a qualitive difference between the object throwing up its hands (aka an exception) and saying "you idiot, don't pass me a null pointer" vs. not even thinking about it and at some undefined point in the future (where it may be harder to track down the source) crashing or overwriting memory due to unchecked usage of a "never-supposed-to-be-null" pointer that secretly is actually null.
Again, in a perfect world these sorts of bugs will be caught in the debug build by the assert. But we do not live in a perfect world, and test coverage is rarely 100%.
Which is why an assert could be used. But throwing doesn't help, unwinding the stack is unlikely to help the library user detect or solve the problem in the wild; a crash dump is much better, not least because you are less likely to further corrupt data.
I challenge you to define what you even mean by this. Do you mean that the code path is simply not common, and that this somehow makes things acceptable? There are plenty of branches in code that are not "normal."
I mean that the code path is never followed at all except in case of programmer error. It's not merely "abnormal" or "rare" but "should never happen unless you have a bug", just like the assert itself. Note that this last statement is NOT equivalent to "will never happen", which is what relying purely and only on asserts encourages.
When people object to using exceptions as control flow, this is not what they mean (because if you want to extend it that far, you're basically just saying "never use exceptions").
Would it help if you thought of it as "throw std::assertion_violation" in some version of "assert" that did not get compiled out for release builds?
I guess it's just hard to determine how to recover at runtime from a program that doesn't do what the programmer thinks it does. Ben