[cfe-commits] r83217 - in /cfe/trunk: lib/Sema/SemaOverload.cpp test/CodeGenCXX/PR5086-ambig-resolution-enum.cpp

[Fariborz Jahanian]

On Oct 5, 2009, at 4:21 PM, Douglas Gregor wrote:

>
> On Oct 1, 2009, at 3:10 PM, Fariborz Jahanian wrote:
>
>> Author: fjahanian
>> Date: Thu Oct  1 17:10:15 2009
>> New Revision: 83217
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=83217&view=rev
>> Log:
>> enumerator value of 0 is not a null pointer constant for
>> deciding const of null pointer conversion. Fixes PR5086.
>>
>> Added:
>>   cfe/trunk/test/CodeGenCXX/PR5086-ambig-resolution-enum.cpp
>> Modified:
>>   cfe/trunk/lib/Sema/SemaOverload.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=83217&r1=83216&r2=83217&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaOverload.cpp Thu Oct  1 17:10:15 2009
>> @@ -887,6 +887,9 @@
>>      Expr->getType()->isIntegralType())
>>    return !InOverloadResolution;
>>
>> +  if (Expr->getType()->isEnumeralType())
>> +    return !InOverloadResolution;
>> +
>>  return Expr->isNullPointerConstant(Context,
>>                    InOverloadResolution?  
>> Expr::NPC_ValueDependentIsNotNull
>>                                        :  
>> Expr::NPC_ValueDependentIsNull);
>
> I don't think this is the right fix. We should change  
> Expr::isNullPointerConstant to meet the C++ rules when in C++ mode,  
> not change overload resolution itself.
>
>> Added: cfe/trunk/test/CodeGenCXX/PR5086-ambig-resolution-enum.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/PR5086-ambig-resolution-enum.cpp?rev=83217&view=auto
>>
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- cfe/trunk/test/CodeGenCXX/PR5086-ambig-resolution-enum.cpp  
>> (added)
>> +++ cfe/trunk/test/CodeGenCXX/PR5086-ambig-resolution-enum.cpp Thu  
>> Oct  1 17:10:15 2009
>> @@ -0,0 +1,21 @@
>> +// RUN: clang-cc -triple x86_64-apple-darwin -std=c++0x -S %s -o  
>> %t-64.s &&
>> +// RUN: FileCheck -check-prefix LP64 --input-file=%t-64.s %s &&
>> +// RUN: clang-cc -triple i386-apple-darwin -std=c++0x -S %s -o  
>> %t-32.s &&
>> +// RUN: FileCheck -check-prefix LP32 --input-file=%t-32.s %s &&
>> +// RUN: true
>> +
>> +class UnicodeString {
>> +public:
>> +        enum EInvariant { kInvariant };
>> +        int extract(int targetCapacity, enum EInvariant inv) const;
>> +        int extract(unsigned targetLength, const char *codepage)  
>> const;
>> +};
>> +
>> +void foo(const UnicodeString& id) {
>> +        enum {BUFLEN = 128 };
>> +        id.extract(BUFLEN - 2, UnicodeString::kInvariant);
>> +}
>> +
>> +// CHECK-LP64: call      
>> __ZNK13UnicodeString7extractEiNS_10EInvariantE
>> +
>> +// CHECK-LP32: call      
>> L__ZNK13UnicodeString7extractEiNS_10EInvariantE
>
> Could we turn this into a Sema-based check, so that we don't have to  
> go through code generation to perform the test? You can give the two  
> "extract" functions different return types (int* vs. float*), and  
> then initialize a variable with the return type. Also, we should  
> sanitize the names of UnicodeString, extract, etc.
>

Fixed in http://llvm.org/viewvc/llvm-project?view=rev&revision=83350
Thanks for the review.

-  Fariborz

> 	- Doug