[cfe-dev] Name lookup in ObjC methods

[Chris Lattner]

On Apr 2, 2008, at 12:57 PM, Argiris Kirtzidis wrote:

> Chris Lattner wrote:
>>> NamespaceDecl itself makes little sense to be a ScopedDecl, but it  
>>> makes sense to give it
>>> a context pointer to its enclosing namespace.
>>
>> Are you sure?  I thought that namespaces do impact unqualified  
>> scope lookups.  Perhaps not though.  If they don't, then I agree it  
>> should go on Decl.
> I see what you mean, a ScopedDecl is something that can "shadow"  
> another decl, right ? It makes perfect sense then.

Right, exactly.

>
>>> Most of the non-ScopedDecls are ObjC specific but for Objective C+ 
>>> + they will be able to use NamespaceDecl
>>> context pointers to, for example, get their fully qualified name.
>>
>> The various objc decls don't interact with C++ namespaces in ObjC+ 
>> +.  They are truly global, effectively always being installed at  
>> global scope.
> Oh, I really should refrain from making assertions about ObjC in the  
> future ;)

Hehe, I admit that I'm still learning ObjC as well.  Steve and  
Fariborz are certainly the gurus here :)

>>> And if RecordDecl is made a ContextDecl (mostly useful for C++), a  
>>> context pointer for FieldDecl would point at
>>> the struct/class that defined it.
>>>
>>> What do you think ?
>>
>> Ok, you convinced me, adding it to Decl makes sense :)

> I'm less convinced than before but at least for the "unifying  
> concept of context" as Ted said and for RecordDecls, maybe it's  
> worth it for now.

>
> Another idea would be:
> -Make Field a ScopedDecl too (at least it makes some sense for C++),  
> and use a ContextDecl member only to ScopedDecl.

I'm not sure about this.  Fields participate in name lookup, but  
aren't scoped in the "name hiding" sense.  In the "reference to  
instance variable" case, I think that the approach taken in  
Sema::ActOnIdentifierExpr for ObjC ivars is really the right  
approach.  With an unqualified name in either ObjC or C++ the lookup  
sequence basically should be:

1. Search "local" variables in the current function, according to  
their scope.
2. Search instance variables in the current class for non-static  
methods.
3. Search "global" variables outside the current function, according  
to their scope.
4. Search "implicitly global" identifiers like ObjC interfaces.
5. Search implicitly defined builtins (e.g. __builtin_cos)

The current implementation for ObjC handles #1,#3,#4 in LookupDecl.   
ActOnIdentifierExpr handles #2/#5, which is suboptimal, but I think  
it's the best thing.

Attempting to handle #2 in the scope lookup could be done by making  
all the fielddecls into scope decls, but I don't think this is a good  
way to go.  In addition to bloating all the fielddecls with this, it  
would require pushing all the ivars onto their scope chains when  
entering a method: this can be very expensive for classes with lots of  
ivars, and requires traversing (potentially deep) inheritance  
hierarchies.  Given this, I think having ActOnIdentifierExpr handle  
this is a good solution.

#5 is also best handled for the same reason as #2: this allows us to  
create builtin decls lazily, which is a big win on startup time and  
memory use.

> -Remove TranslationUnitDecl because otherwise, FunctionDecl would  
> return TranslationUnitDecl as its context and ObjCMethodDecl would
> return NULL (by ContextDecl::getParent()); a bit inconsistent.

That is fine with me.  TranslationUnitDecl has other uses, but I agree  
that this is somewhat strange. :)

> The convention would be that a ContextDecl chain ends at NULL,  
> instead of a TranslationUnitDecl.

Ok!

>> Would it be possible to move ContextDecl to a ContextDecl.h file  
>> that Decl.h #includes?
> "ContextDecl.h" would require the Decl class to be defined because  
> the ContextDecl class uses the Decl::Kind enum. Should I move the  
> Decl class to a "BaseDecl.h" or something ?

Sure, maybe DeclBase.h should have both Decl *and* ContextDecl?

-Chris