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

[Ted Kremenek]

Put another way, the attributes *belong* to a translation unit, and  
mixing and mashing attributes from different translation units into  
the same table causes lots of problems with managing memory,  
serializing the ASTs, etc.

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.
>
> Ted
>
> On Feb 15, 2008, at 3:30 PM, Anders Carlsson wrote:
>
>> Author: andersca
>> Date: Fri Feb 15 17:30:50 2008
>> New Revision: 47191
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=47191&view=rev
>> Log:
>> Store attributes in a global hash map instead.
>>
>> Modified:
>>   cfe/trunk/AST/Decl.cpp
>>   cfe/trunk/include/clang/AST/Attr.h
>>   cfe/trunk/include/clang/AST/Decl.h
>>
>> Modified: cfe/trunk/AST/Decl.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/AST/Decl.cpp?rev=47191&r1=47190&r2=47191&view=diff
>>
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- cfe/trunk/AST/Decl.cpp (original)
>> +++ cfe/trunk/AST/Decl.cpp Fri Feb 15 17:30:50 2008
>> @@ -15,6 +15,8 @@
>> #include "clang/AST/DeclObjC.h"
>> #include "clang/AST/Attr.h"
>> #include "clang/Basic/IdentifierTable.h"
>> +#include "llvm/ADT/DenseMap.h"
>> +
>> using namespace clang;
>>
>> // temporary statistics gathering
>> @@ -43,6 +45,12 @@
>>
>> static bool StatSwitch = false;
>>
>> +// This keeps track of all decl attributes. Since so few decls  
>> have attrs, we
>> +// keep them in a hash map instead of wasting space in the Decl  
>> class.
>> +typedef llvm::DenseMap<const Decl*, Attr*> DeclAttrMapTy;
>> +
>> +static DeclAttrMapTy *DeclAttrs = 0;
>> +
>> const char *Decl::getDeclKindName() const {
>>  switch (DeclKind) {
>>  default: assert(0 && "Unknown decl kind!");
>> @@ -251,6 +259,39 @@
>>
>> // Out-of-line virtual method providing a home for Decl.
>> Decl::~Decl() {
>> +  if (!DeclAttrs)
>> +    return;
>> +
>> +  DeclAttrMapTy::iterator it = DeclAttrs->find(this);
>> +  if (it != DeclAttrs->end()) {
>> +    delete it->second;
>> +    DeclAttrs->erase(it);
>> +    if (DeclAttrs->empty()) {
>> +      delete DeclAttrs;
>> +      DeclAttrs = 0;
>> +    }
>> +  }
>> +}
>> +
>> +void Decl::addAttr(Attr *newattr)
>> +{
>> +  if (!DeclAttrs)
>> +    DeclAttrs = new llvm::DenseMap<const Decl*, Attr*>;
>> +
>> +  Attr *&attr = DeclAttrs->FindAndConstruct(this).second;
>> +
>> +  newattr->setNext(attr);
>> +  attr = newattr;
>> +
>> +  HasAttrs = true;
>> +}
>> +
>> +const Attr *Decl::getAttrs() const
>> +{
>> +  if (!HasAttrs || !DeclAttrs)
>> +    return 0;
>> +
>> +  return DeclAttrs->find(this)->second;
>> }
>>
>> const char *NamedDecl::getName() const {
>> @@ -259,12 +300,6 @@
>>  return "";
>> }
>>
>> -void ValueDecl::addAttr(Attr *attr)
>> -{
>> -  attr->setNext(Attrs);
>> -  Attrs = attr;
>> -}
>> -
>> FunctionDecl::~FunctionDecl() {
>>  delete[] ParamInfo;
>> }
>>
>> Modified: cfe/trunk/include/clang/AST/Attr.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Attr.h?rev=47191&r1=47190&r2=47191&view=diff
>>
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- cfe/trunk/include/clang/AST/Attr.h (original)
>> +++ cfe/trunk/include/clang/AST/Attr.h Fri Feb 15 17:30:50 2008
>> @@ -30,11 +30,11 @@
>>
>> protected:
>>  Attr(Kind AK) : Next(0), AttrKind(AK) {}
>> +public:
>>  virtual ~Attr() {
>>    delete Next;
>>  }
>> -
>> -public:
>> +
>>  Kind getKind() const { return AttrKind; }
>>
>>  Attr *getNext() { return Next; }
>>
>> Modified: cfe/trunk/include/clang/AST/Decl.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=47191&r1=47190&r2=47191&view=diff
>>
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- cfe/trunk/include/clang/AST/Decl.h (original)
>> +++ cfe/trunk/include/clang/AST/Decl.h Fri Feb 15 17:30:50 2008
>> @@ -118,8 +118,11 @@
>>  /// InvalidDecl - This indicates a semantic error occurred.
>>  unsigned int InvalidDecl :  1;
>>
>> +  /// HasAttrs - This indicates whether the decl has attributes or  
>> not.
>> +  unsigned int HasAttrs : 1;
>> protected:
>> -  Decl(Kind DK, SourceLocation L) : Loc(L), DeclKind(DK),  
>> InvalidDecl(0) {
>> +  Decl(Kind DK, SourceLocation L) : Loc(L), DeclKind(DK),  
>> InvalidDecl(0),
>> +    HasAttrs(false) {
>>    if (Decl::CollectingStats()) addDeclKind(DK);
>>  }
>>
>> @@ -132,6 +135,9 @@
>>  Kind getKind() const { return DeclKind; }
>>  const char *getDeclKindName() const;
>>
>> +  void addAttr(Attr *attr);
>> +  const Attr *getAttrs() const;
>> +
>>  /// setInvalidDecl - Indicates the Decl had a semantic error. This
>>  /// allows for graceful error recovery.
>>  void setInvalidDecl() { InvalidDecl = 1; }
>> @@ -253,9 +259,6 @@
>> class ValueDecl : public ScopedDecl {
>>  QualType DeclType;
>>
>> -  /// Attrs - Linked list of attributes that are attached to this
>> -  /// function.
>> -  Attr *Attrs;
>> protected:
>>  ValueDecl(Kind DK, SourceLocation L, IdentifierInfo *Id, QualType T,
>>            ScopedDecl *PrevDecl)
>> @@ -265,9 +268,6 @@
>>  void setType(QualType newType) { DeclType = newType; }
>>  QualType getCanonicalType() const { return  
>> DeclType.getCanonicalType(); }
>>
>> -  void addAttr(Attr *attr);
>> -  const Attr *getAttrs() const { return Attrs; }
>> -
>>  // Implement isa/cast/dyncast/etc.
>>  static bool classof(const Decl *D) {
>>    return D->getKind() >= ValueFirst && D->getKind() <= ValueLast;
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>