r237531 - Don't leak TemplateIds when a plugin parses late-parsed templates at TU end.

Nico Weber nicolasweber at gmx.de
Sat May 16 18:07:16 PDT 2015


Author: nico
Date: Sat May 16 20:07:16 2015
New Revision: 237531

URL: http://llvm.org/viewvc/llvm-project?rev=237531&view=rev
Log:
Don't leak TemplateIds when a plugin parses late-parsed templates at TU end.

In -fdelayed-template-parsing mode, templates that aren't used are not parsed
at all.  For some diagnostic plugins, this is a problem since they want to
analyse the contents of the template function body.  What has been suggested
on cfe-dev [1] is to explicitly parse interesting templates in
HandleTranslationUnit(); IWYU does this for example [2].

This is workable, but since the delayed parsing doesn't run below a call to
ParseTopLevelDecl(), no DestroyTemplateIdAnnotationsRAIIObj object is on the
stack to clean up TemplateIds that are created during parsing.  To fix this,
let ~Parser() clean them up in delayed template parsing mode instead of
leaking (or asserting in +Assert builds).

(r219810, relanded in r220400, fixed the same problem in incremental processing
mode; the review thread of r219810 has a good discussion of the problem.)

To test this, give the PrintFunctionNames plugin a flag to force parsing
of a template and add a test that uses it in -fdelayed-template-parsing mode.
Without the Parser.cpp change, that test asserts.

1: http://lists.cs.uiuc.edu/pipermail/cfe-dev/2014-August/038415.html
2: https://code.google.com/p/include-what-you-use/source/detail?r=566 

Added:
    cfe/trunk/test/Frontend/plugin-delayed-template.cpp
Modified:
    cfe/trunk/examples/PrintFunctionNames/PrintFunctionNames.cpp
    cfe/trunk/lib/Parse/Parser.cpp

Modified: cfe/trunk/examples/PrintFunctionNames/PrintFunctionNames.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/examples/PrintFunctionNames/PrintFunctionNames.cpp?rev=237531&r1=237530&r2=237531&view=diff
==============================================================================
--- cfe/trunk/examples/PrintFunctionNames/PrintFunctionNames.cpp (original)
+++ cfe/trunk/examples/PrintFunctionNames/PrintFunctionNames.cpp Sat May 16 20:07:16 2015
@@ -15,14 +15,23 @@
 #include "clang/Frontend/FrontendPluginRegistry.h"
 #include "clang/AST/AST.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Frontend/CompilerInstance.h"
+#include "clang/Sema/Sema.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace clang;
 
 namespace {
 
 class PrintFunctionsConsumer : public ASTConsumer {
+  CompilerInstance &Instance;
+  std::set<std::string> ParsedTemplates;
+
 public:
+  PrintFunctionsConsumer(CompilerInstance &Instance,
+                         std::set<std::string> ParsedTemplates)
+      : Instance(Instance), ParsedTemplates(ParsedTemplates) {}
+
   bool HandleTopLevelDecl(DeclGroupRef DG) override {
     for (DeclGroupRef::iterator i = DG.begin(), e = DG.end(); i != e; ++i) {
       const Decl *D = *i;
@@ -32,13 +41,47 @@ public:
 
     return true;
   }
+
+  void HandleTranslationUnit(ASTContext& context) override {
+    if (!Instance.getLangOpts().DelayedTemplateParsing)
+      return;
+
+    // This demonstrates how to force instantiation of some templates in
+    // -fdelayed-template-parsing mode. (Note: Doing this unconditionally for
+    // all templates is similar to not using -fdelayed-template-parsig in the
+    // first place.)
+    // The advantage of doing this in HandleTranslationUnit() is that all
+    // codegen (when using -add-plugin) is completely finished and this can't
+    // affect the compiler output.
+    struct Visitor : public RecursiveASTVisitor<Visitor> {
+      const std::set<std::string> &ParsedTemplates;
+      Visitor(const std::set<std::string> &ParsedTemplates)
+          : ParsedTemplates(ParsedTemplates) {}
+      bool VisitFunctionDecl(FunctionDecl *FD) {
+        if (FD->isLateTemplateParsed() &&
+            ParsedTemplates.count(FD->getNameAsString()))
+          LateParsedDecls.insert(FD);
+        return true;
+      }
+
+      std::set<FunctionDecl*> LateParsedDecls;
+    } v(ParsedTemplates);
+    v.TraverseDecl(context.getTranslationUnitDecl());
+    clang::Sema &sema = Instance.getSema();
+    for (const FunctionDecl *FD : v.LateParsedDecls) {
+      clang::LateParsedTemplate* LPT = sema.LateParsedTemplateMap.lookup(FD);
+      sema.LateTemplateParser(sema.OpaqueParser, *LPT);
+      llvm::errs() << "late-parsed-decl: \"" << FD->getNameAsString() << "\"\n";
+    }   
+  }
 };
 
 class PrintFunctionNamesAction : public PluginASTAction {
+  std::set<std::string> ParsedTemplates;
 protected:
   std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
                                                  llvm::StringRef) override {
-    return llvm::make_unique<PrintFunctionsConsumer>();
+    return llvm::make_unique<PrintFunctionsConsumer>(CI, ParsedTemplates);
   }
 
   bool ParseArgs(const CompilerInstance &CI,
@@ -47,12 +90,20 @@ protected:
       llvm::errs() << "PrintFunctionNames arg = " << args[i] << "\n";
 
       // Example error handling.
+      DiagnosticsEngine &D = CI.getDiagnostics();
       if (args[i] == "-an-error") {
-        DiagnosticsEngine &D = CI.getDiagnostics();
         unsigned DiagID = D.getCustomDiagID(DiagnosticsEngine::Error,
                                             "invalid argument '%0'");
         D.Report(DiagID) << args[i];
         return false;
+      } else if (args[i] == "-parse-template") {
+        if (i + 1 >= e) {
+          D.Report(D.getCustomDiagID(DiagnosticsEngine::Error,
+                                     "missing -parse-template argument"));
+          return false;
+        }
+        ++i;
+        ParsedTemplates.insert(args[i]);
       }
     }
     if (!args.empty() && args[0] == "help")

Modified: cfe/trunk/lib/Parse/Parser.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/Parser.cpp?rev=237531&r1=237530&r2=237531&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/Parser.cpp (original)
+++ cfe/trunk/lib/Parse/Parser.cpp Sat May 16 20:07:16 2015
@@ -38,6 +38,26 @@ public:
     return false;
   }
 };
+
+/// \brief RAIIObject to destroy the contents of a SmallVector of
+/// TemplateIdAnnotation pointers and clear the vector.
+class DestroyTemplateIdAnnotationsRAIIObj {
+  SmallVectorImpl<TemplateIdAnnotation *> &Container;
+
+public:
+  DestroyTemplateIdAnnotationsRAIIObj(
+      SmallVectorImpl<TemplateIdAnnotation *> &Container)
+      : Container(Container) {}
+
+  ~DestroyTemplateIdAnnotationsRAIIObj() {
+    for (SmallVectorImpl<TemplateIdAnnotation *>::iterator I =
+             Container.begin(),
+                                                           E = Container.end();
+         I != E; ++I)
+      (*I)->Destroy();
+    Container.clear();
+  }
+};
 } // end anonymous namespace
 
 IdentifierInfo *Parser::getSEHExceptKeyword() {
@@ -414,6 +434,15 @@ Parser::~Parser() {
 
   PP.clearCodeCompletionHandler();
 
+  if (getLangOpts().DelayedTemplateParsing &&
+      !PP.isIncrementalProcessingEnabled() && !TemplateIds.empty()) {
+    // If an ASTConsumer parsed delay-parsed templates in their
+    // HandleTranslationUnit() method, TemplateIds created there were not
+    // guarded by a DestroyTemplateIdAnnotationsRAIIObj object in
+    // ParseTopLevelDecl(). Destroy them here.
+    DestroyTemplateIdAnnotationsRAIIObj CleanupRAII(TemplateIds);
+  }
+
   assert(TemplateIds.empty() && "Still alive TemplateIdAnnotations around?");
 }
 
@@ -490,26 +519,6 @@ void Parser::Initialize() {
   ConsumeToken();
 }
 
-namespace {
-  /// \brief RAIIObject to destroy the contents of a SmallVector of
-  /// TemplateIdAnnotation pointers and clear the vector.
-  class DestroyTemplateIdAnnotationsRAIIObj {
-    SmallVectorImpl<TemplateIdAnnotation *> &Container;
-  public:
-    DestroyTemplateIdAnnotationsRAIIObj(SmallVectorImpl<TemplateIdAnnotation *>
-                                       &Container)
-      : Container(Container) {}
-
-    ~DestroyTemplateIdAnnotationsRAIIObj() {
-      for (SmallVectorImpl<TemplateIdAnnotation *>::iterator I =
-           Container.begin(), E = Container.end();
-           I != E; ++I)
-        (*I)->Destroy();
-      Container.clear();
-    }
-  };
-}
-
 void Parser::LateTemplateParserCleanupCallback(void *P) {
   // While this RAII helper doesn't bracket any actual work, the destructor will
   // clean up annotations that were created during ActOnEndOfTranslationUnit

Added: cfe/trunk/test/Frontend/plugin-delayed-template.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/plugin-delayed-template.cpp?rev=237531&view=auto
==============================================================================
--- cfe/trunk/test/Frontend/plugin-delayed-template.cpp (added)
+++ cfe/trunk/test/Frontend/plugin-delayed-template.cpp Sat May 16 20:07:16 2015
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fdelayed-template-parsing -load %llvmshlibdir/PrintFunctionNames%pluginext -plugin print-fns -plugin-arg-print-fns -parse-template -plugin-arg-print-fns ForcedTemplate %s 2>&1 | FileCheck %s
+// REQUIRES: plugins, examples
+
+template <typename T>
+void TemplateDep();
+
+// CHECK: top-level-decl: "ForcedTemplate"
+// The plugin should force parsing of this template, even though it's
+// not used and -fdelayed-template-parsing is specified.
+// CHECK: warning: expression result unused
+// CHECK: late-parsed-decl: "ForcedTemplate"
+template <typename T>
+void ForcedTemplate() {
+  TemplateDep<T>();  // Shouldn't crash.
+
+  "";  // Triggers the warning checked for above.
+}





More information about the cfe-commits mailing list