[cfe-commits] r77497 - in /cfe/trunk/lib/Sema: Sema.h SemaExpr.cpp

[Fariborz Jahanian]

On Jul 29, 2009, at 1:06 PM, Douglas Gregor wrote:

>
> On Jul 29, 2009, at 12:40 PM, Fariborz Jahanian wrote:
>
>> Author: fjahanian
>> Date: Wed Jul 29 14:40:11 2009
>> New Revision: 77497
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=77497&view=rev
>> Log:
>> Check accessibility when converting object to the base
>> class.
>
> Thanks! Some more comments below.
>
>> Modified:
>>   cfe/trunk/lib/Sema/Sema.h
>>   cfe/trunk/lib/Sema/SemaExpr.cpp
>>
>> Modified: cfe/trunk/lib/Sema/Sema.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=77497&r1=77496&r2=77497&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- cfe/trunk/lib/Sema/Sema.h (original)
>> +++ cfe/trunk/lib/Sema/Sema.h Wed Jul 29 14:40:11 2009
>> @@ -691,7 +691,7 @@
>>  ImplicitConversionSequence TryContextuallyConvertToBool(Expr *From);
>>  bool PerformContextuallyConvertToBool(Expr *&From);
>>
>> -  void PerformObjectMemberConversion(Expr *&From, NamedDecl  
>> *Member);
>> +  bool PerformObjectMemberConversion(Expr *&From, NamedDecl  
>> *Member);
>>
>>  /// OverloadingResult - Capture the result of performing overload
>>  /// resolution.
>>
>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=77497&r1=77496&r2=77497&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> = 
>> =====================================================================
>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Jul 29 14:40:11 2009
>> @@ -1024,7 +1024,7 @@
>>  return BuildDeclarationNameExpr(Loc, D, HasTrailingLParen, SS,  
>> isAddressOfOperand);
>> }
>> /// \brief Cast member's object to its own class if necessary.
>> -void
>> +bool
>> Sema::PerformObjectMemberConversion(Expr *&From, NamedDecl *Member) {
>>  if (FieldDecl *FD = dyn_cast<FieldDecl>(Member))
>>    if (CXXRecordDecl *RD =
>> @@ -1033,11 +1033,23 @@
>>        Context.getCanonicalType(Context.getTypeDeclType(RD));
>>      if (!DestType->isDependentType() &&
>>          !From->getType()->isDependentType()) {
>
> Sorry I didn't think of this before, but an early exit to reduce  
> nesting would make this clearer:
>
>  if (DestType->isDependentType() || From->isTypeDependent())
>    return false;
>
>> -        if (From->getType()->getAsPointerType())
>> +        QualType FromRecordType = From->getType();
>> +        QualType DestRecordType = DestType;
>> +        if (FromRecordType->getAsPointerType()) {
>>          DestType = Context.getPointerType(DestType);
>> +          FromRecordType = FromRecordType->getPointeeType();
>> +        }
>> +        if (IsDerivedFrom(FromRecordType, DestRecordType) &&
>> +            CheckDerivedToBaseConversion(FromRecordType,
>> +                                     DestRecordType,
>> +                                     From- 
>> >getSourceRange().getBegin(),
>> +                                     From->getSourceRange()))
>> +          return true;
>> +
>
> The IsDerivedFrom check can be expensive, so we'd like to avoid  
> doing it twice (once in IsDerivedFrom, once in  
> CheckDerivedToBaseConversion). Since the object conversion will  
> alway have either (canonical) FromRecordType == (canonical)  
> DestRecordType or FromRecordType derived from DestRecordType, why  
> not check for the equality condition and, if the types aren't equal,  
> just call CheckDerivedtoBaseConversion directly? It will assert that  
> there actually was a derivation relationship.
>
>>        ImpCastExprToType(From, DestType, /*isLvalue=*/true);
>
>
> Also: could you add a test case that illustrates the need for  
> CheckDerivedToBaseConversion?

All done in: http://llvm.org/viewvc/llvm-project?view=rev&revision=77502

- fariborz

>
>
> 	- Doug