[clang-tools-extra] [clang-tidy] Fix handling --driver-mode= (PR #66553)
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 15 14:47:20 PDT 2023
https://github.com/PiotrZSL created https://github.com/llvm/llvm-project/pull/66553
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 --.
Fixes: #65108, #53674, #52687, #44701
>From 3bc42b19b245e3497aadfc7642957cd349712723 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Fri, 15 Sep 2023 21:39:17 +0000
Subject: [PATCH] [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 --.
---
.../clang-tidy/tool/ClangTidyMain.cpp | 127 ++++++++++++++----
clang-tools-extra/docs/ReleaseNotes.rst | 5 +
.../clang-tidy/infrastructure/driver-mode.cpp | 57 ++++++++
3 files changed, 164 insertions(+), 25 deletions(-)
create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/driver-mode.cpp
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index dd7f8141a694e2a..152bfbf6bc61f99 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -19,6 +19,7 @@
#include "../ClangTidyForceLinker.h"
#include "../GlobList.h"
#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/Support/InitLLVM.h"
#include "llvm/Support/PluginLoader.h"
@@ -26,6 +27,7 @@
#include "llvm/Support/Signals.h"
#include "llvm/Support/TargetSelect.h"
#include "llvm/Support/WithColor.h"
+#include <algorithm>
#include <optional>
using namespace clang::tooling;
@@ -450,7 +452,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) {
@@ -525,8 +527,95 @@ static bool verifyFileExtensions(
return AnyInvalid;
}
+static SmallString<256> makeAbsolute(llvm::StringRef Input) {
+ if (Input.empty())
+ return {};
+ SmallString<256> AbsolutePath(Input);
+ if (std::error_code EC = llvm::sys::fs::make_absolute(AbsolutePath)) {
+ llvm::errs() << "Can't make absolute path from " << Input << ": "
+ << EC.message() << "\n";
+ }
+ return AbsolutePath;
+}
+
+static llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> createBaseFS() {
+ llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> BaseFS(
+ new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
+
+ if (!VfsOverlay.empty()) {
+ IntrusiveRefCntPtr<vfs::FileSystem> VfsFromFile =
+ getVfsFromFile(VfsOverlay, BaseFS);
+ if (!VfsFromFile)
+ return nullptr;
+ BaseFS->pushOverlay(std::move(VfsFromFile));
+ }
+ return BaseFS;
+}
+
+static llvm::Expected<CommonOptionsParser>
+recreateOptionsParserIfNeeded(llvm::ArrayRef<const char *> Args,
+ llvm::Expected<CommonOptionsParser> OptionsParser,
+ const ClangTidyOptions &EffectiveOptions) {
+
+ auto DoubleDashIt = std::find(Args.begin(), Args.end(), StringRef("--"));
+ if (DoubleDashIt == Args.end() || Args.empty() ||
+ Args.back() == StringRef("--"))
+ return OptionsParser;
+
+ auto IsDriverMode = [](StringRef Argument) {
+ return Argument.startswith("--driver-mode=");
+ };
+
+ if (Args.end() !=
+ std::find_if(std::next(DoubleDashIt), Args.end(), IsDriverMode))
+ return OptionsParser;
+
+ std::vector<std::string> CommandArguments(std::next(DoubleDashIt),
+ Args.end());
+ if (CommandArguments.empty() ||
+ llvm::StringRef(CommandArguments.front()).startswith("-"))
+ CommandArguments.insert(CommandArguments.begin(), "clang-tool");
+
+ CommandArguments =
+ OptionsParser->getArgumentsAdjuster()(CommandArguments, "");
+ if (EffectiveOptions.ExtraArgsBefore)
+ CommandArguments = tooling::getInsertArgumentAdjuster(
+ *EffectiveOptions.ExtraArgsBefore,
+ tooling::ArgumentInsertPosition::BEGIN)(CommandArguments, "");
+
+ if (EffectiveOptions.ExtraArgs)
+ CommandArguments = tooling::getInsertArgumentAdjuster(
+ *EffectiveOptions.ExtraArgs,
+ tooling::ArgumentInsertPosition::END)(CommandArguments, "");
+
+ auto DriverModeIt = std::find_if(CommandArguments.begin(),
+ CommandArguments.end(), IsDriverMode);
+ if (DriverModeIt == CommandArguments.end()) {
+ std::string ExeName = CommandArguments.front();
+ tooling::addTargetAndModeForProgramName(CommandArguments, ExeName);
+ DriverModeIt = std::find_if(CommandArguments.begin(),
+ CommandArguments.end(), IsDriverMode);
+ }
+
+ if (DriverModeIt == CommandArguments.end())
+ return OptionsParser;
+
+ std::vector<const char *> NewArgs(Args.begin(), Args.end());
+ auto InsertIt =
+ NewArgs.begin() + std::distance(Args.begin(), DoubleDashIt) + 1U;
+ if (!StringRef(*InsertIt).startswith("-"))
+ ++InsertIt;
+ NewArgs.insert(InsertIt, DriverModeIt->c_str());
+
+ int ArgC = NewArgs.size();
+ const char **ArgV = NewArgs.data();
+ return CommonOptionsParser::create(ArgC, ArgV, ClangTidyCategory,
+ cl::ZeroOrMore);
+}
+
int clangTidyMain(int argc, const char **argv) {
llvm::InitLLVM X(argc, argv);
+ llvm::ArrayRef<const char *> Args(argv, argc);
// Enable help for -load option, if plugins are enabled.
if (cl::Option *LoadOpt = cl::getRegisteredOptions().lookup("load"))
@@ -540,34 +629,16 @@ int clangTidyMain(int argc, const char **argv) {
return 1;
}
- llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> BaseFS(
- new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
-
- if (!VfsOverlay.empty()) {
- IntrusiveRefCntPtr<vfs::FileSystem> VfsFromFile =
- getVfsFromFile(VfsOverlay, BaseFS);
- if (!VfsFromFile)
- return 1;
- BaseFS->pushOverlay(std::move(VfsFromFile));
- }
+ llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> BaseFS = createBaseFS();
+ if (!BaseFS)
+ return 1;
auto OwningOptionsProvider = createOptionsProvider(BaseFS);
auto *OptionsProvider = OwningOptionsProvider.get();
if (!OptionsProvider)
return 1;
- auto MakeAbsolute = [](const std::string &Input) -> SmallString<256> {
- if (Input.empty())
- return {};
- SmallString<256> AbsolutePath(Input);
- if (std::error_code EC = llvm::sys::fs::make_absolute(AbsolutePath)) {
- llvm::errs() << "Can't make absolute path from " << Input << ": "
- << EC.message() << "\n";
- }
- return AbsolutePath;
- };
-
- SmallString<256> ProfilePrefix = MakeAbsolute(StoreCheckProfile);
+ SmallString<256> ProfilePrefix = makeAbsolute(StoreCheckProfile);
StringRef FileName("dummy");
auto PathList = OptionsParser->getSourcePathList();
@@ -575,9 +646,15 @@ int clangTidyMain(int argc, const char **argv) {
FileName = PathList.front();
}
- SmallString<256> FilePath = MakeAbsolute(std::string(FileName));
-
+ SmallString<256> FilePath = makeAbsolute(FileName);
ClangTidyOptions EffectiveOptions = OptionsProvider->getOptions(FilePath);
+ OptionsParser = recreateOptionsParserIfNeeded(Args, std::move(OptionsParser),
+ EffectiveOptions);
+ if (!OptionsParser) {
+ llvm::WithColor::error() << llvm::toString(OptionsParser.takeError());
+ return 1;
+ }
+
std::vector<std::string> EnabledChecks =
getCheckNames(EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 19c977977f9044c..013f09f97e18ded 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -122,6 +122,11 @@ Improvements to clang-tidy
if any :program:`clang-tidy` subprocess exits with a non-zero code or if
exporting fixes fails.
+- 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
^^^^^^^^^^
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/driver-mode.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/driver-mode.cpp
new file mode 100644
index 000000000000000..fed017e5de09f23
--- /dev/null
+++ b/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;
+};
More information about the cfe-commits
mailing list