Thanks for the feedback, Matt - some quick thoughts:

Not having a default constructor seems unnecessary to me. Why not just have the default constructor dynamically allocate a default constructed object? Similarly, for consistency with other smart pointers, having a conversion to bool would be useful and makes transition to the type simpler.

Often the object pointed to will not have a default constructor, so we wouldn't be able to construct one in shared_ptr_nonnull's default constructor. I also think that having a conversion to bool is misleading - in the use cases I've seen, it would lead to a lot of extraneous if( ptr ) statements, then essentially if( true ), which obscure the programmer's intentions and are easy to remove. I found the elimination of the default constructor and conversion to bool just as useful, if not more useful, than the runtime checks of the class, because they revealed places where we were default constructing or checking a shared_ptr that we expected to be valid at all times.

I don't think an exception here is proper. 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), and similar for other operations that would produce such a "null" non-null object. Rather, these should simply be asserts.

I could go either way on this one. One reason I opted for throwing is because that's the behavior of shared_ptr when you construct it with an empty weak_ptr, and this seemed like a similar situation.

I'm fine with this except for the implicitly convertible to shared_ptr part. What's wrong with explicit conversion here?

I like the implicit conversion for convenience in using the class in place of shared_ptr. Is there a reason you wouldn't want to have implicit conversion? Thanks, Luke On Wed, Oct 2, 2013 at 12:37 AM, Matt Calabrese <rivorus@gmail.com> wrote:

I agree that non-null smart pointers are a useful facility. That said I do have some additional thoughts:

On Tue, Oct 1, 2013 at 3:54 PM, Luke Bradford <lukebradford01@gmail.com

...

wrote:

...

(1) It doesn't have reset() with no arguments, doesn't have a default constructor, and doesn't have implicit conversion to bool (which would always be true.)

Not having a default constructor seems unnecessary to me. Why not just have the default constructor dynamically allocate a default constructed object? Similarly, for consistency with other smart pointers, having a conversion to bool would be useful and makes transition to the type simpler. IMO we should keep a similar interface where possible, even if the result is statically known.

(2) It throws an exception whenever there's an attempt to make it empty,

...

i.e. in constructors, reset, assignment, and swap.

I don't think an exception here is proper. 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), and similar for other operations that would produce such a "null" non-null object. Rather, these should simply be asserts.

For convenience, it's implicitly convertible to shared_ptr, and I have all

...

of shared_ptr's templated operators implemented with possible combinations of shared_ptr and shared_ptr_nonnull. Usually it can be used just by changing the type of a shared_ptr.

I'm fine with this except for the implicitly convertible to shared_ptr part. What's wrong with explicit conversion here?

-- -Matt Calabrese

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

On 02-10-2013 14:52, Jeff Flinn wrote:

As Matt said this would be a precondition violation so an assert would be better than a runtime exception.

I'd rather see we do as in Boost.PtrContainer: use BOOST_THROW_EXCEPTION such that if BOOST_NO_EXCEPTION is defined (or BOOST_SMART_PTR_NO_EXCEPTION), this becomes an assert. The run-time check during construction is not going to be noticed. It's not only a precondition we are dealing with. It's the postcondition of the constructor, that is, the invariant of the class. The normal response to failing to satisfy the postcondition is to throw. -Thorsten

I respectfully disagree. A reference is much more restricted than a smart pointer that cannot be null: it cannot change reference and it never owns the object it references.

Yes, exactly. On Wed, Oct 2, 2013 at 1:48 PM, Mostafa <mostafa_working_away@yahoo.com>wrote:

On Wed, 02 Oct 2013 06:07:40 -0700, Thorsten Ottosen < thorsten.ottosen@dezide.com> wrote:

On 02-10-2013 14:52, Jeff Flinn wrote:

...

As Matt said this would be a precondition violation so an assert would

...

be better than a runtime exception.

I'd rather see we do as in Boost.PtrContainer: use BOOST_THROW_EXCEPTION such that if BOOST_NO_EXCEPTION is defined (or BOOST_SMART_PTR_NO_EXCEPTION), this becomes an assert. The run-time check during construction is not going to be noticed.

+1

Mostafa

______________________________**_________________ Unsubscribe & other changes: http://lists.boost.org/** mailman/listinfo.cgi/boost<http://lists.boost.org/mailman/listinfo.cgi/boost>

On 02-10-2013 21:14, Matt Calabrese wrote:

On Wed, Oct 2, 2013 at 6:07 AM, Thorsten Ottosen < thorsten.ottosen@dezide.com> wrote:

...

It's not only a precondition we are dealing with. It's the postcondition of the constructor, that is, the invariant of the class. The normal response to failing to satisfy the postcondition is to throw.

This is very often the case when violating any precondition. The answer is that if you don't violate the precondition, then your variants are not violated. What you describe regarding exceptions actually changes your function's preconditions. In other words, the assert version's precondition is that you do not pass a null pointer. The throw version's precondition does not include that constraint at all -- instead, you've made passing a null pointer acceptable, with the behavior being that an exception is thrown in the case that such a pointer is passed.

So? -Thorsten

On 03-10-2013 14:47, Jeff Flinn wrote:

On 10/3/2013 7:26 AM, Thorsten Ottosen wrote:

...

...

is that you do not pass a null pointer. The throw version's precondition does not include that constraint at all -- instead, you've made passing a null pointer acceptable, with the behavior being that an exception is thrown in the case that such a pointer is passed.

So?

So you would never get a failing postcondition without having first violated the precondition. Any thing after a violated precondition is suspect. So just assert, IMHO.

It's two different guarantees, I guess. Compared to allocating somthing on the heap, the runtime check for null is cheap. If I see a class called non_null_ptr, I pretty much expect it not to be null (it's an /invariant/), and use it assuming this. How many other classes allow you to construct an illegal object (an object where the invariant is broken?). I can't give any example from Boost or the standard library. Anyway, we don't have to agree. That's why I suggested a way out, like one can do with the pointer containers. -Thorsten

On 3 October 2013 16:04, Matt Calabrese <rivorus@gmail.com> wrote:

The point is, even if you happen to throw in a particular implementation, if the precondition is such that the constructor cannot take a null pointer, then the throwing behavior is never to be relied upon by the user anyway.

It shouldn't be a pre-condition. The intent of the class is to guarantee that it is never null, it shouldn't assume that users are smart enough to get that right, because we really aren't.

On Thu, Oct 3, 2013 at 8:59 AM, Thorsten Ottosen < thorsten.ottosen@dezide.com> wrote:

I can't create a broken std::vector by means of the constructor. At least I don't know how to. WhenI think about it, std::string can crash if you pass it null_ptr. Has anybody profitted from that? It leads to subtle run-time bugs, and I ran into that a few months back.

I think it's just clear that we have some ideological differences here. std::string should not be checking for null as a part of its documented functionality.

It's pretty easy to come up with examples -- again, any

...

function with specified preconditions can do potentially this.

It's not just any function. It's the constructor.

Yeah, and? That doesn't change anything. Any function, including constructors, can put your object into a state with broken invariants if you violate a precondition because it's UB. Constructor or any other function makes no difference.

...

You forget that some people prefer to provide a nice log message and warn the users in a nice way instead of crashing the program.

If it's decided that that is what's important, then that's fine. -- -Matt Calabrese

On Fri, Oct 4, 2013 at 1:56 AM, Thorsten Ottosen < thorsten.ottosen@dezide.com> wrote:

I think you misunderstood. The std::string constructor that takes a const char* cannot be passed NULL. Then add the fact that the string constructors are not explicit. Then add overloads into the mix, some with string, some with ints.

Yeah, I do agree with some of this. I agree with the standard not doing explicit null checking for the constructor as a part of the documented functionality. I do not, however, agree with the constructor being implicit, so I'm with you there.

I'm not a big fan of defensive programming, but the world of programming is complicated, and sometimes it makes sense to be a little defensive.

Understood. I guess it is important to some, and not to others. The thing is, even with

100% test coverage, we can't guarantee that some code won't contain an invalid non_null_shared_ptr at runtime at a customer. Yes, there is a bug in one part of the program, but it certainly matter for us that we don't have to take down the whole program, but can continue gracefully.

Yeah, I do see the other side of this -- there is perhaps a middleground here that I think I'm more okay with than throwing an exception, and that is to do something like terminate, maybe even based on a preprocessor switch as to whether you terminate or simply leave it as UB (that way you can still at least avoid proceeding in a release build without introducing an exception that people could "handle"). If I'm alone on this during review, assuming it gets there, it's not something I'd really put up a stink about, but I definitely am against an exception for this type of programmer error. -- -Matt Calabrese

On 4 Oct 2013 at 9:45, Eric Niebler wrote:

This is the crux of it. If this condition really does represent a programmer error (and IMO in this case it does), then Matt is right. Throwing is wrong. Programmer error == bug == your program is already in some weird state. Continuing by throwing an exception and executing an arbitrary amount of code is not good.

Precondition violations ==> assertions. Use BOOST_ASSERT. That gives people a way to hook the behavior while giving a sane default.

Agreed. Fatal, insta-kill exceptions are underused in C++. I tend to sprinkle checks for "impossible" state all over my code with insta-kill fatal exception termination if they occur. They're very handy for detecting race conditions/memory corruption/bugs in mine or third party code. Niall -- Currently unemployed and looking for work. Work Portfolio: http://careers.stackoverflow.com/nialldouglas/

On Fri, Oct 4, 2013 at 1:09 PM, Daniel James <daniel@calamity.org.uk> wrote:

If throwing an exception when this is null is a bad idea, then not checking in release mode is surely a terrible one.

If you want some kind of a preprocessor switch for terminating on null or just leaving it strictly UB, I think that's fine, but ultimately this is an error that cannot be handled once such a condition is reached. You cannot reliably continue. All I'm saying is what Eric is reiterating -- it's a precondition violation, which is UB, so just use an assertion given that this particular precondition is easily able to be checked. Further, I vote against removing it as a precondition/turning it into documented exception-on-null, since this either A) throws an exception that cannot be properly handled, or B) invites users to rely on the exception behavior to use the exception for control flow (I.E. instead of checking for null before handing it off, they pass it off and check for null by catching an exception, which is a misuse of exceptions). I can maybe see some kind of a preprocessor hook here that users can use to force terminates in release beuilds or do logging or something, but ultimately we're dealing with UB no matter how you look at it and I feel that an assert is proper and also the simplest approach. If the type could be designed to somehow avoid the possibility entirely (i.e. via make_ functions), that's obviously the best way to go, but going back to my pointer-from-factory example, you ultimately still need some kind of raw method of construction unless you completely sacrifice a common use-case. One final approach, though one I don't really recommend, is that you could always make the raw version take a reference instead of a pointer. This side-steps the issue since it means that any corresponding UB was triggered before the function was called. Now your constructor has no non-null precondition and from the constructor's point of view the reference can't be null, so there's no reason to check. Again, I don't really advocate that, I'm just pointing it out as a possible course of action. -- -Matt Calabrese

On Sat, Oct 5, 2013 at 7:09 AM, Daniel James <daniel@calamity.org.uk> wrote:

Preprocessor switches tend to cause more problems than they solve. For a relevant example, see https://svn.boost.org/trac/boost/ticket/7028 There's confusion about what the switches do, there's inconsistencies in what they do, there are subtle issues in implementing them in a consistent manner, and to top it all, it still hasn't been resolved.

I'd rather just an assert. I only mention the switch possibility as a compromise to appease both sides.

but ultimately this is an

...

error that cannot be handled once such a condition is reached. You cannot reliably continue.

I've already given a counter-example to that. If your data is well encapsulated, an unexpected null is not an indicator of a problem in the global state.

I apologize, but can you repeat your example? Skimming back, I do not see it.

...

it's a precondition violation, which is UB, so just use an assertion given that this particular precondition is easily able to be checked. Further, I vote against removing it as a precondition/turning it into documented exception-on-null

This class is meant to make code safer - that is its point. But as you describe it, just using this class introduces the possibility of undefined behaviour where there was none before. And undefined behaviour can do anything. So it actually makes code less safe. So why use the class?

It doesn't introduce UB where there was none before, it properly brings a hypothetical source of UB to initialization as opposed to dereferencing and guarantees that when you have a constructed non-null shared_ptr, you can always safely dereference it. Saying that it somehow makes code less safe is ludicrous. It makes preconditions for functions dealing with non-null shared_ptrs simpler (since the non-nullness is already guaranteed by the invariants), and also simplifies code dealing with them.

since this either A) throws an exception that cannot be

...

properly handled

You keep saying that, but you never justify it. This kind of thing is very contextual, there are no universal laws.

I didn't think it needed explanation: If you are using a non-null shared_ptr it is because you always want the pointer to refer to an object. Therefore, if a user attempts to construct the non-null shared_ptr with a null pointer, it is a programmer error. In other words, the programmer mistakenly believes that he is passing the function a pointer that is not null. Given that the programmer thought he was passing a valid pointer, how can he now possibly handle the exception that pops out of the function? The handler can't fix the mistake in the programmer's code.

or B) invites users to rely on the exception behavior to

...

use the exception for control flow (I.E. instead of checking for null before handing it off, they pass it off and check for null by catching an exception, which is a misuse of exceptions).

You have faith that programmers won't violate your pre-conditions, but you don't have faith in them following your theories about exceptions. Which is odd since the latter is easier to do, and less problematic if it isn't followed.

Exactly what theory are you referring to? That exceptions shouldn't be used as a means of basic control flow? I'd hope that we're beyond that. I present B as simply the alternative to A in the case that the function throws on null. It's just an enumeration of possibilities. If passing the null pointer was not a programmer error as describe in A, in other words, if it was an accepted possibility that the pointer might be null on the part of the programmer, then he is simply relying on the function to do the check and throw an exception. If this is the case, then the exception is no longer an exceptional case and he is instead using the exception for basic control flow. -- -Matt Calabrese

On Sun, Oct 6, 2013 at 2:11 AM, Daniel James <daniel@calamity.org.uk> wrote:

How is that not introducing undefined behaviour? Say there's a function:

void foo(boost::shared_ptr<...> const& x) { if (!x) { something_or_other(); } else blah_blah(*x); }

By using shared_ptr_nonull, that check can be removed:

void foo(boost::shared_ptr_nonnull<...> const& x) { blah_blah(*x); }

But then a caller believes that their shared_ptr is never null, so they copy it into a shared_ptr_nonnull without checking for null first, and that action introduces the possibility of undefined behaviour where there was none before (given that there's always a possibility of bugs in non-trivial code).

With a regular shared pointer, the possibility for UB is whenever you dereference. With a non-null shared pointer, the UB possibility is only during initalization/assignment, assuming that the programmer is doing so from a raw pointer (again, people should use named construction functions whenever they can anyway, which is probably the most-common case, so this is somewhat moot). Whenever you have a non-null shared pointer, you cannot have UB simply by dereferencing it. I know you will retort "but what if someone violated preconditions" again, but I will reiterate, that is the programmer's bug. This is true with every single piece of code in the standard library that has specified preconditions and in any library at all that /properly/ specifies preconditions for functions. It's not simply by convention or doctrine or whatever you want to call it, it's because it is what makes sense. There is nothing special about a non-null shared pointer that changes this. If you violate the precondition it is your fault, not the library's. By providing named construction functions, we get by the issue partially, and these should be preferred whenever possible anyway. As to whether or not non-nullness should be a precondition, I've explained why it should be a precondition as opposed to documented check/throw and I can't say much more if you simply don't see the rationale.

...

Saying that it somehow makes code less safe is ludicrous. It makes preconditions for functions dealing with non-null shared_ptrs simpler (since the non-nullness is already guaranteed by the invariants), and also simplifies code dealing with them.

The invariants don't guarantee anything if there is no check. This false sense of security is what makes the code less safe.

Yes, of course they're still guarantees. Invariants always only hold if you keep up with your end of the contract (meeting the preconditions). It's not a false sense of security. A library can't account for programmer error. If you want an example of a false sense of security, it is having a library check and throw an exception that a programmer can't handle for the reasons I and others have already explained. If you can't deal with an unexpected exception here, how can you deal

with exceptions anywhere? Do you really think that programmers are always aware of every possible exception?

You can't deal with an exception here because it was caused by a programmer passing in a value that violated the preconditions of the function. In other words, the programmer thought that he was passing something that met the preconditions, but he was incorrect. This is a bug and his handler can't fix the bug. He thought he was passing in one thing but was really passing in something else, so you assert. The reason that you can deal with exceptions elsewhere is because they are thrown due to conditions that the caller simply can't account for at the time the arguments are passed, for instance, because those conditions may be nondeterministic from the caller's point of view. Such exceptions do not get thrown because of programmer error. And, of course, undefined behaviour allows for the throwing of

exceptions, so if you can't deal with exceptions, you can't deal with undefined behaviour.

...that's precisely the point. You /can't/ deal with it. That's why it's undefined. These are bugs that are to be found during testing, which is why you assert rather than specify check/throw behavior.

What I was getting at is that it's easier to avoid using exceptions as control flow, as you're always aware that you're doing it. You'll never do it accidentally, it's always a conscious activity. But it's harder to avoid violating pre-conditions, as you're usually unaware that you're doing it, since it's almost always done by accident. And also that using exceptions for control flow might go against our principles, but it'll still cause less harm than undefined behaviour would, since it has more of a chance of being predictable. To say that A is worse than B is not an endorsement of B, it's pointing out the problems in A.

My point isn't that preconditions, in a general sense, are easier or harder to work with than exceptions. I don't think that that is something anyone can say in a general sense. The point is that preconditions and exceptions serve two distinctly different purposes, so we are comparing apples to oranges. In this case, only an apple makes sense, so it's already end of story right there. By your logic

all use of exceptions is wrong, as we rely on functions and objects to throw when it's specified that they will throw. Anything that an exception is thrown for is an accepted possibility, otherwise there would be no exception.

No. -- -Matt Calabrese

On Sun, Oct 6, 2013 at 6:14 PM, Gavin Lambert <gavinl@compacsort.com> wrote:

On 10/7/2013 7:14 AM, Quoth Matt Calabrese:

You can't deal with an exception here because it was caused by a

...

programmer passing in a value that violated the preconditions of the function. In other words, the programmer thought that he was passing something that met the preconditions, but he was incorrect. This is a bug and his handler can't fix the bug. He thought he was passing in one thing but was really passing in something else, so you assert. The reason that you can deal with exceptions elsewhere is because they are thrown due to conditions that the caller simply can't account for at the time the arguments are passed, for instance, because those conditions may be nondeterministic from the caller's point of view. Such exceptions do not get thrown because of programmer error.

Why not? (This is the point of std::logic_error after all.)

std::logic_error is an example of the fallibility of the standards committee and simply should not exist (and similarly, vector's at()). There are plenty of people involved in standardization, some of whom are boost developers, that have explicitly acknowledged this (one is even known for formally defining the exception guarantees, and if you say his name three times, he'll probably appear in this thread). Not that authority at all matters, but be aware that we are not insane and alone here. We've expressed precisely why throwing an exception in this case isn't a good idea and it's both understood and accepted by many. If you don't immediately get the rationale or if it seems unintuitive, please try to really follow the reasoning and be open to changing your opinion. Sure, for performance reasons you want to elide the check that the

precondition has been met by the supplied arguments. I get that.

I haven't mentioned performance because that is entirely besides the point.

But that is a different argument from saying that it is somehow "wrong" to include that check and throw if they are not met, which is what it sounds like you are arguing.

If you are violating your precondition, throwing is not a proper solution for the reasons already explained several times at this point. Why the non-nullness of the pointer should be a precondition was also already explained.

And sure, the code that calls that constructor might not be prepared to deal with exceptions. But that's the entire point of exceptions -- to walk all the way back up the call stack until it finds the code that *is* prepared to deal with the exception (perhaps by cancelling a particular task, perhaps by tearing down and recreating an object or entire module or subsystem, perhaps by aborting the entire program -- but this decision must be in the hands of the application itself).

Here's an actual solution. Don't violate your function's precondition and therefore don't have UB. If your input can potentially violate the preconditions of the function, you check them before passing them off. If there is some way to handle your null case, you need to be doing so at some point before you pass that null pointer. Keep in mind that it's still often entirely acceptable for the would-be caller of the non-null shared_ptr constructor to throw an exception on null (I.E. if /that/ function's preconditions were met, but the null pointer implies that the function cannot meet its post conditions). There is a very big distinction here that is important to recognize and understand. What I would consider reasonable behaviour for this sort of "guaranteed

non-null pointer" is the following:

- constructor will assert() and then throw an exception on receiving a null argument [the first because it's a programmer error, the second because asserts might be disabled]

So in other words, you are writing a hypothetical "handler" that you've never even seen triggered/tested in a debug build (because it can't even get there in a debug build) and this handler exists because you might have some way to handle the null case and it is simply inexplicably a better choice than not violating a function's precondition and relying on UB? You don't see any flaws in this logic? This is because it is reasonable for the constructor of the class to

receive null pointers by accident via simple programming errors (and hands up everyone who has never made any of those). These are also potentially recoverable errors because it does not necessarily signify that anything has gone completely wrong.

What you are describing is a programmer error (bug). If it is /not/ a programmer error and the pointer being null is simply a possible case before calling the constructor, you should be doing that check before passing it off rather than using an exception as control flow. Again, your way of handling it may end up being to throw an exception, and in some cases that is a proper thing to do, it's just /not/ appropriate for the constructor. -- -Matt Calabrese

Moments ago, quoth I:

On 10/7/2013 4:37 PM, Quoth Matt Calabrese:

...

If you are violating your precondition, throwing is not a proper solution for the reasons already explained several times at this point. Why the non-nullness of the pointer should be a precondition was also already explained.

I have yet to see anything that convinces me that there is a valid reason for that. Maybe I missed it, or maybe we just don't agree on this aspect of code design. :)

Just to try to head off an anticipated objection, I'll requote something else you said earlier: On 10/4/2013 4:04 AM, Quoth Matt Calabrese:

If you want unraveling of the stack or catching of the exception to be worthwhile,then you need to eliminate the precondition altogether.

I agree with that. I just think that it is indeed the correct behaviour to eliminate the precondition from the constructor (and replace it with defined behaviour to throw, and ideally to also assert). I don't like the idea of public methods (especially constructors) that make assumptions about their external input without testing it. Input can *always* be screwed up, either by user or programmer error. You can be reasonably confident about your private internal data (that's what unit tests are for). You should still assert on it, of course, because compiler/linker weirdness or memory-overwrite damage from bugs in other code can get you into a weird state, but that sort of thing truly is in UB territory and there isn't much you can do to recover from that.

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?

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

On 08/10/13 05:56, Gavin Lambert wrote:

On 10/7/2013 11:41 PM, Quoth Ben Pope:

...

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?

It depends on the program, which is why recovery has to be in the hands of the program.

It's more general for a non-null pointer to describe the situation of non-null, than it is for the smart pointer to describe that situation and also do validation and potentially throw an arbitrary exception.

Maybe the pointer was null because an earlier operation transiently failed, in which case it could be retried, or the operation abandoned and the next operation attempted (think of a web server or similar -- just because processing of one request got it into a bad state doesn't mean that it can't successfully process other requests, since each request operates on independent state).

If a previous result can fail, that result should be checked during the normal flow of the code. The programmer can then decide to branch or throw an exception, or abort or whatever he wants to do. You asked for opinions on non-null shared_ptr; it will be more generic, more composable and more useful if it doesn't also validate its argument and throw. The throwing behaviour can be added with a factory or a new type that composes this type, but it can't be removed. Ben

Gavin Lambert wrote:

On 10/7/2013 6:56 PM, Quoth Matt Calabrese:

...

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.

While I'm not Matt Calabrese, I think I can restate why it should be a precondition *especially* on the constructor. The entire point of the proposed non-null pointers is to guarantee that the pointer is not null, so that receiving functions and storing objects can assume that it is not null. If you zero-initialize a pointer whose very purpose is not to be null, you defeat the purpose of its existence. Note how this is entirely analogous to std::sort, whose purpose is to perform an operation on a valid range. If you pass an invalid range, you defeat the purpose of the existence of the algorithm. This is why such algorithms have the precondition that you pass a valid range, IOW, that you only use them for what they were meant to be used for. Also note that a developer has the full power to decide whether they call std::sort with a valid range or not. If for whatever reason there may be any uncertainty about the validity of the iterators passed to the algorithm, it is possible to redesign the program to remove that uncertainty. Similarly, a developer can design their program to guarantee with 100% certainty that non-null pointers will never be zero-initialized. The simplest way to do that is by initializing all non-null pointers with make_shared or another non-null pointer. -Julian

On 10/7/2013 11:47 PM, Quoth Julian Gonggrijp:

While I'm not Matt Calabrese, I think I can restate why it should be a precondition *especially* on the constructor. The entire point of the proposed non-null pointers is to guarantee that the pointer is not null, so that receiving functions and storing objects can assume that it is not null. If you zero-initialize a pointer whose very purpose is not to be null, you defeat the purpose of its existence.

This sounds to me like an argument *for* the position I am suggesting, not against it. Throwing an exception means that the internal bare pointer is never initialised to null (or at least that if it is initialised to null, it can never be accessed as such because the resulting object is not retained), which means that it will never be accessed in an undefined way. If you fail to throw the exception (and to assert, because you're in a release build) then you *do* go on to set the bare pointer to null, and *the program does not immediately fail*. By contrast, it will fail at some unknown later point at which someone actually attempts to access the pointer. If you're lucky, that's in the following line, and the bug is easier to find. If you're unlucky, it's three hours later (because the pointer was created as a member field in another object that is only accessed rarely) and it's much harder to figure out what happened. Checking in the constructor is instant and failsafe. Not checking is just leaving a time bomb in the code.

Note how this is entirely analogous to std::sort, whose purpose is to perform an operation on a valid range. If you pass an invalid range, you defeat the purpose of the existence of the algorithm. This is why such algorithms have the precondition that you pass a valid range, IOW, that you only use them for what they were meant to be used for.

The difference in that case is that there is no practical way to test for an invalid range, so you have no choice but to rely on the caller getting it right. (At least not without help from the iterators themselves, which is sometimes supported but implementation-specific and thus not reliable.) Verifying that a supposedly-not-null bare pointer is in fact not null is a trivial check, and one that it seems worthwhile to do prior to storing that pointer in an object that wants to guarantee that its stored pointer is not null.

The simplest way to do that is by initializing all non-null pointers with make_shared or another non-null pointer.

make_shared can fail and throw an exception. So the user already has to be prepared to deal with that concept anyway. And while I don't think boost's implementation supports it, it's not unreasonable to think that some STL's implementation of std::make_shared could return an empty pointer if the user has configured "new" to return null instead of throwing an exception on failure, which would then push the responsibility of throwing the exception onto the not_null_ptr's constructor, or of needlessly complicating every single construction site to avoid UB.

On 10/8/2013 12:16 PM, Quoth Julian Gonggrijp:

I'm sorry to say this, but I think you're now just being evasive. A few posts ago in a reply to Matt, you stated that you agreed that if the assertion is to be replaced by a throw, the precondition should be removed from the constructor. This means that you also agree that if the precondition is *not* removed from the constructor, the assertion should *not* be replaced by a throw (modus tollens).

That's not modus tollens, that's denying the antecedent. Which is a fallacy.

I don't think that anyone disagrees that in the ideal world, all conditions are always checked. Apparently we also agree that checks on preconditions should be assertions when a check is at all possible. So the discussion should be about whether not passing null to the constructor is a precondition.

I'm not trying to be "evasive" or anything, I just think I possibly don't have the same definitions for some of the terms you are using. I have I thought quite clearly stated that: - it should assert != null - it should throw if == null and it survived the assert I don't really care what you call that behaviour, whether this is a "precondition" because it's asserted or whether it's not because it's checked for and thrown, or whatever. Maybe this is not what you are saying, but I get the definite impression from Matt's posts that he thinks that the act of declaring it as a precondition of the constructor means that it must not be tested for in the constructor except as an assert. I do not agree with not testing this; you can read what you like into whether this means that I don't agree with the earlier assertion in this paragraph or whether it means that it shouldn't be a precondition. If it doesn't fit within your worldview to do both things, then you could leave out the assert (though I think this would be a mistake). I don't think that leaving out the throw can ever be a good idea, for reasons I have already explained. (The only reasonable justification for not performing this sort of check is if it is hard to do -- which it isn't; or if it will cause a performance hit -- which will be negligible to nonexistent in this case.)

On 6 October 2013 19:14, Matt Calabrese <rivorus@gmail.com> wrote:

On Sun, Oct 6, 2013 at 2:11 AM, Daniel James <daniel@calamity.org.uk> wrote:

...

How is that not introducing undefined behaviour? Say there's a function:

...

void foo(boost::shared_ptr<...> const& x) { if (!x) { something_or_other(); } else blah_blah(*x); }

By using shared_ptr_nonull, that check can be removed:

void foo(boost::shared_ptr_nonnull<...> const& x) { blah_blah(*x); }

But then a caller believes that their shared_ptr is never null, so they copy it into a shared_ptr_nonnull without checking for null first, and that action introduces the possibility of undefined behaviour where there was none before (given that there's always a possibility of bugs in non-trivial code).

No response to this example?

With a regular shared pointer, the possibility for UB is whenever you dereference. With a non-null shared pointer, the UB possibility is only during initalization/assignment, assuming that the programmer is doing so from a raw pointer

Undefined behaviour doesn't end at after the constructor, it sticks around and affects every other method call. So what was undefined behaviour is still undefined behaviour. And since before you can have null-based undefined behaviour in a shared pointer, it must first be set to null, the probability is strictly greater. It doesn't matter if there are less places it can happen. Btw. it isn't just raw pointers that need checking. It's quite hard to account for everything that needs to be checked.

Whenever you have a non-null shared pointer, you cannot have UB simply by dereferencing it. I know you will retort "but what if someone violated preconditions" again, but I will reiterate, that is the programmer's bug.

You do have a very odd definition of "cannot".

This is true with every single piece of code in the standard library that has specified preconditions and in any library at all that /properly/ specifies preconditions for functions. It's not simply by convention or doctrine or whatever you want to call it, it's because it is what makes sense. There is nothing special about a non-null shared pointer that changes this. If you violate the precondition it is your fault, not the library's. By providing named construction functions, we get by the issue partially, and these should be preferred whenever possible anyway.

As to whether or not non-nullness should be a precondition, I've explained why it should be a precondition as opposed to documented check/throw and I can't say much more if you simply don't see the rationale.

It's a bit arrogant to believe that if someone disagrees with you, it's because they don't understand your argument. I haven't called anything doctrine. If I was going to use a religious word, I think orthodoxy would be more appropriate.

...that's precisely the point. You /can't/ deal with it. That's why it's undefined. These are bugs that are to be found during testing, which is why you assert rather than specify check/throw behavior.

Do you really think all such bugs can be found in testing? This is part of the problem.

...

What I was getting at is that it's easier to avoid using exceptions as control flow, as you're always aware that you're doing it. You'll never do it accidentally, it's always a conscious activity. But it's harder to avoid violating pre-conditions, as you're usually unaware that you're doing it, since it's almost always done by accident. And also that using exceptions for control flow might go against our principles, but it'll still cause less harm than undefined behaviour would, since it has more of a chance of being predictable. To say that A is worse than B is not an endorsement of B, it's pointing out the problems in A.

My point isn't that preconditions, in a general sense, are easier or harder to work with than exceptions.

I wasn't explaining your point, I was explaining my point.

On 4 October 2013 17:45, Eric Niebler <eniebler@boost.org> wrote:

...

On 10/4/2013 9:20 AM, Matt Calabrese wrote:

...

but I definitely am against an exception for this type of programmer error.

This is the crux of it. If this condition really does represent a programmer error (and IMO in this case it does), then Matt is right. Throwing is wrong. Programmer error == bug == your program is already in some weird state. Continuing by throwing an exception and executing an arbitrary amount of code is not good.

I don't think this is always the case. For example, unexpected output from a parser should not >be an indicator of an error in the global state.

...

Precondition violations ==> assertions. Use BOOST_ASSERT. That gives people a way to hook the behavior while giving a sane default.

If throwing an exception when this is null is a bad idea, then not checking in release mode is >surely a terrible one.

It is a relief to me that this is at least a subject of debate. I've read the doctorine of asserts-on-pre-condition-failure for years on forums/mailing lists, but never felt I can truly apply it in the systems I have to deal with in real-life. Even if you concentrate on errors that could only be possibly be client programmer error (as opposed to some weird environmental condition), it rarely seems to be the case that the purist view "this single thing is wrong so I can't trust _anything_ about my system, I have to end now, leave no logging entrails, or give the client any chance to do any sort of salvage" is the least-worst practical option. For me it is often the case that I do have to assume some entire subsystem is invalid (e.g. one entire thread) and have to throw a long way out (and, yes, execute destructors), but at least soldier on or shut down relatively cleanly. If the destructors go bad in this teardown then you end up calling abort due to the double-throw rule.. but hey that's no worse than asserting in the first place! As Thorsten says, devs are not infallible and customers don't like their apps going bang.. Is my experience atypical? Pete

On 5 October 2013 14:49, Stewart, Robert <Robert.Stewart@sig.com> wrote:

Pete Bartlett wrote:

...

On 4 October 2013 17:45, Eric Niebler <eniebler@boost.org> wrote:

...

...

On 10/4/2013 9:20 AM, Matt Calabrese wrote:

...

but I definitely am against an exception for this type of programmer error.

This is the crux of it. If this condition really does represent a programmer error (and IMO in this case it does), then Matt is right. Throwing is wrong. Programmer error == bug == your program is already in some weird state. Continuing by throwing an exception and executing an arbitrary amount of code is not good.

I don't think this is always the case. For example, unexpected output from a parser should not be an indicator of an error in the global state.

There should be code between the two ensuring that the output of one doesn't violate the precondition of the other.

Just because there should be something, doesn't mean that there is. People get this wrong all the time.

...

...

...

Precondition violations ==> assertions. Use BOOST_ASSERT. That gives people a way to hook the behavior while giving a sane default.

If throwing an exception when this is null is a bad idea, then not checking in release mode is surely a terrible one.

If the code isn't checking for precondition violations, then it isn't ready to handle the exception, either. If the programmer knows enough to handle the exception, then s/he knows enough to prevent it.

If you avoid excessive state and make good use of RAII, handling exceptions is generally easier than avoiding bugs. Of course, that does depend on what sort of software you're writing. And if the programmer knows enough to avoid using a null pointer, then they why would they use a class that ensures that a pointer isn't null. Anyway, there are two separate issues here: 1) Whether or not the check should always be present. 2) What should happen when the check fails. My strongest argument concerns the first point, and I hope it's clearly the one I'm more concerned with. So I'm finding a bit odd that it's not really being addressed by most responses.

On 6 October 2013 12:04, Rob Stewart <robertstewart@comcast.net> wrote:

If the exception is triggered by an unknown condition, such as you're advocating, then there can be no handler for it. It must, of necessity, unwind to the top. Code cannot rightly continue as if there had been no exception. In that case, there's little gained over an assertion, save for the possibility that other threads may be able to complete independent work.

later:

The likely outcome of failure in this regard, in a non-debug build, is a null pointer dereference elsewhere. Unchecked, that means a segfault. IOW, the problem will be found.

So you're saying that when it's an exception, it's triggered by an "unknown condition" not by a null, which is surely a known condition. But when there's no check it's known to be a null pointer that will be quickly dereferenced before anything bad might happen. Even if it's in a data structure, or in a container after hundreds of pointers which aren't null. And you're claiming that after an exception the code will continue as if there was no exception. Is that really what you do after catching an exception? But when there's no check, it will do the right thing and continue, which is course as if there was no exception, because there wasn't one. But that's okay now for some reason. Do you see why I don't find your argument persuasive?

The point of the proposed class is that, once (properly) instantiated, it's a signal to code that a null check is unnecessary, just as using a value or reference is.

No it isn't. The description was, "a variation on shared_ptr which is never allowed to be empty".

On 10/8/2013 2:09 PM, Quoth Rob Stewart:

If a null pointer occurs somewhere in your code and this class' ctor throws an exception, the handler has no idea of the source of the null pointer. Thus, the you have an unknown condition as far as the handler is concerned. (I had been discussing a handler in main() or a thread procedure, as opposed to one very near the object mis-construction.)

OTOH, if a null is actual stored in the object, then it will eventually be dereferenced, in the general case anyway, and the segfault will indicate which instance was bad. From that you should be able to discover its genesis, at least if the object's address is consistent or the pointee's type is unique. That makes finding the source of the null pointer somewhat more tractable without a debugger.

This is only the case if exceptions do not include loggable callstacks. Which is not the case in my environment, making exceptions far superior to a segfault. Maybe this is one of the differences leading to some of the disagreements I've had in the other branch of this thread. :) Even without callstacks, it may still be possible to sensibly recover and continue work by abandoning a particular operation at some high-but-not-right-at-the-top-of-the-thread level. Imagine a webserver or the simulation engine that Pete referred to. While it's *possible* that the null came from some global state that will break all future operations, it's more likely that it was some local state within the operation that becomes irrelevant when it starts processing the next operation anyway. Throwing an exception lets you choose how to handle it (or to choose to not handle it and get behaviour little different from a segfault). Letting it segfault gives you no choice, unless your platform lets you convert a segfault into an exception. (And sure, it should be handled more gracefully than this anyway, but we're talking about the case when the caller forgets to check themselves.)

On 08-10-2013 19:19, Nevin Liber wrote:

On 7 October 2013 21:18, Gavin Lambert <gavinl@compacsort.com> wrote:

...

While it's *possible* that the null came from some global state that will break all future operations, it's more likely that it was some local state within the operation that becomes irrelevant when it starts processing the next operation anyway.

In other words, you are pretending the code knows the root cause of the programming bug it was never supposed to have and can magically recover from it.

(Not sure what this has to do with Boost anymore...)

We are indeed going in circles. I can respect that some people don't want the overhead of the runtime check. I can also respect that people don't want to take down the whole server because of a bug in some subsystem. Software development is such a diverse field, that it's hard to find a one true way of doing things. We just need to find a way to give both camps what they want, since both a valid use-cases. Some libs uses a system of macros to do this, but it's usually to enable/disable all exceptions. Others, like Boost.PtrContainer, allow you to customize the behavior in the type declaration. kind regards -Thorsten

Nevin Liber wrote:

On 8 October 2013 13:17, Thorsten Ottosen wrote:

...

We just need to find a way to give both camps what they want, since both a valid use-cases.

No, we don't. Decide if your API has preconditions or not. For instance, the real difference between vector::operator[] and vector::at() is that the former has a precondition while the latter does not. If you define the behavior, it isn't a precondition, precisely because you are explicitly allowing it, and a correct program can call it with any value it likes.

This is, of course, one of the many hard parts of library design. Make strong choices. Making it "configurable" whether or not something is a precondition is a weak choice.

This, of course, doesn't mean you can't be friendlier if/when you detect a precondition violation in debug mode. But be very clear that it is a precondition violation resulting in undefined behavior.

+1

Is template < typename T >struct MySmartyPointer{T *p; explicit constexprMySmartyPointer( T *p, bool throw_on_null = false ): p( p || !throw_on_null ? p : throw Something{} ){}}; a good substitute?

...

On 8 October 2013 13:17, Thorsten Ottosen <thorsten.ottosen@dezide.com>wrote:

...

We are indeed going in circles. I can respect that some people don't want the overhead of the runtime check. I can also respect that people don't want to take down the whole server because of a bug in some subsystem.

Daryle W.

I'm going to add a tangential set of comments to this thread, which has evolved into discussions relating to design-by-contract (among other things). First, I'm completely in the camp of "if you define a pre-condition and it is violated, don't throw an exception or otherwise change the undefined behavior to some sort of different execution path". One of the arguments for throwing an exception (or related design) for logic errors is to aid in bug detection as well as provide a path for abnormal error recovery. Both of these are good, real-world considerations, but (to me) are separate from the DBC discussions. Every non-trival app has bugs in it, and every execution environment has many ways that execution can fail, whether due to software bugs or other problems in the environment. If there are fault-tolerant or high-availability requirements (and I've had a lot of these in my career, as well as working in environments where crashes are not that critical), this must be addressed at a macro / higher level. You cannot design and code for every possible path of failure in an environment, whether throwing exceptions or some other form of error detection and recovery. This might involve process monitoring and restart - segment faults, divide by zero, illegal operation, etc. Or maybe hardware has failed or errored - disk is full, memory card has failed, etc - and a failover or hardware swap-out or cleanup must be performed. Or maybe just a graceful shutdown, if possible - log, cleanup, cleanly shutdown. Adding additional design and code paths for application logic bugs complicates the code, and (obviously) still doesn't achieve 100% safety. Every divisor can be checked for non-zero in code before division, but the benefit eludes me. A stack dump could be generated, aiding in finding the offending code, but there's still no easy recovery other than "starting over", since the underlying design assumptions (preconditions) have been violated. "Starting over" might be exiting the thread or process and re-starting, or it could be a hardware trap that generates an exception that is caught at a high-level and goes through some other sort of "start over" logic. Null pointer checks where the precondition is a valid non-null pointer follow the same logic. (BTW, a non-null pointer might not be a valid pointer - besides checking for null, should the library also check that the pointer is within a valid process data memory area?) I have lots of opinions relating to software reliability (some formed by interacting with flight avionics software and the related validation processes and costs), but I don't want to wander any farther off topic than I already have. Cliff

On 10/10/2013 7:58 AM, Quoth Nevin Liber:

When I say one should assert on a detected precondition failure, it's really shorthand for putting the system into a known state. A mission critical system, for instance, might do something vastly more complicated than aborting.

I just don't see how throwing, which normally involves running code dependent on state, accomplishes this.

Throwing unwinds out of the current call stack to some higher level point of code that has declared that it is prepared to handle errors in some meaningful way. Imagine a server process which has a task loop to process a list of pending operations (each of which are completely independent, or can gracefully fail if they depend on some previous operation that failed -- eg. a webserver or database). Inside the loop it has a try/catch around the call to actually process the operation. Somewhere in the depths of the operation processing, it hits a code path with a bug and a null pointer is encountered where the code did not adequately check for it. One of these things can happen: 1. None of the code paths check for it and the null pointer is actually dereferenced. Depending on the platform, this may immediately terminate the server (bad), perform dodgy things to memory (extremely bad), or throw an exception (which will terminate the server if uncaught, or offer some possible means for recovery). Tracing a callstack from this exception would reveal the place where the pointer is used, which is *not* the site of the actual code bug. (Although it does allow you to guess that in most cases.) 2. The assignment to the "this pointer is never null" pointer did actually check that it wasn't null and throws an exception. This eliminates the possibility of dereferencing this pointer later (since it never really existed), and tracing the callstack reveals the place where the pointer is constructed, which *is* the site of the bug. If we assume that an exception was thrown, then the catch handler in the operation loop can mark that operation as failed (and intervening catch handlers or destructors could have rolled back anything they needed to). Subsequently it would be able to continue running operations. Obviously any dependent on the failed operation would also fail, but that can be dealt with too. As for what happens next, it depends on where the bug actually was and how critical that null pointer is. Maybe it was a temporary failure in some local state, so would not affect any other operations at all -- in this case taking down the entire server is obviously overkill. Or maybe it was a more serious failure in shared state and all (or most) future operations would also fail. In this case it should become apparent quickly to the user and they should be able to do a controlled restart of the server and contact someone to fix the underlying problem. Either way there should be a log of why the operation failed that will help to discover the source of the bug. And note that stack unwinding all the way up is still well defined at this point -- some people have argued that "oh, you're already in an undefined state because you've violated the precondition!" but that's circular logic. If you check for it and throw, it's no longer a precondition, and so nothing has been violated and no UB has occurred yet. Throwing prevents UB from occurring later as a result of "successfully" constructing an object with an invalid invariant. I'm not saying that preconditions are bad. Everything has an implicit precondition of "no other code has trampled over my memory" and "the CPU and RAM actually work properly" after all. But a precondition represents a case where invalid input is not tested, which allows bugs to propagate. Sometimes this is not avoidable (as I said in an earlier post, it's essentially impossible to verify that a pointer isn't dangling). But a null check is quick and easy. Leaving specific inputs as undefined behaviour, when it is cheap to specify that behaviour, is just being lazy. Obviously throwing and catching exceptions doesn't magically fix bugs in the code. (And if done badly, it can make things significantly worse as bugs that have already caused damage to shared state are ignored.) But as long as you do it properly, it does allow you to limit the damage caused by bugs in the code if the "this is possible but not valid, and would leave my object in an unhinged state if I accepted it" cases were allowed to continue unchecked. I'm all for early termination when the developer is around to sort it out immediately. But that's easy too -- just configure a debugger to intercept exceptions immediately when thrown. But instant termination is not a good user experience.

On 10/11/2013 4:00 AM, Quoth Nevin Liber:

In general, destructors are written assuming the invariants of the class hold on entry to the destructor.

And..? If the constructor of an object throws, its destructor is not invoked. So all destructors that are called should still have good invariants. It's only if you *don't* throw from the constructor that you'll have a problem, because now you *do* have an object with an invalid invariant. (And if "my internal pointer is non-null" is not an invariant of this class, then it has no reason to exist.)

In order for what you say about stack unwinding to be true, you have to assume that the null pointer isn't due to a broken invariant somewhere else. Where is your evidence that no other object or program invariants are broken?

Where is your evidence that they are? Given that this must (by definition) be the first time that this particular pointer has been assigned to a "must be non null" smart pointer object, everything else higher up the call stack either must not be using the pointer or must be ok with the idea that it could be null, otherwise the problem would have been encountered earlier.

Data corruption is a far worse user experience.

Granted. But there shouldn't be any of that here, unless the programmer doesn't write their higher-level catch handler properly.

On 10/11/2013 11:50 AM, Quoth Nevin Liber:

If everything is good, where did the bad null pointer come from?? More likely than not it is due to another (undetected) broken invariant.

Possibly, but it's more likely that it came from an unchecked failure condition (by which I mean that a particular method can intentionally return null sometimes, but the developer forgot to check for that). Developers are in general notoriously bad at remembering to do the error checking.

It is up to the designer of the library to determine if passing parameters that make it impossible to set up the class invariant is reasonable or not. If it is reasonable, it is part of the contract, and no problem.

If it is reasonably possible for a method to detect that its input would cause it to violate the class invariant, then it should report that failure in a defined way without UB. The only defined way that a constructor can fail is to throw an exception. Therefore, if it is feasible for a constructor to detect that it has been given invalid input, it should throw an exception. The only time UB should be permissible is if it is not feasible to validate the input, or there is some compelling (and documented) performance reason to skip the validation. Otherwise the library designer is just being lazy. Given that we're talking about a simple null check here, there's no excuse to skip it.

If it was okay with it being null, why did it pass it to the non-null smart pointer?

You are referring to two different things as if they were the same. The "it" being ok with the pointer being null is the code further up the callstack (destructors via stack unwinding) while the "it" with the bug is the immediate caller of the non-null pointer that didn't validate the pointer-that-could-be-null after obtaining it from somewhere-that-could-be-null-because-it-didn't-use-a-not-null-pointer. And because the caller didn't think about it.

Could you please show us your universal catch handler that handles every possible programming error w/o aborting, including corruption that might happen during the stack unwinding?

Could you show how stack unwinding could possibly cause corruption? There's no reason for it to do so, certainly not merely in the face of a bug that led to a pointer being unexpectedly null. Sure, it's possible for there to be lots of bugs, and you're never going to have something that can comprehensively resolve everything. But buggy destructors are something that you tend to notice pretty early on during development. As to whether you can recover, as I said before it's very program-specific and you can't give something that would work generically. But you're more likely to be able to successfully recover in a program that operates on independent requests (eg. webserver) or that is specifically designed to track actions for possible rollback (eg. database) or something highly modular with subsystems that can be destroyed and recreated. It's probably not going to be feasible in something that has a lot of mutable shared state.

On 9 October 2013 10:43, Rob Stewart <robertstewart@comcast.net> wrote:

On Oct 8, 2013, at 2:37 PM, Daniel James <daniel@calamity.org.uk> wrote:

...

On 6 October 2013 19:14, Matt Calabrese <rivorus@gmail.com> wrote:

...

On Sun, Oct 6, 2013 at 2:11 AM, Daniel James <daniel@calamity.org.uk> wrote:

...

How is that not introducing undefined behaviour? Say there's a function:

...

void foo(boost::shared_ptr<...> const& x) { if (!x) { something_or_other(); } else blah_blah(*x); }

By using shared_ptr_nonull, that check can be removed:

void foo(boost::shared_ptr_nonnull<...> const& x) { blah_blah(*x); }

But then a caller believes that their shared_ptr is never null, so they copy it into a shared_ptr_nonnull without checking for null first, and that action introduces the possibility of undefined behaviour where there was none before (given that there's always a possibility of bugs in non-trivial code).

No response to this example?

Why does the author of foo() know enough to test for null, but the caller of foo(), knowing that they are creating a shared_ptr_nonnull, doesn't?

I better explain this, it's easier because foo is only defined once with the assumption that the pointer can be null because it really doesn't know where the pointer came from. But there can be a large number of calls, so when changing foo every call would need to be altered. The dangerous part is when altering the code the programmer may make assumptions because they think they know where the pointer is coming from at the call site. Often they actually do, but sometimes mistakes are made, and the more places the more likelihood of a mistake. Generally speaking, such defensive programming shouldn't be scattered all over the place.

On 09-10-2013 17:18, Nevin Liber wrote:

On 9 October 2013 01:31, Thorsten Ottosen <thorsten.ottosen@dezide.com>wrote:

You seem to be completely obsessed by the point that a precondition or

...

invariant violation must always be checked by compiled-away assertions.

You seem completely obsessed to call it a precondition or invariant violation. Why? All you have to do is make it defined behavior in your library; then it isn't a bug and apparently everyone will be happy.

I'm ok with that, although I think it's possible to have the concept of broken contracts separate from the concept of how to deal with broken contracts. Is that an unreasonable oppinion? Someone suggested that there should be both an assertion and a throw. I would prefer that approach.

...

Your argument is that there is a bug in the program and that you should terminate anyway. Sadly, must programs have bugs, but we use them anyway.

Most programs don't have detectable (by code) bugs.

Just make sure your engineers, business owners and clients understand the risk they are taking on.

...

In practice it matters for some to avoid UB.

I thought in practice it matters to avoid bugs.

That matter's too, of course.

...

It's a major benefit of using Java or C#.

And yet programming bugs happen in those languages too.

Of course.

The great thing about UB is that you can tell people "there be dragons; don't go there", and you can even add checks in some builds to detect such bugs. If the behavior is well defined, you can't do that, because a programmer could be legitimately calling it for that behavior.

Well, the object won't be constructed, so it would just be equivalent to a throw statement. (I have never been part of any discussion saying exceptions are particular bad. They have uses and misuses.) I can't think of anyone that would do that on purpose, but I can think of people that might do it by accident. regards -Thorsten

On 10-10-2013 17:19, Nevin Liber wrote:

On 10 October 2013 03:54, Thorsten Ottosen <thorsten.ottosen@dezide.com>wrote:

...

I'm ok with that, although I think it's possible to have the concept of broken contracts separate from the concept of how to deal with broken contracts. Is that an unreasonable oppinion?

If you are defining the behavior, I don't see how it isn't part of the contract. A contract is pretty much "if you pass in X, I'll do Y".

Look at the difference between vector::at() (wide contract with no preconditions) vs. vector::operator[] (narrow contract with the precondition that n < size()). It is impossible to violate the contract when calling at().

I guess it's because I view the precondition as the requirement for correct execution. It's the same for both functions. They differ in how they respond to an incorrect argument. UB vs. an exception. There is a difference between general program behavior and program correctness and contracts. Assume we specify at() this way: Precondition: n < size() Postcondition: returns a reference to the n'th element. If the precondition is no met: Throws out_of_bounds and operator[]: Precondition: n < size() Postcondition: returns a reference to the n'th element. If the precondition is no met: UB At least some people don't mix normal behavior with exceptional behavior (and so they operate with two different types of postconditions, the normal and the one that happens in case of an exception). /The function needs to return for its normal behavior to have happened/. I guess there exists variations of definitions of the terms used in this thread, and that the terms are not always defined as precise as one could want. Hope this clarifies my view a little. There are many ways to specify behavior, and one particular description may be enough to imply other facts that are left unsaid. Finally, let's take a look at http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3690.pdf 1) "precondition" is not defined. All I could find is this: "Requires: the preconditions for calling the function" 2) Postcondition is defined like this. "Postconditions: the observable results established by the function." 3) 17.6.4.11 says: "17.6.4.11 Requires paragraph [res.on.required] Violation of the preconditions specified in a function’s Requires: paragraph results in undefined behavior unless the function’s Throws: paragraph specifies throwing an exception when the precondition is violated." So at least some people have used my terminology before. kind regards Thorsten

On 10-10-2013 20:56, Nevin Liber wrote:

On 10 October 2013 12:09, Thorsten Ottosen <thorsten.ottosen@dezide.com>wrote:

...

I guess it's because I view the precondition as the requirement for correct execution. It's the same for both functions. They differ in how they respond to an incorrect argument. UB vs. an exception.

That's you.

The standard is quite clear on this: n3690 23.2.4p17: "The member function at() provides bounds-checked access to container elements. at() throws out_of_-range if n >= a.size()."

Right. Different words. Same meaning. You cut out the quote from the standard that said: ""17.6.4.11 Requires paragraph [res.on.required] Violation of the preconditions specified in a function’s Requires: paragraph results in undefined behavior unless the function’s Throws: paragraph specifies throwing an exception when the precondition is violated."

Not fitting your mental model is not a bug in the standard...

Nevin, I think it's pointless to continue this discussion. I know your view, you know mine. kind regards -Thorsten

On 11-10-2013 16:29, Julian Gonggrijp wrote:

Thorsten Ottosen wrote:

...

""17.6.4.11 Requires paragraph [res.on.required]

Violation of the preconditions specified in a function’s Requires: paragraph results in undefined behavior unless the function’s Throws: paragraph specifies throwing an exception when the precondition is violated."

I think you are right about this: IF a function throws when its precondition is violated, then the resulting behaviour is defined. I previously replied with +1 to Nevin when he claimed the opposite (among other things); I admit not studying that bit carefully enough.

No need to apologize. The literature on this subject is full of imprecise, ambugious and non-formal definitions of the terms precondition and postcondition. My own work is no exception. The C++ standard is no exception. Perhaps the spec # guys have some of the best definitions in their papers (I would have to reread them to be sure). At least they distinguish between a normal postcondition and the condition that is guaranteed when the function exists (not returns) via an exception. Take one definition, and all other statements, from persons working with another definition, are close to meaningless. So which definition should one choose? It's up to each person to decide for themselves. I find it /practical/ and /useful/ to state that the following two programs have a bug, and that the bug is a violation of a precondition in both cases: double computeSum( const vector<double>& values ) { double res = 0.; for( size_t n = 0u; n < values.size(); ++n ) res += values[n+1]; return res; } double computeSum( const vector<double>& values ) { double res = 0.; for( size_t n = 0u; n < values.size(); ++n ) res += values.at(n+1); return res; } I don't see any benefit of saying that there is not a precondition violation in both cases, but I see many benefits of doing so. Hence I would prefer a formal definition that allows me to reason like that.

This is however tangential to the question whether a function *should* throw when its precondition is violated. Herb Sutter wrote a piece on error handling [1] which I think is insightful. Herb Sutter states, and I think I agree, that it is the responsibility of the caller to ensure that preconditions are met; hence it is also the responsibility of the caller to report an error when it cannot meet the preconditions of the callee (and it cannot avoid the call). Of course you can do an additional check inside the callee, but that check should be considered to be redundant and on violation the real error is in the caller, not in the callee.

I don't think anybody has ever disagreed with that. That's how it is. In a correct program, all preconditions can be removed without any change to the program's behavior.

Herb Sutter wrote:

...

The code that could cause an error is responsible for detecting and reporting the error. In particular, this means that the caller is responsible for detecting and reporting a to-be-called function's parameter precondition violations. If the caller fails to do so, it is a programming mistake; therefore, when the parameter precondition violation is detected inside the called function, it can be dealt with using an assertion.

I recommend reading all of Sutter's piece.

It's usually worth a read.

I think the discussion so far has concerned several points, the first three of which can now be closed as far as I can tell:

[snip]

* Should a function throw when its precondition is violated? ~ Going with Herb Sutter: no, but the caller should throw when it is about to violate it. (In the case of shared_ptr_non_null, the caller could be a factory function or a wrapper with the specific purpose of checking for null.) * Does checking for null add overhead in the constructor of shared_ptr_non_null? ~ Not much compared to memory allocation, but it is still redundant IF not passing null is a precondition (if you agree with Herb Sutter).

That's one view. It's totally redundant in a correct program. In a non-correct program we are then in the UB vs defined behavior situation. I talked with Herb in various WG21 C++ meetings (after he published that article I think) when we discussed contract programming. His oppinion at that time was that it was OK to throw when a precondition is violated. So I guess people change oppinions. I have done so over the years. Maybe Herb changed his oppinion back. My current oppinion is that if can avoid UB without any major overhead, we should do so. That implies throwing when it's relatively cheap to do so (using boost::throw_exception). It also implies that vector<T>::front() should not be throwing. If we were to add boost::throw_precondition that would be used for low-overhead preconditions (defaulting to assert), then at least I could compile our code-base with such checks. kind regards -Thorsten

On 8 October 2013 02:09, Rob Stewart <robertstewart@comcast.net> wrote:

On Oct 7, 2013, at 5:39 PM, Daniel James <daniel@calamity.org.uk> wrote:

...

On 6 October 2013 12:04, Rob Stewart <robertstewart@comcast.net> wrote:

...

If the exception is triggered by an unknown condition, such as you're advocating, then there can be no handler for it. It must, of necessity, unwind to the top. Code cannot rightly continue as if there had been no exception. In that case, there's little gained over an assertion, save for the possibility that other threads may be able to complete independent work.

later:

...

The likely outcome of failure in this regard, in a non-debug build, is a null pointer dereference elsewhere. Unchecked, that means a segfault. IOW, the problem will be found.

So you're saying that when it's an exception, it's triggered by an "unknown condition" not by a null, which is surely a known condition.

No

"No, there's an unknown condition", or "no, that's not what I'm saying"?

...

And you're claiming that after an exception the code will continue as if there was no exception.

No, Pete, I think it was, advocated letting thousands of tasks continue while a few failed.

How is that continuing as if there was no exception? There are strategies for recovering from such exceptions, a simple one is the write the code in question in a pure functional style.

If a null pointer occurs somewhere in your code and this class' ctor throws an exception, the handler has no idea of the source of the null pointer. Thus, the you have an unknown condition as far as the handler is concerned. (I had been discussing a handler in main() or a thread procedure, as opposed to one very near the object mis-construction.)

If you're doing individual tasks you know which one was associated with the throw. For many cases that's all you need to know. This does depend on the program, which is why the decision to catch logic errors should be up to the programmer, who knows it better than we do. You don't have to catch the exceptions if you don't want to.

OTOH, if a null is actual stored in the object, then it will eventually be dereferenced, in the general case anyway, and the segfault will indicate which instance was bad. From that you should be able to discover its genesis, at least if the object's address is consistent or the pointee's type is unique. That makes finding the source of the null pointer somewhat more tractable without a debugger.

You'd get better data if the failure was in the constructor. It's more likely that you could associate it with the source of the pointer, which might have been destructed by the time the pointer is dereferenced. (Remember that my main argument is that there should always be a check). And, of course, you don't always get the segfault data, you sometimes don't even get notified that there is a bug. You're making the assumption that after a bug is discovered, you'll be quickly notified and nothing bad will happen in the meantime. Unfortunately, that isn't true at all.

Consider the case of taking the address of a static object.

I hope you're using a do-nothing custom deleter.

There's no need to check for null, so there's no need to risk pipeline stalling on a failed branch prediction or cache invalidation due to the exception construction and throwing code you'd add to the ctor otherwise.

Well, there's premature optimization for you. This is a class that's concerned with safety, not efficiency. If there's a safe option and an unsafe option, then the safe option should be the default, and the unsafe option should be the verbose one. Considering your concerns, why are repeatedly creating shared pointers from a static object? This would have to be a very tight loop if a null check on a value that you have to access anyway is going to make a difference. In which case you might as well create one non-null shared pointer and reuse that, as it will be faster. But if you can't do that, what will this check really cost? The branch prediction should be for the non-null case, if you're concerned about that you can add a hint in some compilers. I'd actually hope that the compiler would optimise the check away in many cases, especially when initialising from a static object. And the exception creation code should be separate the normal code anyway, so it shouldn't stress the cache too much. Then you've got the cost of reserving memory for that shared pointer's count, that's expensive. You might try to avoid that by using enable_shared_from_this, but then you've got the weak pointer check. You're also no longer thread safe and have to have a shared_ptr somewhere, so there's little argument against using a static non-null shared pointer which will avoid both the weak pointer check and the null check, which is surely preferable since you're so concerned about performance. But whatever you do creating this shared pointer is going to require memory manipulation on the heap - that's not free either. But really, the only way to tell is to try it out in a real program.

On Oct 8, 2013, at 2:38 PM, Daniel James <daniel@calamity.org.uk> wrote:

On 8 October 2013 02:09, Rob Stewart <robertstewart@comcast.net> wrote:

...

On Oct 7, 2013, at 5:39 PM, Daniel James <daniel@calamity.org.uk> wrote:

...

On 6 October 2013 12:04, Rob Stewart <robertstewart@comcast.net> wrote:

...

If the exception is triggered by an unknown condition, such as you're advocating, then there can be no handler for it. It must, of necessity, unwind to the top. Code cannot rightly continue as if there had been no exception. In that case, there's little gained over an assertion, save for the possibility that other threads may be able to complete independent work.

later:

...

The likely outcome of failure in this regard, in a non-debug build, is a null pointer dereference elsewhere. Unchecked, that means a segfault. IOW, the problem will be found.

So you're saying that when it's an exception, it's triggered by an "unknown condition" not by a null, which is surely a known condition.

No

"No, there's an unknown condition", or "no, that's not what I'm saying"?

I was not saying that it wasn't a null pointer, because it obviously was. I was saying that the exception handler can have no idea what cause the null pointer that was clearly not expected. If a null was anticipated, the programmer would have tested for it. How, then, can the handler account for the problem to allow the program to continue safely?

...

...

And you're claiming that after an exception the code will continue as if there was no exception.

No, Pete, I think it was, advocated letting thousands of tasks continue while a few failed.

How is that continuing as if there was no exception? There are strategies for recovering from such exceptions, a simple one is the write the code in question in a pure functional style.

Since the handler cannot know the cause of the null pointer, which would have been addressed by a conditional otherwise, continuing after the handler is pretending that it never happened. Functional style programming and careful use of RAII are ways to ensure proper cleanup during unwinding, but they require the kind of careful programming that would have inserted a conditional to test for a null pointer.

...

If a null pointer occurs somewhere in your code and this class' ctor throws an exception, the handler has no idea of the source of the null pointer. Thus, the you have an unknown condition as far as the handler is concerned. (I had been discussing a handler in main() or a thread procedure, as opposed to one very near the object mis-construction.)

If you're doing individual tasks you know which one was associated with the throw. For many cases that's all you need to know. This does depend on the program, which is why the decision to catch logic errors should be up to the programmer, who knows it better than we do. You don't have to catch the exceptions if you don't want to.

There are certainly exceptions (pun intended) to most any rule, but a segfault and core dump, for example, are preferable to std::terminate() due to an unhandled exception.

...

OTOH, if a null is actual stored in the object, then it will eventually be dereferenced, in the general case anyway, and the segfault will indicate which instance was bad. From that you should be able to discover its genesis, at least if the object's address is consistent or the pointee's type is unique. That makes finding the source of the null pointer somewhat more tractable without a debugger.

You'd get better data if the failure was in the constructor. It's more likely that you could associate it with the source of the pointer, which might have been destructed by the time the pointer is dereferenced. (Remember that my main argument is that there should always be a check).

In an optimized build, with no preceding assertion, how does throwing an exception help identify the source, short of a stack trace containing exception as one poster mentioned he had?

And, of course, you don't always get the segfault data, you sometimes don't even get notified that there is a bug.

That is clearly a problem.

You're making the assumption that after a bug is discovered, you'll be quickly notified and nothing bad will happen in the meantime. Unfortunately, that isn't true at all.

We all make assumptions based upon the apps we build and how we interact with our users.

...

Consider the case of taking the address of a static object.

I hope you're using a do-nothing custom deleter.

...

There's no need to check for null, so there's no need to risk pipeline stalling on a failed branch prediction or cache invalidation due to the exception construction and throwing code you'd add to the ctor otherwise.

Well, there's premature optimization for you.

Library code can't afford to assume such things don't matter.

This is a class that's concerned with safety, not efficiency. If there's a safe option and an unsafe option, then the safe option should be the default, and the unsafe option should be the verbose one.

As I've said numerous times, factory functions could be created that throw and others that don't. The ctor shouldn't. You could add a no_throw_t overload of the ctor, I suppose.

Considering your concerns, why are repeatedly creating shared pointers from a static object? This would have to be a very tight loop if a null check on a value that you have to access anyway is going to make a difference. In which case you might as well create one non-null shared pointer and reuse that, as it will be faster.

I offered one example of how one can know that the check is unnecessary. I wasn't implying a loop repeatedly creating a pointer to one static object. I get the feeling you're being purposely difficult now. Perhaps there's an array of objects. Perhaps the objects are in a map. There are numerous sources of non-null pointers.

But if you can't do that, what will this check really cost? The branch prediction should be for the non-null case, if you're concerned about that you can add a hint in some compilers. I'd actually hope that the compiler would optimise the check away in many cases, especially when initialising from a static object.

Optimizations are possible, but they can be thwarted by many factors.

And the exception creation code should be separate the normal code anyway, so it shouldn't stress the cache too much.

If you say so. The typical design would put that code in the ctor body, not a function.

Then you've got the cost of reserving memory for that shared pointer's count, that's expensive. You might try to avoid that by using enable_shared_from_this, but then you've got the weak pointer check.

Are you saying the overhead of that would swamp the pipeline stall, if it occurs? You may be right, but the stall can still add a significant delay.

You're also no longer thread safe and have to have a shared_ptr somewhere, so there's little argument against using a static non-null shared pointer which will avoid both the weak pointer check and the null check, which is surely preferable since you're so concerned about performance.

I have no idea where that came from.

But whatever you do creating this shared pointer is going to require memory manipulation on the heap - that's not free either.

OK

But really, the only way to tell is to try it out in a real program.

Right ___ Rob (Sent from my portable computation engine)

Rob Stewart:

If you violate a precondition, and the function scribbles on memory, for example, what do you >know about the state of your system? Can you do anything safely? Everything you do from that >time may be wrong, despite your hopes to the contrary.

I think this is the absolute crux of it, for me. So often I hear "if XXX happens then all bets are off, there's nothing better than going to the command line, directly to the command line, do not pass go, do not collect $200." But my real-life experience is just not like that. What really happens in practice is that some under pressure programmer has forgotten the one code path that means p is NULL and so we can't construct a Widget. That's what I know about the state of my program, that I can't construct a Widget when I really want to. So I throw sufficiently far out of the program until it isn't necessary to have a Widget. And there's always some layer of the program (even it is the end of main) where that's the case. To give a concrete example, suppose I want to carry out 1,000,000 "simulations" of some sort that take all night to run. E.g. the simulations returns a number and the output of my program is some statistics about those numbers (e.g. the mean). The simulations are defined by some scenario description language that gets parsed. In 50 of the simulations, those definitions mean I go down a code path that has the bug. In the other 999,950 definitions, I get the right answer. If I assert, then I lose all information about all scenarios. If I throw, I get all the information I need about 999,950 scenarios, and an error log telling me that a program bug needs to be fixed before the other 50 can be evaluated. The latter is, in practice, much more useful. By "doctrine", I was trying to invoke some thing like "established best practice of the C++ community at large". In every other case I can think of, learning about an established best practice has improved the programs I contribute to. But the assert thing I still don't really believe and I've known about it for 10 years now, so I thought I'd mention it despite dragging things off topic - sorry for that. Pete

On Friday 04 October 2013 21:09:02 Daniel James wrote:

On 4 October 2013 17:45, Eric Niebler <eniebler@boost.org> wrote:

...

On 10/4/2013 9:20 AM, Matt Calabrese wrote:

...

but I definitely am against an exception for this type of programmer error.

This is the crux of it. If this condition really does represent a programmer error (and IMO in this case it does), then Matt is right. Throwing is wrong. Programmer error == bug == your program is already in some weird state. Continuing by throwing an exception and executing an arbitrary amount of code is not good.

I don't think this is always the case. For example, unexpected output from a parser should not be an indicator of an error in the global state.

I agree with Eric. If the unexpected output is the result of an error in the parser implementation then this is a bug in the parser and no exception will fix that. If this is the result of invalid input from the environment then it should be detected by the parser and indicated with an exception or other means. Again, if the parser doesn't do that, it is a bug in the parser. Bottom line: only throw exceptions as a result of abnormal behavior of the environment of the program, and not as a result of internal bugs.

...

Precondition violations ==> assertions. Use BOOST_ASSERT. That gives people a way to hook the behavior while giving a sane default.

If throwing an exception when this is null is a bad idea, then not checking in release mode is surely a terrible one.

I don't think so. If not checking makes it crash and burn then let it be that way. The sooner it crashes, the sooner you'll fix the bug. Exceptions, on the other hand, even if logged, tend to be ignored until it is apparent that something goes on wrong. This may be much much later than when the problem have occurred and makes debugging much harder.

On 03-10-2013 17:04, Matt Calabrese wrote:

anyway. It's still just undefined behavior. Asserting accomplishes this, allows your function to be noexcept,

The constructor can never be no-except here since the counts needs to be allocated. -Thorsten

On Wed, Oct 2, 2013 at 4:18 AM, Luke Bradford <lukebradford01@gmail.com>wrote:

Thanks for the feedback, Matt - some quick thoughts:

...

Not having a default constructor seems unnecessary to me. Why not just have the default constructor dynamically allocate a default constructed object? Similarly, for consistency with other smart pointers, having a conversion to bool would be useful and makes transition to the type simpler.

Often the object pointed to will not have a default constructor, so we wouldn't be able to construct one in shared_ptr_nonnull's default constructor.

This does not prevent your template from having a default constructor in the case that the type has a default constructor. I only consider this very useful because there are plenty of times where generic code takes advantage of default constructability. For example, resizing a container without an explicit value.

I also think that having a conversion to bool is misleading - in the use cases I've seen, it would lead to a lot of extraneous if( ptr ) statements, then essentially if( true ), which obscure the programmer's intentions and are easy to remove. I found the elimination of the default constructor and conversion to bool just as useful, if not more useful, than the runtime checks of the class, because they revealed places where we were default constructing or checking a shared_ptr that we expected to be valid at all times.

The point isn't for people to use them in new code written specifically for the type, it's both to transition from old code and to work fine with generic code. You can also think of it as how boost::variant has an "empty" function that just always returns false.

I like the implicit conversion for convenience in using the class in place of shared_ptr. Is there a reason you wouldn't want to have implicit conversion?

For all of the standard reasons that implicit conversion is to generally be avoided, which I won't spend time repeating here. -- -Matt Calabrese

On Wed, Oct 2, 2013 at 3:19 PM, Mostafa <mostafa_working_away@yahoo.com>wrote:

Maybe I'm just being thick, why can't you just cast it/wrap it as a shared_ptr obj before passing it to such generic code?

You're not being thick and it's not really a huge deal. Yes, you can always wrap it, though you once you do that and pass it off, you're losing some of the benefit of it being non-null and also doing a conversion. Another example of something similar -- why does std::array have a size function (I.E. not just an associated size value)? The size is always known at compile time (just like our non-null-ness is known at compile-time). The reason is so that it models the container concept, of course, and is usable in contexts that expect a container. It's also simply consistent with existing practice. In our case with the non-null pointer template, we don't have an explicit concept to model, but it doesn't hurt to be mindful of consistency. Pointers and smart pointer types generally support checking for null, and we lose nothing by respecting that convention. It also makes drop-in replacement easier while keeping the proper behavior. -- -Matt Calabrese

On 10/02/2013 01:40 PM, Andrey Semashev wrote:

On Wed, Oct 2, 2013 at 2:54 AM, Luke Bradford <lukebradford01@gmail.com> wrote:

...

Hi,

I'm new to this list, so apologies for any oversights or faux pas.

I've been finding a lot of use for a type of smart pointer I call shared_ptr_nonnull, which is a variation on shared_ptr which is never allowed to be empty. Specifically:

(1) It doesn't have reset() with no arguments, doesn't have a default constructor, and doesn't have implicit conversion to bool (which would always be true.) (2) It throws an exception whenever there's an attempt to make it empty, i.e. in constructors, reset, assignment, and swap.

For convenience, it's implicitly convertible to shared_ptr, and I have all of shared_ptr's templated operators implemented with possible combinations of shared_ptr and shared_ptr_nonnull. Usually it can be used just by changing the type of a shared_ptr.

We have a lot of shared_ptrs, especially member variables, which are claimed to be "always valid" and this class enforces that, at compile time (1) and runtime (2).

Has there been any discussion of something like this? Does anybody have any thoughts, suggestions, criticisms?

This is an interesting idea, but I am not sure how much I would need or use such a class template. How do you intend to break cycles? Do we new the weak_ptr_nonull variant as well, or are cycles impossible?

...

Who's in charge of the smart pointer library these days?

This may be useful, although I don't remember needing such a tool in my practice. Somehow it's usually enough for me to simply copy or move objects or have them implemented as shared pimpl using shared_ptr or intrusive_ptr internally. In the latter case the pointer initialization is very well isolated, so there is no problem with NULL pointers.

Anyway, I think, such a pointer does not behave like a real pointer, so it shouldn't be called as such. It looks more like a reference,

+1

except that the referred object is accessed through operator->.

Yes,

Maybe it should be called shared_ref because of it.

+1 too bad there is no overladable operator.() as alternative to operator->, but that would maybe break more generic code than desired. -- Bjørn

Andrey Semashev wrote:

Anyway, I think, such a pointer does not behave like a real pointer, so it shouldn't be called as such. It looks more like a reference, except that the referred object is accessed through operator->. Maybe it should be called shared_ref because of it. Thinking about it this way, there is no need for reset() or constructors from pointers - only assignment, emplace() and forwarding constructors. Interoperability with shared_ptr can be provided as an extension (maybe with free functions), but that would be just an extension, i.e. no implicit conversion.

I respectfully disagree. A reference is much more restricted than a smart pointer that cannot be null: it cannot change reference and it never owns the object it references. In summary, it mostly has the semantics of a second name to a previously created object. Smart pointers that cannot be null have exactly the same range of semantics and syntactical appearance as other pointers, except for the guarantee of not being null. -Julian

On 02-10-2013 13:40, Andrey Semashev wrote:

On Wed, Oct 2, 2013 at 2:54 AM, Luke Bradford <lukebradford01@gmail.com> wrote:

...

Hi,

I'm new to this list, so apologies for any oversights or faux pas.

I've been finding a lot of use for a type of smart pointer I call shared_ptr_nonnull, which is a variation on shared_ptr which is never allowed to be empty. Specifically:

Anyway, I think, such a pointer does not behave like a real pointer, so it shouldn't be called as such. It looks more like a reference, except that the referred object is accessed through operator->. Maybe it should be called shared_ref because of it.

I'm inclined to think that template< class T > class shared_obj; with use like shared_obj<Foo> sharedFoo = make_shared_obj( 42, "foo" ); would be good names. -Thorsten

On 03-10-2013 19:04, Andrey Semashev wrote:

On Thursday 03 October 2013 18:37:45 Thorsten Ottosen wrote:

...

with use like

shared_obj<Foo> sharedFoo = make_shared_obj( 42, "foo" );

would be good names.

I agree, this is a good alternative. Although why not immediately this:

shared_obj<Foo> sharedFoo( 42, "foo" );

Seems good. As long as it implements the "allocate counts and object in one allocation". -Thorsten

On Thu, Oct 3, 2013 at 9:37 AM, Thorsten Ottosen < thorsten.ottosen@dezide.com> wrote:

shared_obj<Foo> sharedFoo = make_shared_obj( 42, "foo" );

would be good names.

The only issue with this is your type may not be allocated with new and you may want a custom deleter (you may also want to take control from a separately allocated object). These are things that shared_ptr can handle, but with a strict interface such as this, the non-null ptr cannot. That said, I personally don't care, but if the intent is to just have a non-null shared_ptr then we shouldn't be sacrificing existing functionality. It should simply be a shared_ptr with a non-null invariant and that is that. -- -Matt Calabrese

On Friday 04 October 2013 15:39:23 Thorsten Ottosen wrote:

On 03-10-2013 21:55, Matt Calabrese wrote:

...

On Thu, Oct 3, 2013 at 9:37 AM, Thorsten Ottosen <

thorsten.ottosen@dezide.com> wrote:

...

shared_obj<Foo> sharedFoo = make_shared_obj( 42, "foo" );

would be good names.

The only issue with this is your type may not be allocated with new and you may want a custom deleter (you may also want to take control from a separately allocated object). These are things that shared_ptr can handle, but with a strict interface such as this, the non-null ptr cannot. That said, I personally don't care, but if the intent is to just have a non-null shared_ptr then we shouldn't be sacrificing existing functionality. It should simply be a shared_ptr with a non-null invariant and that is that.

The deleter thing can probably be handled with a consrtuctor like

template< class D, class Args ... > shared_obj( D, Args args... );

For stuff not allocated via new, it can be probably be handled by another version of make_shared_obj.

I'd suggest both cases to be handled by special factory functions. Otherwise, it is ambiguous whether the first constructor argument is a deleter or an argument to the object constructor.

On Oct 4, 2013, at 1:38 PM, Matt Calabrese <rivorus@gmail.com> wrote:

Almost all of these cases can be handled by named constructor-like functions in a somewhat elegant manner, with the exception perhaps of the last one (what happens if the factory function can return a null pointer? We're back to the original issue, in that case, only worse because the programmer cannot simply handle that case outside of the constructor). I think ultimately my vote would be to support named functions, but there still needs to be a way to do construction via a direct pointer otherwise we are losing functionality that shared_ptr provides.

The factory/named constructor functions can throw, or not, as the case warrants. They can be viewed as conveniences for using the class. The constructor could be public and assert on null pointer, while the factories can offer other policies.

I'll repeat my basic, underlying stance here for people to agree/disagree with. A non-null shared_ptr should be exactly that. It should have the same or extremely similar interface to a shared_ptr (including things like bool-conversion), just with more strict invariants and preconditions. This makes drop-in replacement and use in generic functions easier to deal with and also avoids sacrificing additional functionality that shared_ptr provides. People shouldn't have to avoid switching to a non-null shared pointer simply because they have no way of constructing it with a pointer that they obtained from a factory.

+1 ___ Rob (Sent from my portable computation engine)

On 10/7/2013 10:48 PM, Quoth Thorsten Ottosen:

If the idea is to be 100% shared_ptr interface compatible, it may be easier just to extend boost::shared_ptr a little:

typedef boost::shared_ptr<boost::non_nullable<T>> SharedT;

I don't like that. If you're suggesting using a wrapper type without changing shared_ptr, that won't work because the shared_ptr itself could still be empty, so you haven't gained anything. If you're suggesting specialising shared_ptr for that subtype, I don't see any benefit in doing this over defining a new pointer type, since you have to redefine everything in a specialisation anyway. And there's undoubtedly some existing templated code that operates on boost::shared_ptr<T> that would be confused by this, or at the very least not operate efficiently by including tests for null.

typedef boost::shared_ptr<boost::non_nullable<T>> WeakT;

I assume that was a typo. (Weak pointers are not especially useful without being able to represent null anyway.)

On 08-10-2013 14:00, Thorsten Ottosen wrote:

The shared_ptr implementation would have to check its template argument and emit special code in a few places. This is very easy to do, and far easier than specializing/wrapping the class.

Another benefit of this approach is that we can create different behaviors without relying on the absurd subtleties of the preprocessor: typedef boost::shared_ptr<T> SharedT; // normal shared_ptr typedef boost::shared_ptr<assert_non_null<T>> NonNullSharedT; // no null enforced by assertions typedef boost::shared_ptr<enforce_non_null<T>> NeverNullSharedT; // no null enfoced by exceptions regards -Thorsten

On 09-10-2013 00:44, Gavin Lambert wrote:

On 10/9/2013 3:26 AM, Quoth Thorsten Ottosen:

...

I suppose there is still a need to break cycles. Why would this have changed.

But part of the semantics of weak_ptr is that it can point to a non-existent (expired) object, which is effectively the same as a null one. And lock() must be able to return an empty pointer, so it would have to be a regular shared_ptr<T> rather than a shared_ptr<*_non_null<T>>. So I'm not sure if a weak_ptr<*_non_null<T>> could make sense.

Or lock() could return something else. Anyway, it could be that just reusing the normal weak ptr would be good. -Thorsten

To Andrey's point:

Anyway, I think, such a pointer does not behave like a real pointer, so it shouldn't be called as such. It looks more like a reference, except that the referred object is accessed through operator->.

Well, shared_ptr_nonnull does behave like a ref counted object in many respects - with the notable exception that it can point to different objects over the course of its lifetime. I find this to be a useful property and a good reason to call it a pointer. And I find the interchangeability with shared_ptr very useful as well. Julian, I'm curious to see your proposed pointer too. On Wed, Oct 2, 2013 at 8:03 AM, Julian Gonggrijp <j.gonggrijp@gmail.com>wrote:

Luke Bradford wrote:

...

I've been finding a lot of use for a type of smart pointer I call shared_ptr_nonnull, which is a variation on shared_ptr which is never allowed to be empty. Specifically:

(1) It doesn't have reset() with no arguments, doesn't have a default constructor, and doesn't have implicit conversion to bool (which would always be true.) [...]

Has there been any discussion of something like this? Does anybody have any thoughts, suggestions, criticisms? Who's in charge of the smart pointer library these days?

I think it makes a lot of sense to have a pointer that can't be null and doesn't have a default constructor. Default construction isn't always possible. Otherwise it's against the intuition that a default constructor shouldn't allocate memory. Explicit initialization has inherent value as well, especially for pointers.

Coincidentally I've recently been working on a different kind of smart pointer which has these same properties (no reset, no default ctor, no implicit conversion to bool). Even more coincidentally I might gauge interest today. Please don't let this distract you though; my smart pointer is quite different in other respects.

-Julian

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

On Wed, Oct 2, 2013 at 6:01 AM, Thorsten Ottosen < thorsten.ottosen@dezide.com> wrote:

a) swap would always work because both arguments must be non-null.

From the way he was talking about inter-operability with shared_ptr, I

think he's referring to a swap with a shared_ptr. b) this smart pointer cannot be moved (no move-assignment, no

move-constructor).

It can be moved. A copy is just a move that happens to not alter the state of the original object. Nothing about move operations requires that the moved-from object is in some different state (I.E. C++03 types with just copy constructors are still movable types). In other words, don't explicitly delete the move function. -- -Matt Calabrese