Mistake in BuiltinOperatorOverloadBuilder::getUsualArithmeticConversions in SemaOverload.cpp

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

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. :slight_smile: r304996 was landed
to clean this up.

Thank you!
George

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