[clang-tidy] Define __clang_analyzer__ macro for clang-tidy for compatibility with clang static analyzer

[zinovy.nis]

Summary

This macro is widely used in many well-known projects, ex. Chromium.

But it's not set for clang-tidy, so for ex. DCHECK in Chromium is not considered as [[no-return]], and a lot of false-positive warnings about nullptr dereferenced are emitted.

This patch fixes the issue by explicitly added macro definition.

[alexfh]

clang-tidy/ClangTidy.cpp

490 ↗

(On Diff #139843)

I wonder whether we should instead reuse the logic in the frontend (tools/clang/lib/Frontend/InitPreprocessor.cpp:970). This could be done by overriding ActionFactory::runInvocation and setting FrontendOptions::ProgramAction to frontend::RunAnalysis there. WDYT?

[zinovy.nis]

clang-tidy/ClangTidy.cpp

490 ↗

(On Diff #139843)

Thanks. I know about this code. But I'm not sure that we can reuse RunAnalysys action hre - we need to parse AST only, but RunAnalysys does a lot of other things we dont' need. Is it correct?

[alexfh]

LG

clang-tidy/ClangTidy.cpp

490 ↗

(On Diff #139843)

Clang tooling accepts the action from the client (in this case clang-tidy) and doesn't rely on frontend code that creates an action depending on the value of FrontendOptions::ProgramAction (clang::ExecuteCompilerInvocation). Thus, setting this frontend option will only trigger the logic in Frontend/InitPreprocessor.cpp that defines the macro.

On one hand, doing so will help clang-tidy to keep distance from whatever is done in the frontend to support static analyzer. On the other side, as your answer shows, this may not be a clear way to interact with that logic.

So I'm fine with either implementation.

[thakis]

The test added here doesn't pass on Windows, and the change breaks another test on Windows: http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/9794

[thakis]

Actually, it doesn't pass on non-Windows either: http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/27665/steps/test/logs/stdio

[zinovy.nis]

Looks like quotes are required.
Thanks for pointing! I'll submit a patch for this.

пн, 2 апр. 2018 г. в 17:28, Nico Weber via Phabricator <
[e-mail]>:

thakis added a comment.

Actually, it doesn't pass on non-Windows either:
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/27665/steps/test/logs/stdio

Repository:

rL LLVM

https://reviews.llvm.org/D44906

clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp

 void runClangTidy(clang::tidy::ClangTidyContext &Context,
                   const CompilationDatabase &Compilations,
                   ArrayRef<std::string> InputFiles,
                   llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS,
                   ProfileData *Profile) {
   ClangTool Tool(Compilations, InputFiles,
                  std::make_shared<PCHContainerOperations>(), BaseFS);
 
+  // Add __clang_analyzer__ macro definition for compatibility with the clang
+  // static analyzer.
+  ArgumentsAdjuster ClangTidyMacroDefinitionInserter =
+      [&Context](const CommandLineArguments &Args, StringRef Filename) {
+        ClangTidyOptions Opts = Context.getOptionsForFile(Filename);
+        CommandLineArguments AdjustedArgs = Args;
+        AdjustedArgs.emplace_back("-D__clang_analyzer__");
+        return AdjustedArgs;
+      };
+
   // Add extra arguments passed by the clang-tidy command-line.
   ArgumentsAdjuster PerFileExtraArgumentsInserter =
       [&Context](const CommandLineArguments &Args, StringRef Filename) {
         ClangTidyOptions Opts = Context.getOptionsForFile(Filename);
         CommandLineArguments AdjustedArgs = Args;
         if (Opts.ExtraArgsBefore) {
           auto I = AdjustedArgs.begin();
           if (I != AdjustedArgs.end() && !StringRef(*I).startswith("-"))
ArgumentsAdjuster PluginArgumentsRemover =
               Args[I + 2] == "-Xclang") {
             I += 3;
           } else
             AdjustedArgs.push_back(Args[I]);
         }
         return AdjustedArgs;
       };
 
+  Tool.appendArgumentsAdjuster(ClangTidyMacroDefinitionInserter);
   Tool.appendArgumentsAdjuster(PerFileExtraArgumentsInserter);
   Tool.appendArgumentsAdjuster(PluginArgumentsRemover);
   if (Profile)
     Context.setCheckProfileData(Profile);
 
   ClangTidyDiagnosticConsumer DiagConsumer(Context);
 
   Tool.setDiagnosticConsumer(&DiagConsumer);

clang-tools-extra/trunk/test/clang-tidy/clang-tidy-__clang_analyzer__macro.cpp

+// RUN: %check_clang_tidy %s * %t
+
+#if defined(__clang_analyzer__)
+#warning __clang_analyzer__ is defined
+#endif
+// CHECK-MESSAGES: :[[@LINE-2]]:2: warning: __clang_analyzer__ is defined [clang-diagnostic-#warnings]
+
+