[cfe-commits] r47191 - in /cfe/trunk: AST/Decl.cpp include/clang/AST/Attr.h include/clang/AST/Decl.h

[Chris Lattner]

On Feb 15, 2008, at 4:11 PM, Ted Kremenek wrote:

>
> On Feb 15, 2008, at 3:56 PM, Chris Lattner wrote:
>
>>
>> On Feb 15, 2008, at 3:41 PM, Ted Kremenek wrote:
>>
>>> Hi Anders,
>>>
>>> I think such a map should go in ASTContext, rather than be a global
>>> variable.  It would be nice to invoke a parser and reclaim all
>>> associated state (including attributes) when we are done with a
>>> translation unit.  In general, we should avoid global variables of  
>>> any
>>> kind.
>>
>> In this case, the decls themselves own the attributes, just like  
>> they did before.  It's an implementation detail that the global  
>> exists, but the ownership is the same as before.
>
> That's true.  One problem with this approach is that it makes  
> iterating over all the attributes of a translation unit expensive  
> (requires full traversal of all ASTs).  I suppose this could be  
> perceived as an indexing issue, but since we are using an auxiliary  
> hash table to store attributes why not represent it on a per- 
> translation unit level?  For example, I can think of queries where  
> it would be useful to find all functions with a given attribute, etc.

Anders' patch just changes the representation of the information, it  
doesn't change ownership or structure.  Previously we couldn't  
efficiently iterate over all attributes either.  This could be an  
interesting extension, but I think Anders' patch is a fine step on its  
own.

-Chris