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

[Ted Kremenek]

On Feb 15, 2008, at 4:28 PM, Chris Lattner 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.

I agree that Ander's patch doesn't change the ownership or structure,  
and that it is definitely a step in the right direction.  As our needs  
for the frontend evolve clearly the support for attributes can adapt  
as necessary.

Anders: Thanks so much for doing this.