llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: mitchell (zeyi2) <details> <summary>Changes</summary> As of AI-Usage: Gemini CLI was used for pre-commit review and finding testcases that need to be updated. I'll add a ReleaseNotes entry later (I think they are reset now?) Closes [#<!-- -->133515](https://github.com/llvm/llvm-project/issues/133515) --- Patch is 20.11 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/175735.diff 10 Files Affected: - (modified) clang-tools-extra/test/clang-tidy/check_clang_tidy.py (+159-30) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h (+4-3) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/pass-by-value/header-with-fix.h (+1) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/pass-by-value/header.h (+3) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp (+2-9) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/pass-by-value-header.cpp (+5-9) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/pass-by-value-multi-fixes.cpp (+6-9) - (modified) clang-tools-extra/test/clang-tidy/checkers/performance/Inputs/unnecessary-value-param/header.h (+2) - (modified) clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-header.cpp (+4-5) - (modified) clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include.cpp (+3-2) ``````````diff diff --git a/clang-tools-extra/test/clang-tidy/check_clang_tidy.py b/clang-tools-extra/test/clang-tidy/check_clang_tidy.py index b173ecf4fbdca..2bbf04bbc143a 100755 --- a/clang-tools-extra/test/clang-tidy/check_clang_tidy.py +++ b/clang-tools-extra/test/clang-tidy/check_clang_tidy.py @@ -49,7 +49,7 @@ import re import subprocess import sys -from typing import List, Tuple +from typing import Dict, List, Sequence, Tuple def write_file(file_name: str, text: str) -> None: @@ -93,6 +93,7 @@ def __init__(self, args: argparse.Namespace, extra_args: List[str]) -> None: self.input_file_name = args.input_file_name self.check_name = args.check_name self.temp_file_name = args.temp_file_name + self.check_headers = args.check_headers self.original_file_name = self.temp_file_name + ".orig" self.expect_clang_tidy_error = args.expect_clang_tidy_error self.std = args.std @@ -121,6 +122,18 @@ def __init__(self, args: argparse.Namespace, extra_args: List[str]) -> None: self.clang_extra_args = self.clang_tidy_extra_args[i + 1 :] self.clang_tidy_extra_args = self.clang_tidy_extra_args[:i] + self.check_header_map: Dict[str, str] = {} + self.header_dir: str = self.temp_file_name + ".headers" + if self.check_headers: + self.check_header_map = { + os.path.normcase( + os.path.abspath(os.path.join(self.header_dir, os.path.basename(h))) + ): os.path.abspath(h) + for h in self.check_headers + } + + self.clang_extra_args.insert(0, "-I" + self.header_dir) + # If the test does not specify a config style, force an empty one; otherwise # auto-detection logic can discover a ".clang-tidy" file that is not related to # the test. @@ -196,15 +209,36 @@ def get_prefixes(self) -> None: ) assert expect_diagnosis or self.expect_no_diagnosis + def _sanitize_content(self, text: str) -> str: + return re.sub("// *CHECK-[A-Z0-9\\-]*:[^\r\n]*", "//", text) + + def _filter_prefixes(self, prefixes: Sequence[str], check_file: str) -> List[str]: + if check_file == self.input_file_name: + content = self.input_text + else: + with open(check_file, "r", encoding="utf-8") as f: + content = f.read() + return [p for p in prefixes if p in content] + def prepare_test_inputs(self) -> None: # Remove the contents of the CHECK lines to avoid CHECKs matching on # themselves. We need to keep the comments to preserve line numbers while # avoiding empty lines which could potentially trigger formatting-related # checks. - cleaned_test = re.sub("// *CHECK-[A-Z0-9\\-]*:[^\r\n]*", "//", self.input_text) + cleaned_test = self._sanitize_content(self.input_text) write_file(self.temp_file_name, cleaned_test) write_file(self.original_file_name, cleaned_test) + if self.check_headers: + os.makedirs(self.header_dir, exist_ok=True) + + for temp_header_path, header in self.check_header_map.items(): + with open(header, "r", encoding="utf-8") as f: + cleaned_header = self._sanitize_content(f.read()) + + write_file(temp_header_path, cleaned_header) + write_file(temp_header_path + ".orig", cleaned_header) + def run_clang_tidy(self) -> str: args = ( [ @@ -241,6 +275,18 @@ def run_clang_tidy(self) -> str: diff_output = try_run( ["diff", "-u", self.original_file_name, self.temp_file_name], False ) + if self.check_headers: + for temp_header_path in self.check_header_map: + diff_output += try_run( + [ + "diff", + "-u", + temp_header_path + ".orig", + temp_header_path, + ], + False, + ) + print("------------------------------ Fixes -----------------------------") print(diff_output) print("------------------------------------------------------------------") @@ -250,35 +296,57 @@ def check_no_diagnosis(self, clang_tidy_output: str) -> None: if clang_tidy_output != "": sys.exit("No diagnostics were expected, but found the ones above") - def check_fixes(self) -> None: - if self.has_check_fixes: - try_run( - [ - "FileCheck", - "--input-file=" + self.temp_file_name, - self.input_file_name, - "--check-prefixes=" + ",".join(self.fixes.prefixes), - ( - "--match-full-lines" - if not self.match_partial_fixes - else "--strict-whitespace" # Keeping past behavior. - ), - ] - ) + def check_fixes(self, input_file: str = "", check_file: str = "") -> None: + if not check_file and not self.has_check_fixes: + return - def check_messages(self, clang_tidy_output: str) -> None: - if self.has_check_messages: - messages_file = self.temp_file_name + ".msg" - write_file(messages_file, clang_tidy_output) - try_run( - [ - "FileCheck", - "-input-file=" + messages_file, - self.input_file_name, - "-check-prefixes=" + ",".join(self.messages.prefixes), - "-implicit-check-not={{warning|error}}:", - ] - ) + input_file = input_file or self.temp_file_name + check_file = check_file or self.input_file_name + active_prefixes = self._filter_prefixes(self.fixes.prefixes, check_file) + + if not active_prefixes: + return + + try_run( + [ + "FileCheck", + "--input-file=" + input_file, + check_file, + "--check-prefixes=" + ",".join(active_prefixes), + ( + "--match-full-lines" + if not self.match_partial_fixes + else "--strict-whitespace" # Keeping past behavior. + ), + ] + ) + + def check_messages( + self, + clang_tidy_output: str, + messages_file: str = "", + check_file: str = "", + ) -> None: + if not check_file and not self.has_check_messages: + return + + messages_file = messages_file or (self.temp_file_name + ".msg") + check_file = check_file or self.input_file_name + + active_prefixes = self._filter_prefixes(self.messages.prefixes, check_file) + if not active_prefixes: + return + + write_file(messages_file, clang_tidy_output) + try_run( + [ + "FileCheck", + "-input-file=" + messages_file, + check_file, + "-check-prefixes=" + ",".join(active_prefixes), + "-implicit-check-not={{warning|error}}:", + ] + ) def check_notes(self, clang_tidy_output: str) -> None: if self.has_check_notes: @@ -299,12 +367,60 @@ def check_notes(self, clang_tidy_output: str) -> None: ] ) + def check_header_messages(self, clang_tidy_output: str) -> str: + """ + Filters and verifies clang-tidy diagnostics for headers. + + - Input: The raw diagnostic output from clang-tidy. + - Output: The diagnostic output intended for the main file + verification. + + This function separates messages belonging to headers specified by + `-check-header', verifies them using FileCheck against the header's + own rules, and returns the rest for further processing. + """ + if not self.check_headers: + return clang_tidy_output + + header_messages: Dict[str, List[str]] = { + t: [] for t in self.check_header_map.keys() + } + remaining_lines: List[str] = [] + current_file: str = "" + + for line in clang_tidy_output.splitlines(keepends=True): + if re.match(r"^\d+ warnings? generated\.", line): + continue + # Matches the beginning of a clang-tidy diagnostic line, + # which starts with "file_path:line:col: ". + match = re.match(r"^(.+):\d+:\d+: ", line) + if match: + abs_path = os.path.normcase(os.path.abspath(match.group(1))) + current_file = abs_path if abs_path in header_messages else "" + + header_messages.get(current_file, remaining_lines).append(line) + + for temp_header, messages_list in header_messages.items(): + original_header = self.check_header_map[temp_header] + messages = "".join(messages_list) + if not messages: + continue + + self.check_messages( + messages, + messages_file=temp_header + ".msg", + check_file=original_header, + ) + + return "".join(remaining_lines) + def run(self) -> None: self.read_input() if self.export_fixes is None: self.get_prefixes() self.prepare_test_inputs() clang_tidy_output = self.run_clang_tidy() + clang_tidy_output = self.check_header_messages(clang_tidy_output) if self.expect_no_diagnosis: self.check_no_diagnosis(clang_tidy_output) elif self.export_fixes is None: @@ -312,6 +428,12 @@ def run(self) -> None: self.check_messages(clang_tidy_output) self.check_notes(clang_tidy_output) + for temp_header, original_header in self.check_header_map.items(): + self.check_fixes( + input_file=temp_header, + check_file=original_header, + ) + CPP_STANDARDS = [ "c++98", @@ -370,6 +492,13 @@ def parse_arguments() -> Tuple[argparse.Namespace, List[str]]: type=csv, help="comma-separated list of FileCheck suffixes", ) + parser.add_argument( + "-check-header", + action="append", + dest="check_headers", + default=[], + help="Header files to check", + ) parser.add_argument( "-export-fixes", default=None, diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h index 703d7e05ca2e9..0d748a81c0067 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h @@ -1,8 +1,9 @@ +// CHECK-MESSAGES-NORMAL: :[[@LINE+1]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces] namespace nn1 { namespace nn2 { -// CHECK-FIXES: namespace nn1::nn2 +// CHECK-FIXES-NORMAL: namespace nn1::nn2 { void t(); } // namespace nn2 } // namespace nn1 -// CHECK-FIXES: void t(); -// CHECK-FIXES-NEXT: } // namespace nn1::nn2 +// CHECK-FIXES-NORMAL: void t(); +// CHECK-FIXES-NORMAL-NEXT: } // namespace nn1::nn2 diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/pass-by-value/header-with-fix.h b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/pass-by-value/header-with-fix.h index 62609e25f682a..98512f411fe61 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/pass-by-value/header-with-fix.h +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/pass-by-value/header-with-fix.h @@ -4,5 +4,6 @@ struct S { }; struct Foo { Foo(const S &s); + // CHECK-FIXES: Foo(S s); S s; }; diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/pass-by-value/header.h b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/pass-by-value/header.h index c2103cb3fc7a1..e27104e70ac0b 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/pass-by-value/header.h +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/pass-by-value/header.h @@ -6,5 +6,8 @@ class ThreadId { struct A { A(const ThreadId &tid) : threadid(tid) {} + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass by value and use std::move [modernize-pass-by-value] + // CHECK-FIXES: #include <utility> + // CHECK-FIXES: A(ThreadId tid) : threadid(std::move(tid)) {} ThreadId threadid; }; diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp index 35cb5503ba245..58ae66d0b389f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp @@ -1,14 +1,7 @@ -// RUN: mkdir -p %t.dir -// RUN: cp %S/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h %t.dir/modernize-concat-nested-namespaces.h -// RUN: %check_clang_tidy -std=c++17 -check-suffix=NORMAL %s modernize-concat-nested-namespaces %t.dir/code -- -header-filter=".*" -- -I %t.dir -// RUN: FileCheck -input-file=%t.dir/modernize-concat-nested-namespaces.h %S/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h -check-prefix=CHECK-FIXES -// Restore header file and re-run with c++20: -// RUN: cp %S/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h %t.dir/modernize-concat-nested-namespaces.h -// RUN: %check_clang_tidy -std=c++20 -check-suffixes=NORMAL,CPP20 %s modernize-concat-nested-namespaces %t.dir/code -- -header-filter=".*" -- -I %t.dir -// RUN: FileCheck -input-file=%t.dir/modernize-concat-nested-namespaces.h %S/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h -check-prefix=CHECK-FIXES +// RUN: %check_clang_tidy -std=c++17 -check-suffix=NORMAL -check-header %S/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h %s modernize-concat-nested-namespaces %t -- -header-filter=".*" -- -I %S/Inputs/concat-nested-namespaces +// RUN: %check_clang_tidy -std=c++20 -check-suffixes=NORMAL,CPP20 -check-header %S/Inputs/concat-nested-namespaces/modernize-concat-nested-namespaces.h %s modernize-concat-nested-namespaces %t -- -header-filter=".*" -- -I %S/Inputs/concat-nested-namespaces #include "modernize-concat-nested-namespaces.h" -// CHECK-MESSAGES-NORMAL-DAG: modernize-concat-nested-namespaces.h:1:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces] namespace n1 {} diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/pass-by-value-header.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/pass-by-value-header.cpp index 461a6378d99c0..79165e51f9e14 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/pass-by-value-header.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/pass-by-value-header.cpp @@ -1,10 +1,6 @@ -// RUN: mkdir -p %t.dir -// RUN: cp %S/Inputs/pass-by-value/header.h %t.dir/pass-by-value-header.h -// RUN: clang-tidy %s -checks='-*,modernize-pass-by-value' -header-filter='.*' -fix -- -std=c++11 -I %t.dir | FileCheck %s -check-prefix=CHECK-MESSAGES -implicit-check-not="{{warning|error}}:" -// RUN: FileCheck -input-file=%t.dir/pass-by-value-header.h %s -check-prefix=CHECK-FIXES -// FIXME: Make the test work in all language modes. +// RUN: %check_clang_tidy -check-header %S/Inputs/pass-by-value/header.h \ +// RUN: %s modernize-pass-by-value %t -- -header-filter='.*' \ +// RUN: -- -I %S/Inputs/pass-by-value -std=c++11 -#include "pass-by-value-header.h" -// CHECK-MESSAGES: :8:5: warning: pass by value and use std::move [modernize-pass-by-value] -// CHECK-FIXES: #include <utility> -// CHECK-FIXES: A(ThreadId tid) : threadid(std::move(tid)) {} +#include "header.h" +// FIXME: Make the test work in all language modes. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/pass-by-value-multi-fixes.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/pass-by-value-multi-fixes.cpp index b77c74be02f5f..c5e6e0be30b8f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/pass-by-value-multi-fixes.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/pass-by-value-multi-fixes.cpp @@ -1,13 +1,10 @@ -// RUN: mkdir -p %t.dir -// RUN: cat %S/Inputs/pass-by-value/header-with-fix.h > %t.dir/pass-by-value-header-with-fix.h -// RUN: sed -e 's#//.*$##' %s > %t.dir/code.cpp -// RUN: clang-tidy %t.dir/code.cpp -checks='-*,modernize-pass-by-value' -header-filter='.*' -fix -- -std=c++11 -I %t.dir | FileCheck %s -check-prefix=CHECK-MESSAGES -implicit-check-not="{{warning|error}}:" -// RUN: FileCheck -input-file=%t.dir/code.cpp %s -check-prefix=CHECK-FIXES -// RUN: FileCheck -input-file=%t.dir/pass-by-value-header-with-fix.h %s -check-prefix=CHECK-HEADER-FIXES +// RUN: %check_clang_tidy -check-header %S/Inputs/pass-by-value/header-with-fix.h \ +// RUN: %s modernize-pass-by-value %t -- -header-filter='.*' \ +// RUN: -- -I %S/Inputs/pass-by-value -std=c++11 -#include "pass-by-value-header-with-fix.h" -// CHECK-HEADER-FIXES: Foo(S s); +#include "header-with-fix.h" + +// CHECK-MESSAGES: :[[@LINE+1]]:10: warning: pass by value and use std::move [modernize-pass-by-value] Foo::Foo(const S &s) : s(s) {} -// CHECK-MESSAGES: :10:10: warning: pass by value and use std::move [modernize-pass-by-value] // CHECK-FIXES: #include <utility> // CHECK-FIXES: Foo::Foo(S s) : s(std::move(s)) {} diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/Inputs/unnecessary-value-param/header.h b/clang-tools-extra/test/clang-tidy/checkers/performance/Inputs/unnecessary-value-param/header.h index d6f6e65ace79d..32b3d3374bc59 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/performance/Inputs/unnecessary-value-param/header.h +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/Inputs/unnecessary-value-param/header.h @@ -7,9 +7,11 @@ struct ABC { int f1(int n, ABC v1, ABC v2); // line 9 +// CHECK-FIXES: int f1(int n, const ABC& v1, const ABC& v2); // line 9 int f1(int n, ABC v1); // line 11 void f2( int n, ABC v2); // line 15 +// CHECK-FIXES: void f2( int n, const ABC& v2); // line 15 diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-header.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-header.cpp index a7fc1eace67c5..ded66392e4af5 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-header.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-header.cpp @@ -1,8 +1,7 @@ -// RUN: rm -rf %t -// RUN: mkdir %t -// RUN: cp %S/Inputs/unnecessary-value-param/header.h %t/header.h -// RUN: %check_clang_tidy %s performance-unnecessary-value-param %t/temp -- -- -I %t -// RUN: diff %t/header.h %S/Inputs/unnecessary-value-param/header-fixed.h +// RUN: %check_clang_tidy -check-header %S/Inputs/unnecessary-value-param/header.h \ +// RUN: %s performance-unnecessary-value-param %t -- \ +// RUN: -header-filter='.*' \ +// RUN: -- -I %S/Inputs/unnecessary-value-param #include "header.h" diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include.cpp index c452f69fad07d..8483e895bb635 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include.cpp @@ -1,5 +1,6 @@ -// RUN: %check_clang_tidy %s readability-duplicate-include %t -- \ -// RUN: -header-filter='' \ +// RUN: %check_clang_tidy -check-header %S/Inputs/duplicate-include/duplicate-include.h \ +// RUN: %s readability-duplicate-include %t -- \ +// RUN: -header-... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/175735 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
