[cfe-dev] RFC: Name lookup overhaul

[Chris Lattner]

On Dec 12, 2008, at 2:00 PM, Douglas Gregor wrote:
>>> "enum" could be "enumerator" or "enumeration". It's not a big  
>>> deal---it's pretty obvious from the context that we're talking  
>>> about enumerators here---but I'm not a fan of ambiguous shorthand.
>>
>> Shorthand can be ok as long as it is consistent.  How about enum  
>> for enumerator and enumcst for enumerator constant?  Am I missing  
>> something?
>
> enumerator == enumerator constant, but the latter isn't a standard  
> term. If we were being picky, EnumDecl should be EnumerationDecl (it  
> declares an enumeration type) and EnumConstantDecl should be  
> EnumeratorDecl (it declares an enumerator, e.g., one of the values  
> in the enumeration type). That's why I don't like "enum"... it's  
> shorthand for both of the standard terms.

Ok, I agree.  If you prefer the longer name, I'm completely fine with  
that.

>>> field_iterator has a bit of logic to skip over declarations that  
>>> aren't fields. We could certainly have an iterator adaptor that  
>>> effectively performs a cast<T> on the underlying iterator.
>>
>> This can be handled by making the template take a functor that  
>> decides whether an entry should be returned or skipped.  The  
>> default functor (used by enums) would always return "don't skip".
>
> The proper generic factoring is to have an iterator adaptor that  
> does the cast<T> (a "transform" iterator, in Boost parlance) and  
> another that does the skipping based on a function object (a  
> "filter" iterator, in Boost parlance):
>
> 	http://www.boost.org/doc/libs/1_37_0/libs/iterator/doc/filter_iterator.html
> 	http://www.boost.org/doc/libs/1_37_0/libs/iterator/doc/transform_iterator.html
>
> Right now, we only have one iterator that does the skipping  
> (field_iterator), which also does a dyn_cast rather than a cast, so  
> it's not terribly high priority to factor that out. We could  
> certainly build a simple version of the transform iterator and use  
> that for the casting iterators.

Ok.  The risk of stacking iterators really deep like that is that many  
compilers really do suffer from abstraction penalty issues.

>>>> +  /// Entity - The entity with which this scope is associated. For
>>>> +  /// example, the entity of a class scope is the class itself,  
>>>> the
>>>> +  /// entity of a function scope is a function, etc. This field is
>>>> +  /// maintained by the Action implementation.
>>>> +  void *Entity;
>>>>
>>>> This is orthogonal to this patch, but I wonder if Scope::Entity  
>>>> could be used to simplify your recent "if(int x = ..)" patch.  If  
>>>> the entity for the if-scope was the if statement itself, you  
>>>> wouldn't need an "is defined in an if" bit on decl.
>>>
>>> We can detect the "if" part pretty easy, by looking up the scope  
>>> chain to the control-scope parent. It's knowing that we're in the  
>>> "else" block that required the extra bits... perhaps there's still  
>>> a better way. I'll think about it.
>>
>> Ahhh, good point.  One somewhat hackish solution would be to only  
>> set the entity when parsing the else block instead of setting it  
>> when the scope is formed.
>
> It feels so much cleaner to associate the entity with the "if" itself.

Sure, I agree.  I'm just saying that the entity would only be  
associated when the 'else' block is being parsed.  When the 'then'  
part of the if is being parsed, the scope would just have a null entity,

-Chris