[cfe-dev] Mistake in BuiltinOperatorOverloadBuilder::getUsualArithmeticConversions in SemaOverload.cpp

[Petr Kudryavtsev via cfe-dev]

You are welcome:)

Maybe you also interested in the following:
In file PartialDiagnostic.h, field "PartialDiagnostic::Allocator" 
sometimes is initialized with ~((uintptr_t)0) (looks like as a marker 
that value in "DiagStorage" is owned by someone outside). But later, it 
seems that in several methods that value handled improperly:
1) In method "getStorage" assertion should be moved inside "if 
(Allocator)" branch, not "else" branch.
2) In "freeStorageSlow" branch "else if (Allocator != 
reinterpret_cast<StorageAllocator *>(~uintptr_t(0)))" is never executed 
when "Allocator" is actually has that special value, so it probably 
should be rearranged to be first and changed a bit:
Now it is:
     ...
     if (Allocator)
       Allocator->Deallocate(DiagStorage);
     else if (Allocator != reinterpret_cast<StorageAllocator 
*>(~uintptr_t(0)))
       delete DiagStorage;
     ...
But should be:
     ...
     if (Allocator == reinterpret_cast<StorageAllocator *>(~uintptr_t(0))) {
       // do nothing
     } else if (Allocator) {
       Allocator->Deallocate(DiagStorage);
     } else {
       delete DiagStorage;
     }

Apparently, code was written under assumption that 
"reinterpret_cast<StorageAllocator *>(~uintptr_t(0))" evaluates to 0 (or 
false) but it doesn't seem right.

Thanks,
Petr

On 08.06.2017 21:27, George Burgess IV wrote:
> Happy Thursday,
>
> As it turns out, we never seem to use the result of that function; we
> just shove it into a struct field and ignore it. :) r304996 was landed
> to clean this up.
>
> Thank you!
> George
>
> On Wed, Jun 7, 2017 at 9:50 AM, Petr Kudryavtsev via cfe-dev
> <cfe-dev at lists.llvm.org> wrote:
>> Hello,
>>
>> Looks like the static table for arithmetic conversions inside
>> "getUsualArithmeticConversions" is out of date.
>>
>> It is declared as 12x12 table but is initialized as 11x11 table. It seems
>> that the Float128 type is not handled in it, but it should be.
>>
>> For example, in method "addGenericBinaryArithmeticOverloads" we iterate over
>> all promoted arithmetic types and Float128 is among them. It leads to access
>> to not initialized elements of the table and also, since the Float128 type
>> is 4th in method "getArithmeticType", all returned conversions for types
>> with index more than or equal to 4 may be wrong.
>>
>> Thanks,
>> Petr Kudriavtsev
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev