On Feb 5, 2014, at 1:15 PM, Justin Bogner <[email protected]> wrote:

> Marshall Clow <[email protected]> writes:
>> Again, we have to treat NULL specially- who knew? ;-)
>> More tests, too.
>> 
>> http://llvm.org/bugs/show_bug.cgi?id=17221
>> 
>> Thanks to Howard for the suggested fix.
> 
> I'll defer to Howard on the actual change - I just have a couple of stylistic 
> nits:

thanks for taking a look.

> 
>> Index: src/private_typeinfo.cpp
>> ===================================================================
>> --- src/private_typeinfo.cpp (revision 200864)
>> +++ src/private_typeinfo.cpp (working copy)
>> @@ -301,17 +301,20 @@
>>                                                     void* adjustedPtr,
>>                                                     int path_below) const
>> {
>> -    ptrdiff_t offset_to_base = __offset_flags >> __offset_shift;
>> -    if (__offset_flags & __virtual_mask)
>> +    ptrdiff_t offset_to_base = 0;
>> +    if (adjustedPtr != nullptr)
>>     {
>> -        const char* vtable = *static_cast<const char*const*>(adjustedPtr);
>> -        offset_to_base = *reinterpret_cast<const ptrdiff_t*>(vtable + 
>> offset_to_base);
>> +        offset_to_base = __offset_flags >> __offset_shift;
>> +        if (__offset_flags & __virtual_mask)
>> +        {
>> +            const char* vtable = *static_cast<const 
>> char*const*>(adjustedPtr);
>> +            offset_to_base = *reinterpret_cast<const ptrdiff_t*>(vtable + 
>> offset_to_base);
> 
> 80 column?

libc++ has not traditionally followed the “80 column” guideline.
If you look at the rest of that file, you’ll see what I mean.

> 
>> +        }
>>     }
>> -    __base_type->has_unambiguous_public_base(info,
>> -                                             
>> static_cast<char*>(adjustedPtr) + offset_to_base,
>> -                                             (__offset_flags & 
>> __public_mask) ?
>> -                                                 path_below :
>> -                                                 not_public_path);
>> +    __base_type->has_unambiguous_public_base(
>> +            info,
>> +            static_cast<char*>(adjustedPtr) + offset_to_base,
>> +            (__offset_flags & __public_mask) ?path_below :not_public_path);
> 
> The conditional operator should have spaces on both sides of ? and :,
> like so:
> 
>    (__offset_flags & __public_mask) ? path_below : not_public_path);

Fixed.

>> }
>> 
>> void
>> @@ -358,7 +361,8 @@
>>                                void*& adjustedPtr) const
>> {
>>     // Do the dereference adjustment
>> -    adjustedPtr = *static_cast<void**>(adjustedPtr);
>> +    if ( adjustedPtr != NULL )
> 
> No spaces inside the if's parens

Fixed.


-- Marshall

Marshall Clow     Idio Software   <mailto:[email protected]>

A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly 
moderated down to (-1, Flamebait).
        -- Yu Suzuki


_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to