[clang-tidy] Fix handling --driver-mode=

Driver mode passed as an extra argument (command line or config) were not utilized for removing invalid arguments in stripPositionalArgs function, and even if passed as config driver mode were not used for dependency file striping leading to invalid handling of -MD. Additionally driver mode were needed even if user already added cl.exe after --.

Author: PiotrZSL

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

@@ -19,14 +19,20 @@
 #include "../ClangTidyForceLinker.h"
 #include "../GlobList.h"
 #include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/PluginLoader.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/WithColor.h"
+#include <algorithm>
+#include <iterator>
 #include <optional>
+#include <string>
+#include <vector>
 
 using namespace clang::tooling;
 using namespace llvm;
@@ -476,7 +482,7 @@ static StringRef closest(StringRef Value, const StringSet<> &Allowed) {
  return Closest;
 }
 
-static constexpr StringLiteral VerifyConfigWarningEnd = " [-verify-config]\n";
+static constexpr StringRef VerifyConfigWarningEnd = " [-verify-config]\n";
 
 static bool verifyChecks(const StringSet<> &AllChecks, StringRef CheckGlob,
  StringRef Source) {
@@ -551,9 +557,92 @@ static llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> createBaseFS() {
  return BaseFS;
 }
 
+static void recreateOptionsParserIfNeeded(
+ llvm::Expected<CommonOptionsParser> &OptionsParser,
+ llvm::ArrayRef<const char *> Args,
+ const ClangTidyOptions &EffectiveOptions) {
+
+ if (Args.empty())
+ return;
+
+ const auto DoubleDashIt = llvm::find(Args, StringRef("--"));
+
+ // Exit if we don't have any compiler arguments
+ if (DoubleDashIt == Args.end() || Args.back() == StringRef("--"))
+ return;
+
+ auto IsDriverMode = [](StringRef Argument) {
+ return Argument.starts_with("--driver-mode=");
+ };
+
+ // Exit if --driver-mode= is explicitly passed in compiler arguments
+ if (Args.end() !=
+ std::find_if(std::next(DoubleDashIt), Args.end(), IsDriverMode))
+ return;
+
+ std::vector<std::string> CommandArguments(std::next(DoubleDashIt),
+ Args.end());
+
+ // Add clang-tool as program name if not added
+ if (CommandArguments.empty() ||
+ llvm::StringRef(CommandArguments.front()).starts_with("-"))
+ CommandArguments.insert(CommandArguments.begin(), "clang-tool");
+
+ // Apply --extra-arg and --extra-arg-before to compiler arguments
+ CommandArguments =
+ OptionsParser->getArgumentsAdjuster()(CommandArguments, "");
+
+ // Apply ExtraArgsBefore from clang-tidy config to compiler arguments
+ if (EffectiveOptions.ExtraArgsBefore)
+ CommandArguments = tooling::getInsertArgumentAdjuster(
+ *EffectiveOptions.ExtraArgsBefore,
+ tooling::ArgumentInsertPosition::BEGIN)(CommandArguments, "");
+
+ // Apply ExtraArgs from clang-tidy config to compiler arguments
+ if (EffectiveOptions.ExtraArgs)
+ CommandArguments = tooling::getInsertArgumentAdjuster(
+ *EffectiveOptions.ExtraArgs,
+ tooling::ArgumentInsertPosition::END)(CommandArguments, "");
+
+ // Check if now we have --driver-mode=
+ auto DriverModeIt = std::find_if(CommandArguments.begin(),
+ CommandArguments.end(), IsDriverMode);
+ if (DriverModeIt == CommandArguments.end()) {
+ // Try to detect and add --driver-mode=
+ const std::string ExeName = CommandArguments.front();
+ tooling::addTargetAndModeForProgramName(CommandArguments, ExeName);
+ DriverModeIt = llvm::find_if(CommandArguments, IsDriverMode);
+ }
+
+ // Exit if there is no --driver-mode= at this stage
+ if (DriverModeIt == CommandArguments.end())
+ return;
+
+ std::vector<const char *> NewArgs = Args.vec();
+
+ // Find place to insert --driver-mode= into new args, best after program name.
+ auto InsertIt =
+ NewArgs.begin() + std::distance(Args.begin(), DoubleDashIt) + 1U;
+ if (!StringRef(*InsertIt).starts_with("-"))
+ ++InsertIt;
+ NewArgs.insert(InsertIt, DriverModeIt->c_str());
+
+ // Re-create CommonOptionsParser with assumption that
+ // FixedCompilationDatabase::loadFromCommandLine will be now called with
+ // proper --driver-mode=
+ int ArgC = NewArgs.size();
+ const char **ArgV = NewArgs.data();
+ OptionsParser = CommonOptionsParser::create(ArgC, ArgV, ClangTidyCategory,
+ cl::ZeroOrMore);
+}
+
 int clangTidyMain(int argc, const char **argv) {
  llvm::InitLLVM X(argc, argv);
 
+ // Save original arguments because CommonOptionsParser::create will change
+ // `argc`.
+ llvm::ArrayRef<const char *> Args(argv, argc);
+
  // Enable help for -load option, if plugins are enabled.
  if (cl::Option *LoadOpt = cl::getRegisteredOptions().lookup("load"))
  LoadOpt->addCategory(ClangTidyCategory);
@@ -586,6 +675,12 @@ int clangTidyMain(int argc, const char **argv) {
  SmallString<256> FilePath = makeAbsolute(FileName);
  ClangTidyOptions EffectiveOptions = OptionsProvider->getOptions(FilePath);
 
+ recreateOptionsParserIfNeeded(OptionsParser, Args, EffectiveOptions);
+ if (!OptionsParser) {
+ llvm::WithColor::error() << llvm::toString(OptionsParser.takeError());
+ return 1;
+ }
+
  std::vector<std::string> EnabledChecks =
  getCheckNames(EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
 

clang-tools-extra/docs/ReleaseNotes.rst

@@ -106,12 +106,10 @@ Improvements to clang-query
 Improvements to clang-tidy
 --------------------------
 
-- Improved :program:`clang-tidy`'s `--verify-config` flag by adding support for
- the configuration options of the `Clang Static Analyzer Checks
- <https://clang.llvm.org/docs/analyzer/checkers.html>`_.
-
-- Improved :program:`run-clang-tidy.py` script. Fixed minor shutdown noise
- happening on certain platforms when interrupting the script.
+- Improved handling of `--driver-mode=`, now automatically deducing it from
+ the compiler name after `--`, or properly utilizing it when passed as an
+ extra argument during :program:`clang-tidy` invocation with explicit compiler
+ arguments.
 
 New checks
 ^^^^^^^^^^

clang-tools-extra/test/clang-tidy/infrastructure/driver-mode.cpp

@@ -0,0 +1,57 @@
+// REQUIRES: shell
+// RUN: rm -rf "%t"
+// RUN: mkdir "%t"
+// RUN: cp "%s" "%t/code.cpp"
+// RUN: echo '' > "%t/.clang-tidy"
+
+// Compile commands tests (explicit --driver-mode):
+// RUN: echo '[{"directory":"%t/","file":"code.cpp","command":"dummy-compiler /W4 code.cpp"}]' > "%t/compile_commands.json"
+// RUN: clang-tidy --checks="-*,clang-diagnostic-reorder-ctor,readability-redundant-string-cstr" -p="%t" "%t/code.cpp" \
+// RUN: --extra-arg=--driver-mode=cl 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy --checks="-*,clang-diagnostic-reorder-ctor,readability-redundant-string-cstr" -p="%t" "%t/code.cpp" \
+// RUN: --extra-arg-before=--driver-mode=cl 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy --checks="-*,clang-diagnostic-reorder-ctor,readability-redundant-string-cstr" -p="%t" "%t/code.cpp" \
+// RUN: --config='{ExtraArgs: ["--driver-mode=cl"]}' 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy --checks="-*,clang-diagnostic-reorder-ctor,readability-redundant-string-cstr" -p="%t" "%t/code.cpp" \
+// RUN: --config='{ExtraArgsBefore: ["--driver-mode=cl"]}' 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' %s
+
+// Compile commands tests (implicit --driver-mode):
+// RUN: echo '[{"directory":"%t/","file":"code.cpp","command":"cl.exe /W4 code.cpp"}]' > "%t/compile_commands.json"
+// RUN: clang-tidy --checks="-*,clang-diagnostic-reorder-ctor,readability-redundant-string-cstr" -p="%t" "%t/code.cpp" \
+// RUN: 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' %s
+
+// Compile commands tests (negative)
+// RUN: echo '[{"directory":"%t/","file":"code.cpp","command":"dummy-compiler -MT /W4 code.cpp"}]' > "%t/compile_commands.json"
+// RUN: clang-tidy --checks="-*,clang-diagnostic-reorder-ctor,readability-redundant-string-cstr" -p="%t" "%t/code.cpp" \
+// RUN: 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' --check-prefix=NEGATIVE_CHECK --allow-empty %s
+
+// Command line tests (explicit --driver-mode):
+// RUN: clang-tidy --checks="-*,clang-diagnostic-reorder-ctor,readability-redundant-string-cstr" "%t/code.cpp" \
+// RUN: -- --driver-mode=cl -MD /W4 code.cpp 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy --checks="-*,clang-diagnostic-reorder-ctor,readability-redundant-string-cstr" "%t/code.cpp" \
+// RUN: --extra-arg=--driver-mode=cl -- -MD /W4 code.cpp 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy --checks="-*,clang-diagnostic-reorder-ctor,readability-redundant-string-cstr" "%t/code.cpp" \
+// RUN: --extra-arg-before=--driver-mode=cl -- -MD /W4 code.cpp 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy --checks="-*,clang-diagnostic-reorder-ctor,readability-redundant-string-cstr" -p="%t" "%t/code.cpp" \
+// RUN: --config='{ExtraArgs: ["--driver-mode=cl"]}' -- -MD /W4 code.cpp 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy --checks="-*,clang-diagnostic-reorder-ctor,readability-redundant-string-cstr" -p="%t" "%t/code.cpp" \
+// RUN: --config='{ExtraArgsBefore: ["--driver-mode=cl"]}' -- -MD /W4 code.cpp 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' %s
+
+// Command line tests (implicit --driver-mode):
+// RUN: clang-tidy --checks="-*,clang-diagnostic-reorder-ctor,readability-redundant-string-cstr" -p="%t" "%t/code.cpp" \
+// RUN: -- cl.exe -MD /W4 code.cpp 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' %s
+
+// Command line tests (negative)
+// RUN: clang-tidy --checks="-*,clang-diagnostic-reorder-ctor,readability-redundant-string-cstr" -p="%t" "%t/code.cpp" \
+// RUN: -- dummy-compiler -MT /W4 code.cpp 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' --check-prefix=NEGATIVE_CHECK --allow-empty %s
+
+struct string {};
+
+struct A {
+ A(string aa, string bb) : b(bb), a(aa) {}
+// CHECK: code.cpp:[[@LINE-1]]:31: warning: field 'b' will be initialized after field 'a' [clang-diagnostic-reorder-ctor]
+// NEGATIVE_CHECK-NOT: warning: field 'b' will be initialized after field 'a' [clang-diagnostic-reorder-ctor]
+
+ string a;
+ string b;
+};