[libc++] Add missing %{flags} substitution to clang-tidy#171689
[libc++] Add missing %{flags} substitution to clang-tidy#171689
Conversation
Flags that should be used both for compiling and for linking
are provided through the %{flags} substitution. Our clang-tidy
tests should be using them, not only %{compile_flags}.
|
@llvm/pr-subscribers-libcxx Author: Louis Dionne (ldionne) ChangesFlags that should be used both for compiling and for linking are provided through the %{flags} substitution. Our clang-tidy tests should be using them, not only %{compile_flags}. Full diff: https://github.com/llvm/llvm-project/pull/171689.diff 1 Files Affected:
diff --git a/libcxx/test/libcxx/clang_tidy.gen.py b/libcxx/test/libcxx/clang_tidy.gen.py
index 16c90c3ef7130..b46cc72c08dfd 100644
--- a/libcxx/test/libcxx/clang_tidy.gen.py
+++ b/libcxx/test/libcxx/clang_tidy.gen.py
@@ -27,7 +27,7 @@
{lit_header_undeprecations.get(header, '')}
// TODO: run clang-tidy with modules enabled once they are supported
-// RUN: %{{clang-tidy}} %s --warnings-as-errors=* -header-filter=.* --config-file=%{{libcxx-dir}}/.clang-tidy --load=%{{test-tools-dir}}/clang_tidy_checks/libcxx-tidy.plugin -- -Wweak-vtables %{{compile_flags}} -fno-modules
+// RUN: %{{clang-tidy}} %s --warnings-as-errors=* -header-filter=.* --config-file=%{{libcxx-dir}}/.clang-tidy --load=%{{test-tools-dir}}/clang_tidy_checks/libcxx-tidy.plugin -- -Wweak-vtables %{{flags}} %{{compile_flags}} -fno-modules
#include <{header}>
""")
|
You can test this locally with the following command:darker --check --diff -r origin/main...HEAD libcxx/test/libcxx/clang_tidy.gen.py
View the diff from darker here.--- clang_tidy.gen.py 2025-12-11 17:32:36.000000 +0000
+++ clang_tidy.gen.py 2025-12-11 17:34:07.500648 +0000
@@ -32,6 +32,7 @@
// RUN: --config-file=%{{libcxx-dir}}/.clang-tidy \\
// RUN: --load=%{{test-tools-dir}}/clang_tidy_checks/libcxx-tidy.plugin \\
// RUN: -- -Wweak-vtables %{{flags}} %{{compile_flags}} -fno-modules
#include <{header}>
-""")
+"""
+ )
|
philnik777
left a comment
There was a problem hiding this comment.
FWIW IMO %{flags} is a bad name here. I expected %{compile_flags} to contain all flags that should be used when compiling, %{link_flags} to contain all flags that should be used when linking, and %{flags} to contain all flags that should be used when compiling and linking in one command.
Perhaps |
|
Yeah, I think so. |
|
Flags that should be used both for compiling and for linking are
provided through the %{flags} substitution. Our clang-tidy tests should
be using them, not only %{compile_flags}.
Flags that should be used both for compiling and for linking are
provided through the %{flags} substitution. Our clang-tidy tests should
be using them, not only %{compile_flags}.
Flags that should be used both for compiling and for linking are provided through the %{flags} substitution. Our clang-tidy tests should be using them, not only %{compile_flags}.