Not all of these attributes use strings based on StringExprs, but for those that do that's a good idea.
On Feb 10, 2010, at 9:38 PM, Chris Lattner wrote: > > On Feb 10, 2010, at 9:28 PM, Ted Kremenek wrote: > >> Author: kremenek >> Date: Wed Feb 10 23:28:37 2010 >> New Revision: 95853 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=95853&view=rev >> Log: >> Remove use of 'std::string' from Attr objects, using instead a byte >> array allocated using the allocator in ASTContext. This addresses >> these strings getting leaked when using a BumpPtrAllocator (in >> ASTContext). >> >> Fixes: <rdar://problem/7636765> > > Would it work to just make these have StringExpr*'s? The memory is already > allocated by the parsing/sema code, do we really need to copy it? > > -Chris > >> >> Modified: >> cfe/trunk/include/clang/AST/Attr.h >> cfe/trunk/lib/AST/AttrImpl.cpp >> cfe/trunk/lib/AST/Decl.cpp >> cfe/trunk/lib/CodeGen/CodeGenModule.cpp >> cfe/trunk/lib/Frontend/PCHReaderDecl.cpp >> cfe/trunk/lib/Sema/SemaDecl.cpp >> cfe/trunk/lib/Sema/SemaDeclAttr.cpp >> >> Modified: cfe/trunk/include/clang/AST/Attr.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Attr.h?rev=95853&r1=95852&r2=95853&view=diff >> >> ============================================================================== >> --- cfe/trunk/include/clang/AST/Attr.h (original) >> +++ cfe/trunk/include/clang/AST/Attr.h Wed Feb 10 23:28:37 2010 >> @@ -18,7 +18,6 @@ >> #include "llvm/ADT/StringRef.h" >> #include <cassert> >> #include <cstring> >> -#include <string> >> #include <algorithm> >> using llvm::dyn_cast; >> >> @@ -120,8 +119,7 @@ >> assert(Next == 0 && "Destroy didn't work"); >> } >> public: >> - >> - void Destroy(ASTContext &C); >> + virtual void Destroy(ASTContext &C); >> >> /// \brief Whether this attribute should be merged to new >> /// declarations. >> @@ -157,6 +155,18 @@ >> // Implement isa/cast/dyncast/etc. >> static bool classof(const Attr *) { return true; } >> }; >> + >> +class AttrWithString : public Attr { >> +private: >> + const char *Str; >> + unsigned StrLen; >> +protected: >> + AttrWithString(Attr::Kind AK, ASTContext &C, llvm::StringRef s); >> + llvm::StringRef getString() const { return llvm::StringRef(Str, StrLen); } >> + void ReplaceString(ASTContext &C, llvm::StringRef newS); >> +public: >> + virtual void Destroy(ASTContext &C); >> +}; >> >> #define DEF_SIMPLE_ATTR(ATTR) \ >> class ATTR##Attr : public Attr { \ >> @@ -214,12 +224,12 @@ >> static bool classof(const AlignedAttr *A) { return true; } >> }; >> >> -class AnnotateAttr : public Attr { >> - std::string Annotation; >> +class AnnotateAttr : public AttrWithString { >> public: >> - AnnotateAttr(llvm::StringRef ann) : Attr(Annotate), Annotation(ann) {} >> + AnnotateAttr(ASTContext &C, llvm::StringRef ann) >> + : AttrWithString(Annotate, C, ann) {} >> >> - const std::string& getAnnotation() const { return Annotation; } >> + llvm::StringRef getAnnotation() const { return getString(); } >> >> virtual Attr* clone(ASTContext &C) const; >> >> @@ -230,12 +240,12 @@ >> static bool classof(const AnnotateAttr *A) { return true; } >> }; >> >> -class AsmLabelAttr : public Attr { >> - std::string Label; >> +class AsmLabelAttr : public AttrWithString { >> public: >> - AsmLabelAttr(llvm::StringRef L) : Attr(AsmLabel), Label(L) {} >> + AsmLabelAttr(ASTContext &C, llvm::StringRef L) >> + : AttrWithString(AsmLabel, C, L) {} >> >> - const std::string& getLabel() const { return Label; } >> + llvm::StringRef getLabel() const { return getString(); } >> >> virtual Attr* clone(ASTContext &C) const; >> >> @@ -248,12 +258,12 @@ >> >> DEF_SIMPLE_ATTR(AlwaysInline); >> >> -class AliasAttr : public Attr { >> - std::string Aliasee; >> +class AliasAttr : public AttrWithString { >> public: >> - AliasAttr(llvm::StringRef aliasee) : Attr(Alias), Aliasee(aliasee) {} >> + AliasAttr(ASTContext &C, llvm::StringRef aliasee) >> + : AttrWithString(Alias, C, aliasee) {} >> >> - const std::string& getAliasee() const { return Aliasee; } >> + llvm::StringRef getAliasee() const { return getString(); } >> >> virtual Attr *clone(ASTContext &C) const; >> >> @@ -322,12 +332,12 @@ >> DEF_SIMPLE_ATTR(Deprecated); >> DEF_SIMPLE_ATTR(Final); >> >> -class SectionAttr : public Attr { >> - std::string Name; >> +class SectionAttr : public AttrWithString { >> public: >> - SectionAttr(llvm::StringRef N) : Attr(Section), Name(N) {} >> + SectionAttr(ASTContext &C, llvm::StringRef N) >> + : AttrWithString(Section, C, N) {} >> >> - const std::string& getName() const { return Name; } >> + llvm::StringRef getName() const { return getString(); } >> >> virtual Attr *clone(ASTContext &C) const; >> >> @@ -380,15 +390,14 @@ >> static bool classof(const NonNullAttr *A) { return true; } >> }; >> >> -class FormatAttr : public Attr { >> - std::string Type; >> +class FormatAttr : public AttrWithString { >> int formatIdx, firstArg; >> public: >> - FormatAttr(llvm::StringRef type, int idx, int first) : Attr(Format), >> - Type(type), formatIdx(idx), firstArg(first) {} >> + FormatAttr(ASTContext &C, llvm::StringRef type, int idx, int first) >> + : AttrWithString(Format, C, type), formatIdx(idx), firstArg(first) {} >> >> - const std::string& getType() const { return Type; } >> - void setType(llvm::StringRef type) { Type = type; } >> + llvm::StringRef getType() const { return getString(); } >> + void setType(ASTContext &C, llvm::StringRef type); >> int getFormatIdx() const { return formatIdx; } >> int getFirstArg() const { return firstArg; } >> >> >> Modified: cfe/trunk/lib/AST/AttrImpl.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/AttrImpl.cpp?rev=95853&r1=95852&r2=95853&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/AST/AttrImpl.cpp (original) >> +++ cfe/trunk/lib/AST/AttrImpl.cpp Wed Feb 10 23:28:37 2010 >> @@ -11,11 +11,45 @@ >> // >> //===----------------------------------------------------------------------===// >> >> - >> #include "clang/AST/Attr.h" >> #include "clang/AST/ASTContext.h" >> using namespace clang; >> >> +void Attr::Destroy(ASTContext &C) { >> + if (Next) { >> + Next->Destroy(C); >> + Next = 0; >> + } >> + this->~Attr(); >> + C.Deallocate((void*)this); >> +} >> + >> +AttrWithString::AttrWithString(Attr::Kind AK, ASTContext &C, >> llvm::StringRef s) >> + : Attr(AK) { >> + assert(!s.empty()); >> + StrLen = s.size(); >> + Str = new (C) char[StrLen]; >> + memcpy(const_cast<char*>(Str), s.data(), StrLen); >> +} >> + >> +void AttrWithString::Destroy(ASTContext &C) { >> + C.Deallocate(const_cast<char*>(Str)); >> + Attr::Destroy(C); >> +} >> + >> +void AttrWithString::ReplaceString(ASTContext &C, llvm::StringRef newS) { >> + if (newS.size() > StrLen) { >> + C.Deallocate(const_cast<char*>(Str)); >> + Str = new char[newS.size()]; >> + } >> + StrLen = newS.size(); >> + memcpy(const_cast<char*>(Str), newS.data(), StrLen); >> +} >> + >> +void FormatAttr::setType(ASTContext &C, llvm::StringRef type) { >> + ReplaceString(C, type); >> +} >> + >> #define DEF_SIMPLE_ATTR_CLONE(ATTR) \ >> Attr *ATTR##Attr::clone(ASTContext &C) const { \ >> return ::new (C) ATTR##Attr; \ >> @@ -66,15 +100,15 @@ >> } >> >> Attr* AnnotateAttr::clone(ASTContext &C) const { >> - return ::new (C) AnnotateAttr(Annotation); >> + return ::new (C) AnnotateAttr(C, getAnnotation()); >> } >> >> Attr *AsmLabelAttr::clone(ASTContext &C) const { >> - return ::new (C) AsmLabelAttr(Label); >> + return ::new (C) AsmLabelAttr(C, getLabel()); >> } >> >> Attr *AliasAttr::clone(ASTContext &C) const { >> - return ::new (C) AliasAttr(Aliasee); >> + return ::new (C) AliasAttr(C, getAliasee()); >> } >> >> Attr *ConstructorAttr::clone(ASTContext &C) const { >> @@ -94,7 +128,7 @@ >> } >> >> Attr *SectionAttr::clone(ASTContext &C) const { >> - return ::new (C) SectionAttr(Name); >> + return ::new (C) SectionAttr(C, getName()); >> } >> >> Attr *NonNullAttr::clone(ASTContext &C) const { >> @@ -102,7 +136,7 @@ >> } >> >> Attr *FormatAttr::clone(ASTContext &C) const { >> - return ::new (C) FormatAttr(Type, formatIdx, firstArg); >> + return ::new (C) FormatAttr(C, getType(), formatIdx, firstArg); >> } >> >> Attr *FormatArgAttr::clone(ASTContext &C) const { >> >> Modified: cfe/trunk/lib/AST/Decl.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=95853&r1=95852&r2=95853&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/AST/Decl.cpp (original) >> +++ cfe/trunk/lib/AST/Decl.cpp Wed Feb 10 23:28:37 2010 >> @@ -29,15 +29,6 @@ >> >> using namespace clang; >> >> -void Attr::Destroy(ASTContext &C) { >> - if (Next) { >> - Next->Destroy(C); >> - Next = 0; >> - } >> - this->~Attr(); >> - C.Deallocate((void*)this); >> -} >> - >> /// \brief Return the TypeLoc wrapper for the type source info. >> TypeLoc TypeSourceInfo::getTypeLoc() const { >> return TypeLoc(Ty, (void*)(this + 1)); >> >> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=95853&r1=95852&r2=95853&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original) >> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Wed Feb 10 23:28:37 2010 >> @@ -1283,8 +1283,8 @@ >> const llvm::Type *DeclTy = getTypes().ConvertTypeForMem(D->getType()); >> >> // Unique the name through the identifier table. >> - const char *AliaseeName = AA->getAliasee().c_str(); >> - AliaseeName = getContext().Idents.get(AliaseeName).getNameStart(); >> + const char *AliaseeName = >> + getContext().Idents.get(AA->getAliasee()).getNameStart(); >> >> // Create a reference to the named value. This ensures that it is emitted >> // if a deferred decl. >> >> Modified: cfe/trunk/lib/Frontend/PCHReaderDecl.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/PCHReaderDecl.cpp?rev=95853&r1=95852&r2=95853&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Frontend/PCHReaderDecl.cpp (original) >> +++ cfe/trunk/lib/Frontend/PCHReaderDecl.cpp Wed Feb 10 23:28:37 2010 >> @@ -446,7 +446,7 @@ >> >> #define STRING_ATTR(Name) \ >> case Attr::Name: \ >> - New = ::new (*Context) Name##Attr(ReadString(Record, Idx)); \ >> + New = ::new (*Context) Name##Attr(*Context, ReadString(Record, Idx)); \ >> break >> >> #define UNSIGNED_ATTR(Name) \ >> @@ -497,7 +497,7 @@ >> std::string Type = ReadString(Record, Idx); >> unsigned FormatIdx = Record[Idx++]; >> unsigned FirstArg = Record[Idx++]; >> - New = ::new (*Context) FormatAttr(Type, FormatIdx, FirstArg); >> + New = ::new (*Context) FormatAttr(*Context, Type, FormatIdx, >> FirstArg); >> break; >> } >> >> >> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=95853&r1=95852&r2=95853&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Feb 10 23:28:37 2010 >> @@ -2387,7 +2387,7 @@ >> if (Expr *E = (Expr*) D.getAsmLabel()) { >> // The parser guarantees this is a string. >> StringLiteral *SE = cast<StringLiteral>(E); >> - NewVD->addAttr(::new (Context) AsmLabelAttr(SE->getString())); >> + NewVD->addAttr(::new (Context) AsmLabelAttr(Context, SE->getString())); >> } >> >> // Don't consider existing declarations that are in a different >> @@ -2910,7 +2910,7 @@ >> if (Expr *E = (Expr*) D.getAsmLabel()) { >> // The parser guarantees this is a string. >> StringLiteral *SE = cast<StringLiteral>(E); >> - NewFD->addAttr(::new (Context) AsmLabelAttr(SE->getString())); >> + NewFD->addAttr(::new (Context) AsmLabelAttr(Context, SE->getString())); >> } >> >> // Copy the parameter declarations from the declarator D to the function >> @@ -4315,8 +4315,8 @@ >> bool HasVAListArg; >> if (Context.BuiltinInfo.isPrintfLike(BuiltinID, FormatIdx, HasVAListArg)) >> { >> if (!FD->getAttr<FormatAttr>()) >> - FD->addAttr(::new (Context) FormatAttr("printf", FormatIdx + 1, >> - HasVAListArg ? 0 : FormatIdx + >> 2)); >> + FD->addAttr(::new (Context) FormatAttr(Context, "printf", >> FormatIdx+1, >> + HasVAListArg ? 0 : >> FormatIdx+2)); >> } >> >> // Mark const if we don't care about errno and that is the only >> @@ -4353,15 +4353,15 @@ >> // FIXME: NSLog and NSLogv should be target specific >> if (const FormatAttr *Format = FD->getAttr<FormatAttr>()) { >> // FIXME: We known better than our headers. >> - const_cast<FormatAttr *>(Format)->setType("printf"); >> + const_cast<FormatAttr *>(Format)->setType(Context, "printf"); >> } else >> - FD->addAttr(::new (Context) FormatAttr("printf", 1, >> + FD->addAttr(::new (Context) FormatAttr(Context, "printf", 1, >> Name->isStr("NSLogv") ? 0 : 2)); >> } else if (Name->isStr("asprintf") || Name->isStr("vasprintf")) { >> // FIXME: asprintf and vasprintf aren't C99 functions. Should they be >> // target-specific builtins, perhaps? >> if (!FD->getAttr<FormatAttr>()) >> - FD->addAttr(::new (Context) FormatAttr("printf", 2, >> + FD->addAttr(::new (Context) FormatAttr(Context, "printf", 2, >> Name->isStr("vasprintf") ? 0 : >> 3)); >> } >> } >> >> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=95853&r1=95852&r2=95853&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Feb 10 23:28:37 2010 >> @@ -329,7 +329,7 @@ >> >> // FIXME: check if target symbol exists in current file >> >> - d->addAttr(::new (S.Context) AliasAttr(Str->getString())); >> + d->addAttr(::new (S.Context) AliasAttr(S.Context, Str->getString())); >> } >> >> static void HandleAlwaysInlineAttr(Decl *d, const AttributeList &Attr, >> @@ -942,7 +942,7 @@ >> return; >> } >> >> - D->addAttr(::new (S.Context) SectionAttr(SE->getString())); >> + D->addAttr(::new (S.Context) SectionAttr(S.Context, SE->getString())); >> } >> >> >> @@ -1257,7 +1257,7 @@ >> return; >> } >> >> - d->addAttr(::new (S.Context) FormatAttr(Format, Idx.getZExtValue(), >> + d->addAttr(::new (S.Context) FormatAttr(S.Context, Format, >> Idx.getZExtValue(), >> FirstArg.getZExtValue())); >> } >> >> @@ -1343,7 +1343,7 @@ >> S.Diag(ArgExpr->getLocStart(), diag::err_attribute_not_string) >> <<"annotate"; >> return; >> } >> - d->addAttr(::new (S.Context) AnnotateAttr(SE->getString())); >> + d->addAttr(::new (S.Context) AnnotateAttr(S.Context, SE->getString())); >> } >> >> static void HandleAlignedAttr(Decl *d, const AttributeList &Attr, Sema &S) { >> @@ -1924,7 +1924,7 @@ >> if (W.getAlias()) { // clone decl, impersonate __attribute(weak,alias(...)) >> IdentifierInfo *NDId = ND->getIdentifier(); >> NamedDecl *NewD = DeclClonePragmaWeak(ND, W.getAlias()); >> - NewD->addAttr(::new (Context) AliasAttr(NDId->getName())); >> + NewD->addAttr(::new (Context) AliasAttr(Context, NDId->getName())); >> NewD->addAttr(::new (Context) WeakAttr()); >> WeakTopLevelDecl.push_back(NewD); >> // FIXME: "hideous" code from Sema::LazilyCreateBuiltin >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
