2014-11-17 9:40 GMT+01:00 Matt Calabrese
On Sun, Nov 16, 2014 at 11:04 PM, Andrzej Krzemienski
wrote: Hi Everyone, I would like to run an idea through everyone in this list. There is a recurring complaint about Boost.Optional that it allows you to do "unsafe" things: 1. Inadvertent mixed comparisons between T and optional<T> 2. Unintended conversion from T to optional<T> 3. "Unchecked" access to the contained object, which causes an UB when performed on an uninitialized optional object.
There are valid reasons why optional is defined the way it is defined. But at the same time the complaints above are also valid. This makes some people abandon Boost.Optional and use their own alternative.
I don't immediately see the problem with the mixed comparisons between T and optional T... except maybe confusion regarding optional<bool> or bool-like types. Is this what you're referring to? If so, then I sort of disagree that it's a legitimate problem. If you are not talking about issues relating to bool or have some really compelling optional<bool> cases, can you point me to examples as to why this is suggested to be unsafe. I don't really care that much about the mixed comparisons, but I consider them pretty benign and if people find it useful then I won't remove it unless the current uses are fully considered.
As for #2, I think I agree that conversion should probably be more explicit and is potentially a step in a positive direction.
As for #3, if you are implying the alternative is to throw an exception, then I'd say that's a flat-out no. That's a logic error and we shouldn't throw an exception. The only thing that comes of that is people using the exception for basic control flow -- if they knew the optional didn't point to anything, then they wouldn't be dereferencing it. If they THOUGHT the optional pointed to something and it didn't, then they have a bug, which means the way to "handle" it is to fix the code. If they don't know at the time of the access whether there was something there or not, then they shouldn't deference it, using an exception for control flow. In that last case, they should either be explicitly branching or they should use some kind of visitation. If you're not talking about exceptions, then what exactly are you proposing?
No, no exceptions. The solution is based on the observation that there is a limited number of things you can do with optional<T> optional<int> oi = ...; 1: if (oi) doSomething(*oi); else doSomethingElse(); // or nothing 2: if (oi) doSomething(*oi); else doSomething(-1); // use default value 3: if (!oi) oi = (int)getIntByOtherMeans(); doSomething(*oi); // now it is safe Now we simply provide a dedicated interface for each of these three usages: 1: oi.use(&doSomething, &doSomethingElse); // or use lambdas 2: doSomething(oi.value_or(-1)); 3: doSomething(oi.value_or_eval(getIntByOtherMeans));
Regardless, if people come to an agreement on changes, I'm much more in favor of simply making breaking changes to the current optional rather than introducing another one. It's unfortunate for a type as fundamental and widely used as optional, but people will get over it, especially if it makes it closer to the standard proposal.
I would rather not go that way. boost::optional is not unsafe. It is simply something else than what some people think. Its conceptual model is more like any value of T, plus one additional value. In this model implicit conversions make perfect sense and are n fact desirable.