[cfe-commits] r111116 - in /cfe/trunk: include/clang/Checker/PathSensitive/ConstraintManager.h include/clang/Checker/PathSensitive/GRState.h lib/Checker/FlatStore.cpp lib/Checker/RegionStore.cpp lib/Checker/SimpleConstraintManager.cpp lib/Checker/SimpleConstraintManager.h lib/Checker/Store.cpp test/Analysis/outofbound.c

[Jordy Rose]

On Mon, 16 Aug 2010 09:38:35 -0700, Ted Kremenek <kremenek at apple.com>
wrote:
> On Aug 16, 2010, at 9:23 AM, Douglas Gregor wrote:
> 
>> 
>> On Aug 16, 2010, at 9:06 AM, Ted Kremenek wrote:
>> 
>>> 
>>> On Aug 15, 2010, at 6:15 PM, Jordy Rose wrote:
>>> 
>>>> +
>>>> +  operator bool() const {
>>>> +    return getRegion() != NULL;
>>>> +  }
>>>> };
>>> 
>>> Hi Jordy,
>>> 
>>> I'm really mixed about this.  operator bool() is convenient, but in my
>>> experience it doesn't lead to better readability in the code.  Why not
>>> just have clients call 'getRegion()', where the check becomes
explicit? 
>>> Using operator bool() also compounds the often abused use of bool. 
For
>>> me, bool is 'true' and 'false', not "is valid" or "is not valid".  If
>>> someone doesn't know the correct interpretation of operator bool() in
a
>>> specific context, it is easy to mess things up.
>> 
>> In general, conversion to bool is useful when the class may be used in
>> the pattern
>> 
>> 	if (C c = maybeGetAC()) {
>> 	}
>> 
>> I have no idea whether this pattern will be used with this class.
> 
> That's fair, and this is possibly one of those cases.  That's why I had
> mixed feelings. :)

Good point. Still, the way the code is right now, it isn't one of those
cases (and isn't likely to become one), so I'll go ahead and change it to
IsValid.