[PATCH] Replace OwningPtr::isValid() with conversion to bool.

David Blaikie dblaikie at gmail.com
Fri Mar 7 10:53:36 PST 2014


On Fri, Mar 7, 2014 at 10:38 AM, Arthur O'Dwyer
<arthur.j.odwyer at gmail.com> wrote:
> On Fri, Mar 7, 2014 at 8:14 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> ================
>> Comment at: include/clang/Frontend/ASTUnit.h:503
>> @@ -502,3 +502,3 @@
>>
>> -  bool hasSema() const { return TheSema.isValid(); }
>> +  bool hasSema() const { return bool(TheSema); }
>>    Sema &getSema() const {
>> ----------------
>> I /think/ we favor C++ style casts even in this context (static_cast<bool>(TheSema)), but if not, I'd at least prefer (bool)TheSema over function-style cast (though perhaps there's not precedent there either)
>
> Alternatively, instead of adding a terse `operator bool() const`,
> Ahmed could add a `bool operator!=(std::nullptr_t) const` so that this
> sort of comparison-to-null could be written as
>
>     bool hasSema() const { return (TheSema != nullptr); }
>
> IMHO this idiom is easier to read, and `std::unique_ptr` actually
> supports both idioms so it wouldn't impede moving to `std::unique_ptr`
> later.

Good point - I was searching for a more obvious explicit way to test a
std::unique_ptr and this one didn't occur to me... thanks for pointing
it out. Seems like the right way to write this.

(nit: I'd suggest omitting the () from the return expression ("return
TheSema != nullptr;") - but that's just me)



More information about the cfe-commits mailing list