r202989 - [C++11] Using std::unique_ptr to ensure that Argument objects do not leak (since clang-tblgen isn't long-lived, the old leak is probably acceptable, but it offended my senses nonetheless).

Jordan Rose jordan_rose at apple.com
Wed Mar 5 09:19:03 PST 2014


IIRC TableGen has plenty of leaks, but I agree on the principle of the thing.

On Mar 5, 2014, at 8:49 , Aaron Ballman <aaron at aaronballman.com> wrote:

> Author: aaronballman
> Date: Wed Mar  5 10:49:55 2014
> New Revision: 202989
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=202989&view=rev
> Log:
> [C++11] Using std::unique_ptr to ensure that Argument objects do not leak (since clang-tblgen isn't long-lived, the old leak is probably acceptable, but it offended my senses nonetheless).
> 
> Modified:
>    cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
> 
> Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=202989&r1=202988&r2=202989&view=diff
> ==============================================================================
> --- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original)
> +++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Wed Mar  5 10:49:55 2014
> @@ -21,6 +21,7 @@
> #include "llvm/TableGen/TableGenBackend.h"
> #include <algorithm>
> #include <cctype>
> +#include <memory>
> #include <set>
> #include <sstream>
> 
> @@ -967,8 +968,8 @@ namespace {
>   };
> }
> 
> -static Argument *createArgument(Record &Arg, StringRef Attr,
> -                                Record *Search = 0) {
> +static std::unique_ptr<Argument> createArgument(Record &Arg, StringRef Attr,
> +                                                Record *Search = 0) {
>   if (!Search)
>     Search = &Arg;
> 
> @@ -1008,7 +1009,7 @@ static Argument *createArgument(Record &
>     // Search in reverse order so that the most-derived type is handled first.
>     std::vector<Record*> Bases = Search->getSuperClasses();
>     for (auto i = Bases.rbegin(), e = Bases.rend(); i != e; ++i) {
> -      Ptr = createArgument(Arg, Attr, *i);
> +      Ptr = createArgument(Arg, Attr, *i).release();
>       if (Ptr)
>         break;
>     }
> @@ -1017,7 +1018,7 @@ static Argument *createArgument(Record &
>   if (Ptr && Arg.getValueAsBit("Optional"))
>     Ptr->setOptional(true);
> 
> -  return Ptr;
> +  return std::unique_ptr<Argument>(Ptr);
> }
> 
> static void writeAvailabilityValue(raw_ostream &OS) {
> @@ -1052,8 +1053,10 @@ static void writeGetSpellingFunction(Rec
>   OS << "}\n\n";
> }
> 
> -static void writePrettyPrintFunction(Record &R, std::vector<Argument*> &Args,
> -                                     raw_ostream &OS) {
> +static void
> +writePrettyPrintFunction(Record &R,
> +                         const std::vector<std::unique_ptr<Argument>> &Args,
> +                         raw_ostream &OS) {
>   std::vector<FlattenedSpelling> Spellings = GetFlattenedSpellings(R);
> 
>   OS << "void " << R.getName() << "Attr::printPretty("
> @@ -1106,7 +1109,8 @@ static void writePrettyPrintFunction(Rec
>       "  case " << I << " : {\n"
>       "    OS << \"" + Prefix.str() + Spelling.str();
> 
> -    if (Args.size()) OS << "(";
> +    if (!Args.empty())
> +      OS << "(";
>     if (Spelling == "availability") {
>       writeAvailabilityValue(OS);
>     } else {
> @@ -1116,7 +1120,8 @@ static void writePrettyPrintFunction(Rec
>       }
>     }
> 
> -    if (Args.size()) OS << ")";
> +    if (!Args.empty())
> +      OS << ")";
>     OS << Suffix.str() + "\";\n";
> 
>     OS <<
> @@ -1379,15 +1384,12 @@ void EmitClangAttrClass(RecordKeeper &Re
>     OS << "class " << R.getName() << "Attr : public " << SuperName << " {\n";
> 
>     std::vector<Record*> ArgRecords = R.getValueAsListOfDefs("Args");
> -    std::vector<Argument*> Args;
> +    std::vector<std::unique_ptr<Argument>> Args;
>     Args.reserve(ArgRecords.size());
> 
>     for (auto ArgRecord : ArgRecords) {
> -      Argument *Arg = createArgument(*ArgRecord, R.getName());
> -      assert(Arg);
> -      Args.push_back(Arg);
> -
> -      Arg->writeDeclarations(OS);
> +      Args.emplace_back(createArgument(*ArgRecord, R.getName()));
> +      Args.back()->writeDeclarations(OS);
>       OS << "\n\n";
>     }
> 
> @@ -1411,7 +1413,7 @@ void EmitClangAttrClass(RecordKeeper &Re
>     OS << "ASTContext &Ctx";
>     if (!ElideSpelling)
>       OS << ", Spelling S";
> -    for (auto ai : Args) {
> +    for (auto const &ai : Args) {
>       OS << ", ";
>       ai->writeCtorParameters(OS);
>     }
> @@ -1419,7 +1421,7 @@ void EmitClangAttrClass(RecordKeeper &Re
>     OS << ") {\n";
>     OS << "    " << R.getName() << "Attr *A = new (Ctx) " << R.getName();
>     OS << "Attr(Loc, Ctx, ";
> -    for (auto ai : Args) {
> +    for (auto const &ai : Args) {
>       ai->writeImplicitCtorArgs(OS);
>       OS << ", ";
>     }
> @@ -1430,7 +1432,7 @@ void EmitClangAttrClass(RecordKeeper &Re
>     OS << "  " << R.getName() << "Attr(SourceRange R, ASTContext &Ctx\n";
> 
>     bool HasOpt = false;
> -    for (auto ai : Args) {
> +    for (auto const &ai : Args) {
>       OS << "              , ";
>       ai->writeCtorParameters(OS);
>       OS << "\n";
> @@ -1444,7 +1446,7 @@ void EmitClangAttrClass(RecordKeeper &Re
>     OS << "             )\n";
>     OS << "    : " << SuperName << "(attr::" << R.getName() << ", R, SI)\n";
> 
> -    for (auto ai : Args) {
> +    for (auto const &ai : Args) {
>       OS << "              , ";
>       ai->writeCtorInitializers(OS);
>       OS << "\n";
> @@ -1452,7 +1454,7 @@ void EmitClangAttrClass(RecordKeeper &Re
> 
>     OS << "  {\n";
> 
> -    for (auto ai : Args) {
> +    for (auto const &ai : Args) {
>       ai->writeCtorBody(OS);
>       OS << "\n";
>     }
> @@ -1462,7 +1464,7 @@ void EmitClangAttrClass(RecordKeeper &Re
>     // optional arguments as well.
>     if (HasOpt) {
>       OS << "  " << R.getName() << "Attr(SourceRange R, ASTContext &Ctx\n";
> -      for (auto ai : Args) {
> +      for (auto const &ai : Args) {
>         if (!ai->isOptional()) {
>           OS << "              , ";
>           ai->writeCtorParameters(OS);
> @@ -1476,7 +1478,7 @@ void EmitClangAttrClass(RecordKeeper &Re
>       OS << "             )\n";
>       OS << "    : " << SuperName << "(attr::" << R.getName() << ", R, SI)\n";
> 
> -      for (auto ai : Args) {
> +      for (auto const &ai : Args) {
>         OS << "              , ";
>         ai->writeCtorDefaultInitializers(OS);
>         OS << "\n";
> @@ -1484,7 +1486,7 @@ void EmitClangAttrClass(RecordKeeper &Re
> 
>       OS << "  {\n";
> 
> -      for (auto ai : Args) {
> +      for (auto const &ai : Args) {
>         if (!ai->isOptional()) {
>           ai->writeCtorBody(OS);
>           OS << "\n";
> @@ -1508,14 +1510,15 @@ void EmitClangAttrClass(RecordKeeper &Re
> 
>     writeAttrAccessorDefinition(R, OS);
> 
> -    for (auto ai : Args) {
> +    for (auto const &ai : Args) {
>       ai->writeAccessors(OS);
>       OS << "\n\n";
> 
>       if (ai->isEnumArg())
> -        static_cast<EnumArgument *>(ai)->writeConversion(OS);
> +        static_cast<const EnumArgument *>(ai.get())->writeConversion(OS);
>       else if (ai->isVariadicEnumArg())
> -        static_cast<VariadicEnumArgument *>(ai)->writeConversion(OS);
> +        static_cast<const VariadicEnumArgument *>(ai.get())
> +            ->writeConversion(OS);
>     }
> 
>     OS << R.getValueAsString("AdditionalMembers");
> @@ -1548,19 +1551,19 @@ void EmitClangAttrImpl(RecordKeeper &Rec
> 
>     if (!R.getValueAsBit("ASTNode"))
>       continue;
> -    
> +
>     std::vector<Record*> ArgRecords = R.getValueAsListOfDefs("Args");
> -    std::vector<Argument*> Args;
> +    std::vector<std::unique_ptr<Argument>> Args;
>     for (auto ri : ArgRecords)
> -      Args.push_back(createArgument(*ri, R.getName()));
> +      Args.emplace_back(createArgument(*ri, R.getName()));
> 
> -    for (auto ai : Args)
> +    for (auto const &ai : Args)
>       ai->writeAccessorDefinitions(OS);
> 
>     OS << R.getName() << "Attr *" << R.getName()
>        << "Attr::clone(ASTContext &C) const {\n";
>     OS << "  return new (C) " << R.getName() << "Attr(getLocation(), C";
> -    for (auto ai : Args) {
> +    for (auto const &ai : Args) {
>       OS << ", ";
>       ai->writeCloneArgs(OS);
>     }
> @@ -1651,7 +1654,7 @@ void EmitClangAttrPCHRead(RecordKeeper &
>   Record *InhClass = Records.getClass("InheritableAttr");
>   std::vector<Record*> Attrs = Records.getAllDerivedDefinitions("Attr"),
>                        ArgRecords;
> -  std::vector<Argument*> Args;
> +  std::vector<std::unique_ptr<Argument>> Args;
> 
>   OS << "  switch (Kind) {\n";
>   OS << "  default:\n";
> @@ -1670,12 +1673,11 @@ void EmitClangAttrPCHRead(RecordKeeper &
>     ArgRecords = R.getValueAsListOfDefs("Args");
>     Args.clear();
>     for (auto ai : ArgRecords) {
> -      Argument *A = createArgument(*ai, R.getName());
> -      Args.push_back(A);
> -      A->writePCHReadDecls(OS);
> +      Args.emplace_back(createArgument(*ai, R.getName()));
> +      Args.back()->writePCHReadDecls(OS);
>     }
>     OS << "    New = new (Context) " << R.getName() << "Attr(Range, Context";
> -    for (auto ri : Args) {
> +    for (auto const &ri : Args) {
>       OS << ", ";
>       ri->writePCHReadArgs(OS);
>     }
> @@ -1846,12 +1848,8 @@ void EmitClangAttrASTVisitor(RecordKeepe
>        << "    return false;\n";
> 
>     std::vector<Record*> ArgRecords = R.getValueAsListOfDefs("Args");
> -    for (auto ri : ArgRecords) {
> -      Record &ArgRecord = *ri;
> -      Argument *Arg = createArgument(ArgRecord, R.getName());
> -      assert(Arg);
> -      Arg->writeASTVisitorTraversal(OS);
> -    }
> +    for (auto ri : ArgRecords)
> +      createArgument(*ri, R.getName())->writeASTVisitorTraversal(OS);
> 
>     OS << "  return true;\n";
>     OS << "}\n\n";
> @@ -1921,20 +1919,17 @@ void EmitClangAttrTemplateInstantiate(Re
>     }
> 
>     std::vector<Record*> ArgRecords = R.getValueAsListOfDefs("Args");
> -    std::vector<Argument*> Args;
> +    std::vector<std::unique_ptr<Argument>> Args;
>     Args.reserve(ArgRecords.size());
> 
> -    for (auto ArgRecord : ArgRecords) {
> -      Argument *Arg = createArgument(*ArgRecord, R.getName());
> -      assert(Arg);
> -      Args.push_back(Arg);
> -    }
> +    for (auto ArgRecord : ArgRecords)
> +      Args.emplace_back(createArgument(*ArgRecord, R.getName()));
> 
> -    for (auto ai : Args) {
> +    for (auto const &ai : Args)
>       ai->writeTemplateInstantiation(OS);
> -    }
> +
>     OS << "      return new (C) " << R.getName() << "Attr(A->getLocation(), C";
> -    for (auto ai : Args) {
> +    for (auto const &ai : Args) {
>       OS << ", ";
>       ai->writeTemplateInstantiationArgs(OS);
>     }
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140305/5925d974/attachment.html>


More information about the cfe-commits mailing list