[clang-tools-extra] [clang-tidy] Fix handling --driver-mode= (PR #66553)
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Sat Mar 30 22:39:25 PDT 2024
https://github.com/PiotrZSL updated https://github.com/llvm/llvm-project/pull/66553
>From f4f9c996171892813dc11eb5bcb29e0616475f40 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 1/3] [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 | 96 ++++++++++++++++++-
clang-tools-extra/docs/ReleaseNotes.rst | 5 +
.../clang-tidy/infrastructure/driver-mode.cpp | 57 +++++++++++
3 files changed, 157 insertions(+), 1 deletion(-)
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 9f3d6b6db6cbca1..b594319187dbcc8 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,7 +27,11 @@
#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;
@@ -450,7 +455,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) {
@@ -550,9 +555,91 @@ static llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> createBaseFS() {
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("--"));
+
+ // Exit if we don't have any compiler arguments
+ if (DoubleDashIt == Args.end() || Args.empty() ||
+ Args.back() == StringRef("--"))
+ return OptionsParser;
+
+ auto IsDriverMode = [](StringRef Argument) {
+ return Argument.startswith("--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 OptionsParser;
+
+ 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()).startswith("-"))
+ 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=
+ std::string ExeName = CommandArguments.front();
+ tooling::addTargetAndModeForProgramName(CommandArguments, ExeName);
+ DriverModeIt = std::find_if(CommandArguments.begin(),
+ CommandArguments.end(), IsDriverMode);
+ }
+
+ // Exit if there is no --driver-mode= at this stage
+ if (DriverModeIt == CommandArguments.end())
+ return OptionsParser;
+
+ std::vector<const char *> NewArgs(Args.begin(), Args.end());
+
+ // 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).startswith("-"))
+ ++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();
+ return 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);
@@ -585,6 +672,13 @@ int clangTidyMain(int argc, const char **argv) {
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 a604e9276668aec..2da6ee4d272a47a 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -101,6 +101,11 @@ Improvements to clang-tidy
to filter source files from the compilation database, via a RegEx. In a
similar fashion to what `-header-filter` does for header files.
+- 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;
+};
>From 444211f62e9bfd75d1e17cd61a752f58b713e260 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Sat, 16 Sep 2023 12:19:34 +0000
Subject: [PATCH 2/3] [clang-tidy] Fix unchecked copy of llvm::Expected
Change code to avoid doing a copy of a source
llvm::Expected.
---
.../clang-tidy/tool/ClangTidyMain.cpp | 21 +++++++++----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index b594319187dbcc8..f9103ef2ab8d1ed 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -555,17 +555,17 @@ static llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> createBaseFS() {
return BaseFS;
}
-static llvm::Expected<CommonOptionsParser>
-recreateOptionsParserIfNeeded(llvm::ArrayRef<const char *> Args,
- llvm::Expected<CommonOptionsParser> OptionsParser,
- const ClangTidyOptions &EffectiveOptions) {
+static void recreateOptionsParserIfNeeded(
+ llvm::Expected<CommonOptionsParser> &OptionsParser,
+ llvm::ArrayRef<const char *> Args,
+ const ClangTidyOptions &EffectiveOptions) {
auto DoubleDashIt = std::find(Args.begin(), Args.end(), StringRef("--"));
// Exit if we don't have any compiler arguments
if (DoubleDashIt == Args.end() || Args.empty() ||
Args.back() == StringRef("--"))
- return OptionsParser;
+ return;
auto IsDriverMode = [](StringRef Argument) {
return Argument.startswith("--driver-mode=");
@@ -574,7 +574,7 @@ recreateOptionsParserIfNeeded(llvm::ArrayRef<const char *> Args,
// Exit if --driver-mode= is explicitly passed in compiler arguments
if (Args.end() !=
std::find_if(std::next(DoubleDashIt), Args.end(), IsDriverMode))
- return OptionsParser;
+ return;
std::vector<std::string> CommandArguments(std::next(DoubleDashIt),
Args.end());
@@ -613,7 +613,7 @@ recreateOptionsParserIfNeeded(llvm::ArrayRef<const char *> Args,
// Exit if there is no --driver-mode= at this stage
if (DriverModeIt == CommandArguments.end())
- return OptionsParser;
+ return;
std::vector<const char *> NewArgs(Args.begin(), Args.end());
@@ -629,8 +629,8 @@ recreateOptionsParserIfNeeded(llvm::ArrayRef<const char *> Args,
// proper --driver-mode=
int ArgC = NewArgs.size();
const char **ArgV = NewArgs.data();
- return CommonOptionsParser::create(ArgC, ArgV, ClangTidyCategory,
- cl::ZeroOrMore);
+ OptionsParser = CommonOptionsParser::create(ArgC, ArgV, ClangTidyCategory,
+ cl::ZeroOrMore);
}
int clangTidyMain(int argc, const char **argv) {
@@ -672,8 +672,7 @@ int clangTidyMain(int argc, const char **argv) {
SmallString<256> FilePath = makeAbsolute(FileName);
ClangTidyOptions EffectiveOptions = OptionsProvider->getOptions(FilePath);
- OptionsParser = recreateOptionsParserIfNeeded(Args, std::move(OptionsParser),
- EffectiveOptions);
+ recreateOptionsParserIfNeeded(OptionsParser, Args, EffectiveOptions);
if (!OptionsParser) {
llvm::WithColor::error() << llvm::toString(OptionsParser.takeError());
return 1;
>From 37ed735950846f2367134f72b5cc92b977dce308 Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Sun, 31 Mar 2024 05:36:50 +0000
Subject: [PATCH 3/3] Fix nits
---
.../clang-tidy/tool/ClangTidyMain.cpp | 22 ++++++++++---------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index f9103ef2ab8d1ed..007eaf18413c9d5 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -20,6 +20,7 @@
#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"
@@ -560,15 +561,17 @@ static void recreateOptionsParserIfNeeded(
llvm::ArrayRef<const char *> Args,
const ClangTidyOptions &EffectiveOptions) {
- auto DoubleDashIt = std::find(Args.begin(), Args.end(), StringRef("--"));
+ 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.empty() ||
- Args.back() == StringRef("--"))
+ if (DoubleDashIt == Args.end() || Args.back() == StringRef("--"))
return;
auto IsDriverMode = [](StringRef Argument) {
- return Argument.startswith("--driver-mode=");
+ return Argument.starts_with("--driver-mode=");
};
// Exit if --driver-mode= is explicitly passed in compiler arguments
@@ -581,7 +584,7 @@ static void recreateOptionsParserIfNeeded(
// Add clang-tool as program name if not added
if (CommandArguments.empty() ||
- llvm::StringRef(CommandArguments.front()).startswith("-"))
+ llvm::StringRef(CommandArguments.front()).starts_with("-"))
CommandArguments.insert(CommandArguments.begin(), "clang-tool");
// Apply --extra-arg and --extra-arg-before to compiler arguments
@@ -605,22 +608,21 @@ static void recreateOptionsParserIfNeeded(
CommandArguments.end(), IsDriverMode);
if (DriverModeIt == CommandArguments.end()) {
// Try to detect and add --driver-mode=
- std::string ExeName = CommandArguments.front();
+ const std::string ExeName = CommandArguments.front();
tooling::addTargetAndModeForProgramName(CommandArguments, ExeName);
- DriverModeIt = std::find_if(CommandArguments.begin(),
- CommandArguments.end(), IsDriverMode);
+ 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.begin(), Args.end());
+ 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).startswith("-"))
+ if (!StringRef(*InsertIt).starts_with("-"))
++InsertIt;
NewArgs.insert(InsertIt, DriverModeIt->c_str());
More information about the cfe-commits
mailing list