Skip to content

[clang-tidy] support parameters file in command line #120547

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

HerrCai0907
Copy link
Contributor

Fixes: #103499

@llvmbot
Copy link
Member

llvmbot commented Dec 19, 2024

@llvm/pr-subscribers-clang-tidy

Author: Congcong Cai (HerrCai0907)

Changes

Fixes: #103499


Full diff: https://github.com/llvm/llvm-project/pull/120547.diff

4 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp (+16)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+2)
  • (added) clang-tools-extra/test/clang-tidy/infrastructure/Inputs/param/parameters.txt (+2)
  • (added) clang-tools-extra/test/clang-tidy/infrastructure/read-parameters-from-file.cpp (+5)
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index d42dafa8ffc362..9aebd450e458c1 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -20,12 +20,14 @@
 #include "../GlobList.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "llvm/ADT/StringSet.h"
+#include "llvm/Support/CommandLine.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 "llvm/TargetParser/Host.h"
 #include <optional>
 
 using namespace clang::tooling;
@@ -553,6 +555,20 @@ static llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> createBaseFS() {
 
 int clangTidyMain(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
+  SmallVector<const char *> Args{argv, argv + argc};
+
+  llvm::BumpPtrAllocator Alloc;
+  llvm::cl::TokenizerCallback Tokenizer =
+      llvm::Triple(llvm::sys::getProcessTriple()).isOSWindows()
+          ? llvm::cl::TokenizeWindowsCommandLine
+          : llvm::cl::TokenizeGNUCommandLine;
+  llvm::cl::ExpansionContext ECtx(Alloc, Tokenizer);
+  if (llvm::Error Err = ECtx.expandResponseFiles(Args)) {
+    llvm::WithColor::error() << Err << "\n";
+    return 1;
+  }
+  argc = static_cast<int>(Args.size());
+  argv = Args.data();
 
   // Enable help for -load option, if plugins are enabled.
   if (cl::Option *LoadOpt = cl::getRegisteredOptions().lookup("load"))
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3fd7a4f9da18ad..5999a7134c7528 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,8 @@ Improvements to clang-tidy
 - Improved :program:`run-clang-tidy.py` script. Fixed minor shutdown noise
   happening on certain platforms when interrupting the script.
 
+- Improved :program:`clang-tidy` by accepting parameters file in command line.
+
 - Removed :program:`clang-tidy`'s global options for most of checks. All options
   are changed to local options except `IncludeStyle`, `StrictMode` and
   `IgnoreMacros`.
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/param/parameters.txt b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/param/parameters.txt
new file mode 100644
index 00000000000000..a6d8fa7ee299fa
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/param/parameters.txt
@@ -0,0 +1,2 @@
+-checks='-*,llvm-namespace-comment'
+--warnings-as-errors=llvm-namespace-comment
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/read-parameters-from-file.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/read-parameters-from-file.cpp
new file mode 100644
index 00000000000000..9d8c40a2e7d415
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/read-parameters-from-file.cpp
@@ -0,0 +1,5 @@
+// RUN: not clang-tidy %s @%S/Inputs/param/parameters.txt -- | FileCheck %s
+
+namespace i {
+}
+// CHECK: error: namespace 'i' not terminated with a closing comment [llvm-namespace-comment,-warnings-as-errors]

@llvmbot
Copy link
Member

llvmbot commented Dec 19, 2024

@llvm/pr-subscribers-clang-tools-extra

Author: Congcong Cai (HerrCai0907)

Changes

Fixes: #103499


Full diff: https://github.com/llvm/llvm-project/pull/120547.diff

4 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp (+16)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+2)
  • (added) clang-tools-extra/test/clang-tidy/infrastructure/Inputs/param/parameters.txt (+2)
  • (added) clang-tools-extra/test/clang-tidy/infrastructure/read-parameters-from-file.cpp (+5)
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index d42dafa8ffc362..9aebd450e458c1 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -20,12 +20,14 @@
 #include "../GlobList.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "llvm/ADT/StringSet.h"
+#include "llvm/Support/CommandLine.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 "llvm/TargetParser/Host.h"
 #include <optional>
 
 using namespace clang::tooling;
@@ -553,6 +555,20 @@ static llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> createBaseFS() {
 
 int clangTidyMain(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
+  SmallVector<const char *> Args{argv, argv + argc};
+
+  llvm::BumpPtrAllocator Alloc;
+  llvm::cl::TokenizerCallback Tokenizer =
+      llvm::Triple(llvm::sys::getProcessTriple()).isOSWindows()
+          ? llvm::cl::TokenizeWindowsCommandLine
+          : llvm::cl::TokenizeGNUCommandLine;
+  llvm::cl::ExpansionContext ECtx(Alloc, Tokenizer);
+  if (llvm::Error Err = ECtx.expandResponseFiles(Args)) {
+    llvm::WithColor::error() << Err << "\n";
+    return 1;
+  }
+  argc = static_cast<int>(Args.size());
+  argv = Args.data();
 
   // Enable help for -load option, if plugins are enabled.
   if (cl::Option *LoadOpt = cl::getRegisteredOptions().lookup("load"))
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3fd7a4f9da18ad..5999a7134c7528 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,8 @@ Improvements to clang-tidy
 - Improved :program:`run-clang-tidy.py` script. Fixed minor shutdown noise
   happening on certain platforms when interrupting the script.
 
+- Improved :program:`clang-tidy` by accepting parameters file in command line.
+
 - Removed :program:`clang-tidy`'s global options for most of checks. All options
   are changed to local options except `IncludeStyle`, `StrictMode` and
   `IgnoreMacros`.
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/param/parameters.txt b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/param/parameters.txt
new file mode 100644
index 00000000000000..a6d8fa7ee299fa
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/param/parameters.txt
@@ -0,0 +1,2 @@
+-checks='-*,llvm-namespace-comment'
+--warnings-as-errors=llvm-namespace-comment
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/read-parameters-from-file.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/read-parameters-from-file.cpp
new file mode 100644
index 00000000000000..9d8c40a2e7d415
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/read-parameters-from-file.cpp
@@ -0,0 +1,5 @@
+// RUN: not clang-tidy %s @%S/Inputs/param/parameters.txt -- | FileCheck %s
+
+namespace i {
+}
+// CHECK: error: namespace 'i' not terminated with a closing comment [llvm-namespace-comment,-warnings-as-errors]

@HerrCai0907 HerrCai0907 force-pushed the 103499-feat-clang-tidy-avoid-limits-on-number-of-sources-by-accepting-a-params-file branch from 9d26de4 to f76fc26 Compare December 19, 2024 16:00
@@ -553,6 +555,20 @@ static llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> createBaseFS() {

int clangTidyMain(int argc, const char **argv) {
llvm::InitLLVM X(argc, argv);
SmallVector<const char *> Args{argv, argv + argc};

llvm::BumpPtrAllocator Alloc;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a one-line comment describing what this block of code does/is meant for? Even better, can it be put into its own function?

Copy link
Contributor Author

@HerrCai0907 HerrCai0907 Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract to another function is less benefit because variable lifetime issue. The allocator must have same lifetime with argc
and argv.

Copy link
Contributor

@carlosgalvezp carlosgalvezp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but I had a couple of comments

Copy link
Contributor

@5chmidti 5chmidti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is not using llvm::cl, it is not automatically documented in --help. Maybe some sort of comment inside ClangTidyHelp would help with the discoverability from the CLI.

@carlosgalvezp
Copy link
Contributor

I had the realization now that as a user I would like this to be supported:

clang-tidy @params

Where params can contain:

<clang-tidy-flags> foo.cpp -- <compiler flags>

This would make it also consistent with clang, where you can do clang @params.

Would this be feasible?

@HerrCai0907
Copy link
Contributor Author

HerrCai0907 commented Dec 20, 2024

I had the realization now that as a user I would like this to be supported:

clang-tidy @params

Where params can contain:

<clang-tidy-flags> foo.cpp -- <compiler flags>

This would make it also consistent with clang, where you can do clang @params.

Would this be feasible?

Full supporting is hard. But we can do like this:

in build/sources

build/a.cpp
build/init.cpp

in build/options

-std=c++23
-isysroot
/Library/Developer/CommandLineTools/SDKs/MacOSX15.sdk

Then you can run clang and clang-tidy with

 build/release/bin/clang @build/sources @build/options
 build/release/bin/clang-tidy @build/sources -- @build/options

Actually I think it is very helpful to bazel user since bazel will generate this parameters file. 😆

@HerrCai0907 HerrCai0907 merged commit 2ff614a into llvm:main Dec 24, 2024
9 checks passed
@HerrCai0907 HerrCai0907 deleted the 103499-feat-clang-tidy-avoid-limits-on-number-of-sources-by-accepting-a-params-file branch December 24, 2024 15:24
@llvm-ci
Copy link
Collaborator

llvm-ci commented Dec 24, 2024

LLVM Buildbot has detected a new failure on builder clang-ppc64le-linux-multistage running on ppc64le-clang-multistage-test while building clang-tools-extra at step 4 "build stage 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/76/builds/5609

Here is the relevant piece of the build log for the reference
Step 4 (build stage 1) failure: 'ninja' (failure)
...
[470/493] Creating library symlink lib/libclangTidyCppCoreGuidelinesModule.so
[471/493] Linking CXX shared library lib/libclangTidyBugproneModule.so.20.0git
[472/493] Creating library symlink lib/libclangTidyBugproneModule.so
[473/493] Linking CXX shared library lib/libclangTidyHICPPModule.so.20.0git
[474/493] Creating library symlink lib/libclangTidyHICPPModule.so
[475/493] Linking CXX shared library lib/libclangTidyCERTModule.so.20.0git
[476/493] Creating library symlink lib/libclangTidyCERTModule.so
[477/493] Linking CXX shared library lib/libclangTidyPlugin.so.20.0git
[478/493] Creating library symlink lib/libclangTidyPlugin.so
[479/493] Linking CXX shared library lib/libclangTidyMain.so.20.0git
FAILED: lib/libclangTidyMain.so.20.0git 
: && /usr/lib64/ccache/c++ -fPIC -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -fno-lifetime-dse -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-uninitialized -Wno-nonnull -Wno-class-memaccess -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -Wno-misleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG  -Wl,-z,defs -Wl,-z,nodelete   -Wl,-rpath-link,/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-multistage-test/clang-ppc64le-multistage/stage1/./lib  -Wl,--gc-sections -shared -Wl,-soname,libclangTidyMain.so.20.0git -o lib/libclangTidyMain.so.20.0git tools/clang/tools/extra/clang-tidy/tool/CMakeFiles/obj.clangTidyMain.dir/ClangTidyMain.cpp.o  -Wl,-rpath,"\$ORIGIN/../lib:/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-multistage-test/clang-ppc64le-multistage/stage1/lib:"  lib/libclangTidyAndroidModule.so.20.0git  lib/libclangTidyAbseilModule.so.20.0git  lib/libclangTidyAlteraModule.so.20.0git  lib/libclangTidyBoostModule.so.20.0git  lib/libclangTidyCERTModule.so.20.0git  lib/libclangTidyConcurrencyModule.so.20.0git  lib/libclangTidyDarwinModule.so.20.0git  lib/libclangTidyFuchsiaModule.so.20.0git  lib/libclangTidyHICPPModule.so.20.0git  lib/libclangTidyLinuxKernelModule.so.20.0git  lib/libclangTidyLLVMModule.so.20.0git  lib/libclangTidyLLVMLibcModule.so.20.0git  lib/libclangTidyObjCModule.so.20.0git  lib/libclangTidyOpenMPModule.so.20.0git  lib/libclangTidyPortabilityModule.so.20.0git  lib/libclangTidyZirconModule.so.20.0git  lib/libclangTidyMPIModule.so.20.0git  lib/libLLVMAArch64AsmParser.so.20.0git  lib/libLLVMAMDGPUAsmParser.so.20.0git  lib/libLLVMARMAsmParser.so.20.0git  lib/libLLVMAVRAsmParser.so.20.0git  lib/libLLVMBPFAsmParser.so.20.0git  lib/libLLVMHexagonAsmParser.so.20.0git  lib/libLLVMLanaiAsmParser.so.20.0git  lib/libLLVMLoongArchAsmParser.so.20.0git  lib/libLLVMMipsAsmParser.so.20.0git  lib/libLLVMMSP430AsmParser.so.20.0git  lib/libLLVMPowerPCAsmParser.so.20.0git  lib/libLLVMRISCVAsmParser.so.20.0git  lib/libLLVMSparcAsmParser.so.20.0git  lib/libLLVMSystemZAsmParser.so.20.0git  lib/libLLVMVEAsmParser.so.20.0git  lib/libLLVMWebAssemblyAsmParser.so.20.0git  lib/libLLVMX86AsmParser.so.20.0git  lib/libLLVMAArch64Desc.so.20.0git  lib/libLLVMAMDGPUDesc.so.20.0git  lib/libLLVMARMDesc.so.20.0git  lib/libLLVMAVRDesc.so.20.0git  lib/libLLVMBPFDesc.so.20.0git  lib/libLLVMHexagonDesc.so.20.0git  lib/libLLVMLanaiDesc.so.20.0git  lib/libLLVMLoongArchDesc.so.20.0git  lib/libLLVMMipsDesc.so.20.0git  lib/libLLVMMSP430Desc.so.20.0git  lib/libLLVMNVPTXDesc.so.20.0git  lib/libLLVMPowerPCDesc.so.20.0git  lib/libLLVMRISCVDesc.so.20.0git  lib/libLLVMSparcDesc.so.20.0git  lib/libLLVMSystemZDesc.so.20.0git  lib/libLLVMVEDesc.so.20.0git  lib/libLLVMWebAssemblyDesc.so.20.0git  lib/libLLVMX86Desc.so.20.0git  lib/libLLVMXCoreDesc.so.20.0git  lib/libLLVMAArch64Info.so.20.0git  lib/libLLVMAMDGPUInfo.so.20.0git  lib/libLLVMARMInfo.so.20.0git  lib/libLLVMAVRInfo.so.20.0git  lib/libLLVMBPFInfo.so.20.0git  lib/libLLVMHexagonInfo.so.20.0git  lib/libLLVMLanaiInfo.so.20.0git  lib/libLLVMLoongArchInfo.so.20.0git  lib/libLLVMMipsInfo.so.20.0git  lib/libLLVMMSP430Info.so.20.0git  lib/libLLVMNVPTXInfo.so.20.0git  lib/libLLVMPowerPCInfo.so.20.0git  lib/libLLVMRISCVInfo.so.20.0git  lib/libLLVMSparcInfo.so.20.0git  lib/libLLVMSystemZInfo.so.20.0git  lib/libLLVMVEInfo.so.20.0git  lib/libLLVMWebAssemblyInfo.so.20.0git  lib/libLLVMX86Info.so.20.0git  lib/libLLVMXCoreInfo.so.20.0git  lib/libclangTidyBugproneModule.so.20.0git  lib/libclangTidyCppCoreGuidelinesModule.so.20.0git  lib/libclangTidyGoogleModule.so.20.0git  lib/libclangTidyMiscModule.so.20.0git  lib/libclangTidyModernizeModule.so.20.0git  lib/libclangTidyPerformanceModule.so.20.0git  lib/libclangTidyReadabilityModule.so.20.0git  lib/
chers.so.20.0git  lib/libclangAST.so.20.0git  lib/libclangBasic.so.20.0git  lib/libLLVMFrontendOpenMP.so.20.0git  lib/libLLVMSupport.so.20.0git  -Wl,-rpath-link,/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-multistage-test/clang-ppc64le-multistage/stage1/lib && :
tools/clang/tools/extra/clang-tidy/tool/CMakeFiles/obj.clangTidyMain.dir/ClangTidyMain.cpp.o: In function `clang::tidy::clangTidyMain(int, char const**)':
ClangTidyMain.cpp:(.text._ZN5clang4tidy13clangTidyMainEiPPKc+0x134): undefined reference to `llvm::sys::getProcessTriple[abi:cxx11]()'
ClangTidyMain.cpp:(.text._ZN5clang4tidy13clangTidyMainEiPPKc+0x150): undefined reference to `llvm::Triple::Triple(llvm::Twine const&)'
collect2: error: ld returned 1 exit status
[480/493] Linking CXX shared library lib/libclangDaemon.so.20.0git
[481/493] Linking CXX shared library lib/libclang-cpp.so.20.0git
ninja: build stopped: subcommand failed.

HerrCai0907 added a commit to HerrCai0907/llvm-project that referenced this pull request Dec 24, 2024
Fix build issue introduced in llvm#120547
HerrCai0907 added a commit that referenced this pull request Dec 25, 2024
pranavk added a commit that referenced this pull request Dec 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: clang-tidy: avoid limits on number of sources by accepting a params file
6 participants