From: Nico Weber Date: Sun, 17 May 2015 01:07:16 +0000 (+0000) Subject: Don't leak TemplateIds when a plugin parses late-parsed templates at TU end. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=acb8cdf0f824ce159f956b057bab349a6eaa6e8f;p=clang 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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@237531 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/examples/PrintFunctionNames/PrintFunctionNames.cpp b/examples/PrintFunctionNames/PrintFunctionNames.cpp index 39443f597f..9f8f6e3f05 100644 --- a/examples/PrintFunctionNames/PrintFunctionNames.cpp +++ b/examples/PrintFunctionNames/PrintFunctionNames.cpp @@ -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 ParsedTemplates; + public: + PrintFunctionsConsumer(CompilerInstance &Instance, + std::set 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 { + const std::set &ParsedTemplates; + Visitor(const std::set &ParsedTemplates) + : ParsedTemplates(ParsedTemplates) {} + bool VisitFunctionDecl(FunctionDecl *FD) { + if (FD->isLateTemplateParsed() && + ParsedTemplates.count(FD->getNameAsString())) + LateParsedDecls.insert(FD); + return true; + } + + std::set 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 ParsedTemplates; protected: std::unique_ptr CreateASTConsumer(CompilerInstance &CI, llvm::StringRef) override { - return llvm::make_unique(); + return llvm::make_unique(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") diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp index a45eaa0e33..dea7a6998a 100644 --- a/lib/Parse/Parser.cpp +++ b/lib/Parse/Parser.cpp @@ -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 &Container; + +public: + DestroyTemplateIdAnnotationsRAIIObj( + SmallVectorImpl &Container) + : Container(Container) {} + + ~DestroyTemplateIdAnnotationsRAIIObj() { + for (SmallVectorImpl::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 &Container; - public: - DestroyTemplateIdAnnotationsRAIIObj(SmallVectorImpl - &Container) - : Container(Container) {} - - ~DestroyTemplateIdAnnotationsRAIIObj() { - for (SmallVectorImpl::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 diff --git a/test/Frontend/plugin-delayed-template.cpp b/test/Frontend/plugin-delayed-template.cpp new file mode 100644 index 0000000000..c57ec30768 --- /dev/null +++ b/test/Frontend/plugin-delayed-template.cpp @@ -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 +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 +void ForcedTemplate() { + TemplateDep(); // Shouldn't crash. + + ""; // Triggers the warning checked for above. +}