[cfe-commits] r93271 - in /cfe/trunk: lib/Sema/Sema.h lib/Sema/SemaExpr.cpp test/SemaObjC/ivar-lookup-resolution-builtin.m

[Fariborz Jahanian]

On Jan 12, 2010, at 4:42 PM, Douglas Gregor wrote:

>
> On Jan 12, 2010, at 3:58 PM, Fariborz Jahanian wrote:
>
>> Author: fjahanian
>> Date: Tue Jan 12 17:58:59 2010
>> New Revision: 93271
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=93271&view=rev
>> Log:
>> When in objective-c methods, do the built-in name lookup after
>> ivar name lookup. Fixes pr5986.
>>
>> Added:
>>   cfe/trunk/test/SemaObjC/ivar-lookup-resolution-builtin.m
>> 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=93271&r1=93270&r2=93271&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- cfe/trunk/lib/Sema/Sema.h (original)
>> +++ cfe/trunk/lib/Sema/Sema.h Tue Jan 12 17:58:59 2010
>> @@ -1481,7 +1481,8 @@
>>
>>  OwningExprResult LookupInObjCMethod(LookupResult &R,
>>                                      Scope *S,
>> -                                      IdentifierInfo *II);
>> +                                      IdentifierInfo *II,
>> +                                      bool  
>> AllowBuiltinCreation=false);
>>
>>  OwningExprResult ActOnDependentIdExpression(const CXXScopeSpec &SS,
>>                                              DeclarationName Name,
>>
>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=93271&r1=93270&r2=93271&view=diff
>>
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Jan 12 17:58:59 2010
>> @@ -1033,12 +1033,13 @@
>>    // Just re-use the lookup done by isTemplateName.
>>    DecomposeTemplateName(R, Id);
>>  } else {
>> -    LookupParsedName(R, S, &SS, true);
>> +    bool IvarLookupFollowUp = (!SS.isSet() && II &&  
>> getCurMethodDecl());
>> +    LookupParsedName(R, S, &SS, !IvarLookupFollowUp);
>>
>>    // If this reference is in an Objective-C method, then we need  
>> to do
>>    // some special Objective-C lookup, too.
>> -    if (!SS.isSet() && II && getCurMethodDecl()) {
>> -      OwningExprResult E(LookupInObjCMethod(R, S, II));
>> +    if (IvarLookupFollowUp) {
>> +      OwningExprResult E(LookupInObjCMethod(R, S, II, true));
>>      if (E.isInvalid())
>>        return ExprError();
>>
>> @@ -1218,7 +1219,8 @@
>> /// Returns a null sentinel to indicate trivial success.
>> Sema::OwningExprResult
>> Sema::LookupInObjCMethod(LookupResult &Lookup, Scope *S,
>> -                         IdentifierInfo *II) {
>> +                         IdentifierInfo *II,
>> +                         bool AllowBuiltinCreation) {
>>  SourceLocation Loc = Lookup.getNameLoc();
>>
>>  // There are two cases to handle here.  1) scoped lookup could  
>> have failed,
>> @@ -1299,7 +1301,18 @@
>>      T = Context.getObjCClassType();
>>    return Owned(new (Context) ObjCSuperExpr(Loc, T));
>>  }
>> -
>> +  if (Lookup.empty() && II && AllowBuiltinCreation) {
>> +    // FIXME. Consolidate this with similar code in LookupName.
>> +    if (unsigned BuiltinID = II->getBuiltinID()) {
>> +      if (!(getLangOptions().CPlusPlus &&
>> +             
>> Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID))) {
>> +        NamedDecl *D = LazilyCreateBuiltin((IdentifierInfo *)II,  
>> BuiltinID,
>> +                                           S,  
>> Lookup.isForRedeclaration(),
>> +                                           Lookup.getNameLoc());
>> +        if (D) Lookup.addDecl(D);
>> +      }
>> +    }
>> +  }
>>  // Sentinel value saying that we didn't do anything special.
>>  return Owned((Expr*) 0);
>> }
> C++ has a cleaner way of handling lookup for its data members, e.g.,
>
>  struct X {
>    int member;
>    int getMember() { return member; }
>  };
>
> C++ actually walks the Scope chain upward, and checks for each Scope  
> whether it has an Entity (which is a DeclContext). For entities that  
> are C++ member functions (like X::getMember()), it then performs  
> lookup into the class type (X), its base classes, and so on.
>
> Now, the C++ implementation is slower than just checking the  
> identifier chain, but it would make sense to take this slower path  
> when we know we're in an Objective-C method (getCurMethodDecl() !=  
> 0). That would unify the name-lookup logic for Objective-C and C++  
> somewhat, which is good for Objective-C++ and also would probably  
> address
>
> 	http://llvm.org/bugs/show_bug.cgi?id=5984

Agreed as a long term redo of objective-c's lookup. But above patch  
(for pr5986) doesn't depend on it. It is more for pr5984 as you  
pointed out.

- Fariborz


>
>
>  - Doug