On 10/7/2013 6:56 PM, Quoth Matt Calabrese:
Again, if you can keep the system running in the case where you would be passing the null pointer, then it is your responsibility to do that.
I'm not disputing that.
Your sanity check is the assert.
Which disappear in release builds, so are not sufficient for bugs that are not observed until after release.
When did I say that I don't run tests on release builds? The point is you are knowingly introducing a completely separate code path that only occurs in release builds, that is only encountered as a part of UB
You were objecting to the code path not being exercisable in debug builds, and I pointed out that it doesn't matter as both paths are tested if you test both debug and release builds. And while yes, it's a different code path, it is a trivial difference, and better than the alternative. (Assert is itself a code path that is different between debug and release builds anyway, and it is common for iterators and memory allocators to have different code in debug builds, to say nothing of compiler optimisation, so it's hardly unusual.) 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.
No, it's not thrown before UB occurs. By having a function with preconditions and calling that function with values that don't meet those preconditions, you have undefined behavior. If you want specified behavior, then it's not a precondition. That said, I've already also explained why it should be a precondition. Please, let's not turn this into a vector at().
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. And again, we're only talking about the constructor here. I have no problem with operator-> etc assuming the invariant that the internal pointer is non-null, or that anything that accepts an already-constructed pointer can assume that the pointer is non-null. I just don't agree that this applies to the constructor.
An exception does not "avoid" anything. The mistake is still made and it is still a bug.
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.) 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%.
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?