Add a clang-tidy flag to support temporary destructor-aware analysis (workaround for bug 15599).

[alexmc]

This is my first time sending a review with arc & phabricator: please let me know if I've set anything up incorrectly!

[alexmc]

• Fix typo.

[klimek]

+cfe-commits

Alex, you'll always want to cc' cfe-commits when sending patches to clang-related things.

[alexfh]

Looks good modulo comments. Thanks for working on this!

clang-tidy/ClangTidy.h

116

I'd not make any assumptions about the life-time of Options. Please just copy it here, not store a reference.

test/clang-tidy/temporaries.cpp

2

Add -disable-checks='', so that we don't rely on its default value.

3

Could you instead add test code which actually generates a core.NullDereference warning, so that we can be sure it's not just completely disabled.

// CHECK-NOT: warning
void testNullPointerDereferencePositive() {
  int *value = 0;
  // CHECK: :[[@LINE+1]]:3: warning: .....
  *value = 1;
}

// CHECK-NOT: warning
<the rest of your test case>

[alexmc]

• Respond to review comments: copy options instead of maintaining a reference, and add more test cases.

[alexmc]

Submitting now.

clang-tidy/ClangTidy.h

116

Done.

test/clang-tidy/temporaries.cpp

2

Done.

3

Done.

[alexfh]

test/clang-tidy/temporaries.cpp

3

You don't need this now as the correct output is guaranteed to be non-empty. And you can now just pipe clang-tidy output to FileCheck

[alexmc]

test/clang-tidy/temporaries.cpp

3

Good catch: fixed in llvm-reviews.chandlerc.com/rL207653

clang-tidy/ClangTidy.h

private:
   ClangTidyContext *Context;
   std::string CheckName;
 };
 
 class ClangTidyCheckFactories;
 
 class ClangTidyASTConsumerFactory {
 public:
-  ClangTidyASTConsumerFactory(ClangTidyContext &Context);
+  ClangTidyASTConsumerFactory(ClangTidyContext &Context,
+                              const ClangTidyOptions &Options);
   ~ClangTidyASTConsumerFactory();
 
   /// \brief Returns an ASTConsumer that runs the specified clang-tidy checks.
   clang::ASTConsumer *CreateASTConsumer(clang::CompilerInstance &Compiler,
                                         StringRef File);
 
   /// \brief Get the list of enabled checks.
   std::vector<std::string> getCheckNames();
 
 private:
   typedef std::vector<std::pair<std::string, bool> > CheckersList;
   CheckersList getCheckersControlList();
 
   SmallVector<ClangTidyCheck *, 8> Checks;
   ClangTidyContext &Context;
   ast_matchers::MatchFinder Finder;
   std::unique_ptr<ClangTidyCheckFactories> CheckFactories;
+  ClangTidyOptions Options;
 };
 
 /// \brief Fills the list of check names that are enabled when the provided
 /// filters are applied.
 std::vector<std::string> getCheckNames(const ClangTidyOptions &Options);
 
 /// \brief Run a set of clang-tidy checks on a set of files.
 void runClangTidy(const ClangTidyOptions &Options,

clang-tidy/ClangTidy.cpp

private:
   bool ApplyFixes;
   unsigned TotalFixes;
   unsigned AppliedFixes;
 };
 
 } // namespace
 
 ClangTidyASTConsumerFactory::ClangTidyASTConsumerFactory(
-    ClangTidyContext &Context)
-    : Context(Context), CheckFactories(new ClangTidyCheckFactories) {
+    ClangTidyContext &Context, const ClangTidyOptions &Options)
+    : Context(Context), CheckFactories(new ClangTidyCheckFactories),
+      Options(Options) {
   for (ClangTidyModuleRegistry::iterator I = ClangTidyModuleRegistry::begin(),
                                          E = ClangTidyModuleRegistry::end();
        I != E; ++I) {
     std::unique_ptr<ClangTidyModule> Module(I->instantiate());
     Module->addCheckFactories(*CheckFactories);
   }
 
   CheckFactories->createChecks(Context.getChecksFilter(), Checks);
clang::ASTConsumer *ClangTidyASTConsumerFactory::CreateASTConsumer(
   Context.setSourceManager(&Compiler.getSourceManager());
   for (ClangTidyCheck *Check : Checks)
     Check->registerPPCallbacks(Compiler);
 
   SmallVector<ASTConsumer *, 2> Consumers;
   if (!CheckFactories->empty())
     Consumers.push_back(Finder.newASTConsumer());
 
-  AnalyzerOptionsRef Options = Compiler.getAnalyzerOpts();
-  Options->CheckersControlList = getCheckersControlList();
-  if (!Options->CheckersControlList.empty()) {
-    Options->AnalysisStoreOpt = RegionStoreModel;
-    Options->AnalysisDiagOpt = PD_NONE;
-    Options->AnalyzeNestedBlocks = true;
-    Options->eagerlyAssumeBinOpBifurcation = true;
+  AnalyzerOptionsRef AnalyzerOptions = Compiler.getAnalyzerOpts();
+  // FIXME: Remove this option once clang's cfg-temporary-dtors option defaults
+  // to true.
+  AnalyzerOptions->Config["cfg-temporary-dtors"] =
+      Options.AnalyzeTemporaryDtors ? "true" : "false";
+
+  AnalyzerOptions->CheckersControlList = getCheckersControlList();
+  if (!AnalyzerOptions->CheckersControlList.empty()) {
+    AnalyzerOptions->AnalysisStoreOpt = RegionStoreModel;
+    AnalyzerOptions->AnalysisDiagOpt = PD_NONE;
+    AnalyzerOptions->AnalyzeNestedBlocks = true;
+    AnalyzerOptions->eagerlyAssumeBinOpBifurcation = true;
     ento::AnalysisASTConsumer *AnalysisConsumer = ento::CreateAnalysisConsumer(
         Compiler.getPreprocessor(), Compiler.getFrontendOpts().OutputFile,
-        Options, Compiler.getFrontendOpts().Plugins);
+        AnalyzerOptions, Compiler.getFrontendOpts().Plugins);
     AnalysisConsumer->AddDiagnosticConsumer(
         new AnalyzerDiagnosticConsumer(Context));
     Consumers.push_back(AnalysisConsumer);
   }
   return new MultiplexConsumer(Consumers);
 }
 
 std::vector<std::string> ClangTidyASTConsumerFactory::getCheckNames() {

 void ClangTidyCheck::setName(StringRef Name) {
   assert(CheckName.empty());
   CheckName = Name.str();
 }
 
 std::vector<std::string> getCheckNames(const ClangTidyOptions &Options) {
   SmallVector<ClangTidyError, 8> Errors;
   clang::tidy::ClangTidyContext Context(&Errors, Options);
-  ClangTidyASTConsumerFactory Factory(Context);
+  ClangTidyASTConsumerFactory Factory(Context, Options);
   return Factory.getCheckNames();
 }
 
 void runClangTidy(const ClangTidyOptions &Options,
                   const tooling::CompilationDatabase &Compilations,
                   ArrayRef<std::string> Ranges,
                   SmallVectorImpl<ClangTidyError> *Errors) {
   // FIXME: Ranges are currently full files. Support selecting specific
private:
 
     private:
       ClangTidyASTConsumerFactory *Factory;
     };
 
     ClangTidyASTConsumerFactory *ConsumerFactory;
   };
 
-  Tool.run(new ActionFactory(new ClangTidyASTConsumerFactory(Context)));
+  Tool.run(new ActionFactory(new ClangTidyASTConsumerFactory(Context, Options)));
 }
 
 void handleErrors(SmallVectorImpl<ClangTidyError> &Errors, bool Fix) {
   ErrorReporter Reporter(Fix);
   for (const ClangTidyError &Error : Errors) {
     Reporter.reportDiagnostic(Error.Message, DiagnosticsEngine::Warning,
                               &Error.Fix);
     for (const ClangTidyMessage &Note : Error.Notes)
       Reporter.reportDiagnostic(Note, DiagnosticsEngine::Note);
   }
   Reporter.Finish();
 }
 
 } // namespace tidy
 } // namespace clang

clang-tidy/ClangTidyOptions.h

 namespace clang {
 namespace tidy {
 
 /// \brief Contains options for clang-tidy.
 struct ClangTidyOptions {
   ClangTidyOptions() : EnableChecksRegex(".*") {}
   std::string EnableChecksRegex;
   std::string DisableChecksRegex;
+  bool AnalyzeTemporaryDtors;
 };
 
 } // end namespace tidy
 } // end namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANG_TIDY_OPTIONS_H

clang-tidy/tool/ClangTidyMain.cpp

static cl::opt<std::string> DisableChecks(
     cl::cat(ClangTidyCategory));
 static cl::opt<bool> Fix("fix", cl::desc("Fix detected errors if possible."),
                          cl::init(false), cl::cat(ClangTidyCategory));
 
 static cl::opt<bool> ListChecks("list-checks",
                                 cl::desc("List all enabled checks and exit."),
                                 cl::init(false), cl::cat(ClangTidyCategory));
 
+static cl::opt<bool> AnalyzeTemporaryDtors(
+    "analyze-temporary-dtors",
+    cl::desc("Enable temporary destructor-aware analysis in clang-analyzer- "
+             "checks."),
+    cl::init(false),
+    cl::cat(ClangTidyCategory));
+
 int main(int argc, const char **argv) {
   CommonOptionsParser OptionsParser(argc, argv, ClangTidyCategory);
 
   clang::tidy::ClangTidyOptions Options;
   Options.EnableChecksRegex = Checks;
   Options.DisableChecksRegex = DisableChecks;
+  Options.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
 
   // FIXME: Allow using --list-checks without positional arguments.
   if (ListChecks) {
     llvm::outs() << "Enabled checks:";
     for (auto CheckName : clang::tidy::getCheckNames(Options))
       llvm::outs() << "\n    " << CheckName;
     llvm::outs() << "\n\n";
     return 0;

test/clang-tidy/temporaries.cpp

+// RUN: clang-tidy -checks=clang-analyzer-core.NullDereference -disable-checks='' -analyze-temporary-dtors %s -- > %t.log
+// FileCheck complains if the input file is empty, so add a dummy line.
+// RUN: echo foo >> %t.log
+// RUN: FileCheck %s < %t.log
+
+struct NoReturnDtor {
+  ~NoReturnDtor() __attribute__((noreturn));
+};
+
+extern bool check(const NoReturnDtor &);
+
+// CHECK-NOT: warning
+void testNullPointerDereferencePositive() {
+  int *value = 0;
+  // CHECK: [[@LINE+1]]:10: warning: Dereference of null pointer (loaded from variable 'value') [clang-analyzer-core.NullDereference]
+  *value = 1;
+}
+
+// CHECK-NOT: warning
+void testNullPointerDereference() {
+  int *value = 0;
+  if (check(NoReturnDtor())) {
+    // This unreachable code causes a warning if we don't run with -analyze-temporary-dtors
+    *value = 1;
+  }
+}