From 5148727e060c947a3fc0ebb942e4ce2dc2dad2e7 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Sat, 13 Jul 2024 12:50:17 -0500 Subject: [PATCH 01/46] Create simple warning check tool and add to ubuntu build and test job --- .github/workflows/reusable-ubuntu.yml | 4 +- Tools/build/.warnignore_ubuntu | 3 + Tools/build/check_warnings.py | 181 ++++++++++++++++++++++++++ 3 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 Tools/build/.warnignore_ubuntu create mode 100644 Tools/build/check_warnings.py diff --git a/.github/workflows/reusable-ubuntu.yml b/.github/workflows/reusable-ubuntu.yml index 54d7765d159d49..3a6f2b910d0da0 100644 --- a/.github/workflows/reusable-ubuntu.yml +++ b/.github/workflows/reusable-ubuntu.yml @@ -74,10 +74,12 @@ jobs: ${{ fromJSON(inputs.free-threading) && '--disable-gil' || '' }} - name: Build CPython out-of-tree working-directory: ${{ env.CPYTHON_BUILDDIR }} - run: make -j4 + run: make -j4 &> compiler_output.txt - name: Display build info working-directory: ${{ env.CPYTHON_BUILDDIR }} run: make pythoninfo + - name: Check compiler warnings + run: python Tools/build/check_warnings.py --compiler-output-file-path=${{ env.CPYTHON_BUILDDIR }}/compiler_output.txt --warning-ignore-file-path ${GITHUB_WORKSPACE}/Tools/build/.warningignore_ubuntu - name: Remount sources writable for tests # some tests write to srcdir, lack of pyc files slows down testing run: sudo mount $CPYTHON_RO_SRCDIR -oremount,rw diff --git a/Tools/build/.warnignore_ubuntu b/Tools/build/.warnignore_ubuntu new file mode 100644 index 00000000000000..8242c8d17c89fb --- /dev/null +++ b/Tools/build/.warnignore_ubuntu @@ -0,0 +1,3 @@ +# Files listed will be ignored by the compiler warning checker +# for the Ubuntu/build and test job. +# Keep lines sorted lexicographically to help avoid merge conflicts. diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py new file mode 100644 index 00000000000000..b2119b40f3de6e --- /dev/null +++ b/Tools/build/check_warnings.py @@ -0,0 +1,181 @@ +#!/usr/bin/env python3 +import sys +from pathlib import Path +import argparse +import json + + +def extract_json_objects(compiler_output: str) -> list[dict]: + + return json_objects + + +def extract_warnings_from_compiler_output(compiler_output: str) -> list[dict]: + """ + Extracts warnings from the compiler output when using -fdiagnostics-format=json + + Compiler output as a whole is not a valid json document, but includes many json + objects and may include other output that is not json. + """ + # Extract JSON objects from the raw compiler output + compiler_output_json_objects = [] + stack = [] + start_index = None + for index, char in enumerate(compiler_output): + if char == '[': + if len(stack) == 0: + start_index = index # Start of a new JSON array + stack.append(char) + elif char == ']': + if len(stack) > 0: + stack.pop() + if len(stack) == 0 and start_index is not None: + try: + json_data = json.loads(compiler_output[start_index:index+1]) + compiler_output_json_objects.extend(json_data) + start_index = None + except json.JSONDecodeError: + continue # Skip malformed JSON + + compiler_warnings = [entry for entry in compiler_output_json_objects if entry.get('kind') == 'warning'] + + return compiler_warnings + + + +def get_unexpected_warnings( + warnings: list[dict], + files_with_expected_warnings: set[str], +) -> int: + """ + Fails if there are unexpected warnings + """ + unexpected_warnings = [] + for warning in warnings: + locations = warning['locations'] + for location in locations: + for key in ['caret', 'start', 'end']: + if key in location: + filename = location[key]['file'] + if filename not in files_with_expected_warnings: + unexpected_warnings.append(warning) + + if unexpected_warnings: + print("Unexpected warnings:") + for warning in unexpected_warnings: + print(warning) + return 1 + + + return 0 + + +def get_unexpected_improvements( + warnings: list[dict], + files_with_expected_warnings: set[str], +) -> int: + """ + Fails if files that were expected to have warnings have no warnings + """ + + # Create set of files with warnings + files_with_ewarnings = set() + for warning in warnings: + locations = warning['locations'] + for location in locations: + for key in ['caret', 'start', 'end']: + if key in location: + filename = location[key]['file'] + files_with_ewarnings.add(filename) + + + + unexpected_improvements = [] + for filename in files_with_expected_warnings: + if filename not in files_with_ewarnings: + unexpected_improvements.append(filename) + + if unexpected_improvements: + print("Unexpected improvements:") + for filename in unexpected_improvements: + print(filename) + return 1 + + + return 0 + + + + + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser() + parser.add_argument( + "--compiler-output-file-path", + type=str, + required=True, + help="Path to the file" + ) + parser.add_argument( + "--warning-ignore-file-path", + type=str, + required=True, + help="Path to the warning ignore file" + ) + + + args = parser.parse_args(argv) + + exit_code = 0 + + # Check that the compiler output file is a valid path + if not Path(args.compiler_output_file_path).is_file(): + print(f"Compiler output file does not exist: {args.compiler_output_file_path}") + return 1 + + # Check that the warning ignore file is a valid path + if not Path(args.warning_ignore_file_path).is_file(): + print(f"Warning ignore file does not exist: {args.warning_ignore_file_path}") + return 1 + + with Path(args.compiler_output_file_path).open(encoding="UTF-8") as f: + compiler_output_file_contents = f.read() + + with Path(args.warning_ignore_file_path).open(encoding="UTF-8") as clean_files: + files_with_expected_warnings = { + filename.strip() + for filename in clean_files + if filename.strip() and not filename.startswith("#") + } + + + if len(compiler_output_file_contents) > 0: + print("Successfully got compiler output") + else: + exit_code = 1 + + if len(files_with_expected_warnings) > 0: + print("we have some exceptions") + + + warnings = extract_warnings_from_compiler_output(compiler_output_file_contents) + + exit_code = get_unexpected_warnings(warnings, files_with_expected_warnings) + + exit_code = get_unexpected_improvements(warnings, files_with_expected_warnings) + + + + + return exit_code + + + + + + + + +if __name__ == "__main__": + sys.exit(main()) \ No newline at end of file From 3bd5a10a9b57d3eafb94bc4a9a739f5834094593 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Sat, 13 Jul 2024 13:18:25 -0500 Subject: [PATCH 02/46] Add flags to check warnings script to fail on regression or improvement --- Tools/build/check_warnings.py | 81 ++++++++++++++--------------------- 1 file changed, 33 insertions(+), 48 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index b2119b40f3de6e..daa60fb5282c9f 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -1,15 +1,13 @@ #!/usr/bin/env python3 +""" +Parses compiler output with -fdiagnostics-format=json and checks that warnings exist +only in files that are expected to have warnings. +""" import sys from pathlib import Path import argparse import json - -def extract_json_objects(compiler_output: str) -> list[dict]: - - return json_objects - - def extract_warnings_from_compiler_output(compiler_output: str) -> list[dict]: """ Extracts warnings from the compiler output when using -fdiagnostics-format=json @@ -41,14 +39,13 @@ def extract_warnings_from_compiler_output(compiler_output: str) -> list[dict]: return compiler_warnings - - def get_unexpected_warnings( warnings: list[dict], files_with_expected_warnings: set[str], ) -> int: """ - Fails if there are unexpected warnings + Returns failure status if warnings discovered in list of warnings are associated with a file + that is not found in the list of files with expected warnings """ unexpected_warnings = [] for warning in warnings: @@ -66,7 +63,6 @@ def get_unexpected_warnings( print(warning) return 1 - return 0 @@ -75,24 +71,23 @@ def get_unexpected_improvements( files_with_expected_warnings: set[str], ) -> int: """ - Fails if files that were expected to have warnings have no warnings + Returns failure status if there are no warnings in the list of warnings for a file + that is in the list of files with expected warnings """ # Create set of files with warnings - files_with_ewarnings = set() + files_with_warnings = set() for warning in warnings: locations = warning['locations'] for location in locations: for key in ['caret', 'start', 'end']: if key in location: filename = location[key]['file'] - files_with_ewarnings.add(filename) - - + files_with_warnings.add(filename) unexpected_improvements = [] for filename in files_with_expected_warnings: - if filename not in files_with_ewarnings: + if filename not in files_with_warnings: unexpected_improvements.append(filename) if unexpected_improvements: @@ -101,21 +96,15 @@ def get_unexpected_improvements( print(filename) return 1 - return 0 - - - - - def main(argv: list[str] | None = None) -> int: parser = argparse.ArgumentParser() parser.add_argument( "--compiler-output-file-path", type=str, required=True, - help="Path to the file" + help="Path to the compiler output file" ) parser.add_argument( "--warning-ignore-file-path", @@ -123,7 +112,18 @@ def main(argv: list[str] | None = None) -> int: required=True, help="Path to the warning ignore file" ) - + parser.add_argument( + "--fail-on-regression", + action="store_true", + default=False, + help="Flag to fail if new warnings are found" + ) + parser.add_argument( + "--fail-on-improvement", + action="store_true", + default=False, + help="Flag to fail if files that were expected to have warnings have no warnings" + ) args = parser.parse_args(argv) @@ -133,12 +133,12 @@ def main(argv: list[str] | None = None) -> int: if not Path(args.compiler_output_file_path).is_file(): print(f"Compiler output file does not exist: {args.compiler_output_file_path}") return 1 - + # Check that the warning ignore file is a valid path if not Path(args.warning_ignore_file_path).is_file(): print(f"Warning ignore file does not exist: {args.warning_ignore_file_path}") return 1 - + with Path(args.compiler_output_file_path).open(encoding="UTF-8") as f: compiler_output_file_contents = f.read() @@ -149,33 +149,18 @@ def main(argv: list[str] | None = None) -> int: if filename.strip() and not filename.startswith("#") } - - if len(compiler_output_file_contents) > 0: - print("Successfully got compiler output") - else: - exit_code = 1 - - if len(files_with_expected_warnings) > 0: - print("we have some exceptions") - + if not len(compiler_output_file_contents) > 0:exit_code = 1 warnings = extract_warnings_from_compiler_output(compiler_output_file_contents) - exit_code = get_unexpected_warnings(warnings, files_with_expected_warnings) - - exit_code = get_unexpected_improvements(warnings, files_with_expected_warnings) - - + status = get_unexpected_warnings(warnings, files_with_expected_warnings) + if args.fail_on_regression: exit_code |= status + status = get_unexpected_improvements(warnings, files_with_expected_warnings) + if args.fail_on_improvement: exit_code |= status + print("Returning exit code: ", exit_code) return exit_code - - - - - - - if __name__ == "__main__": - sys.exit(main()) \ No newline at end of file + sys.exit(main()) From 68130035e6f1f9ca16af4defe38ff12d46f5eb28 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Sat, 13 Jul 2024 13:34:59 -0500 Subject: [PATCH 03/46] Remove redundant comment --- Tools/build/check_warnings.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index daa60fb5282c9f..2e2c6b5b8a9d45 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -15,7 +15,6 @@ def extract_warnings_from_compiler_output(compiler_output: str) -> list[dict]: Compiler output as a whole is not a valid json document, but includes many json objects and may include other output that is not json. """ - # Extract JSON objects from the raw compiler output compiler_output_json_objects = [] stack = [] start_index = None From ab4d754775810ea8669868e00a30e5097e6c37ea Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Sat, 13 Jul 2024 14:24:34 -0500 Subject: [PATCH 04/46] Rename warnigore file to warningignore --- Tools/build/{.warnignore_ubuntu => .warningignore_ubuntu} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename Tools/build/{.warnignore_ubuntu => .warningignore_ubuntu} (100%) diff --git a/Tools/build/.warnignore_ubuntu b/Tools/build/.warningignore_ubuntu similarity index 100% rename from Tools/build/.warnignore_ubuntu rename to Tools/build/.warningignore_ubuntu From 615d2282aa9baa5183acedb47bf950633b085486 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Sat, 13 Jul 2024 16:13:45 -0500 Subject: [PATCH 05/46] Use regex to extract json arrays --- Tools/build/check_warnings.py | 63 +++++++++++++++-------------------- 1 file changed, 27 insertions(+), 36 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 2e2c6b5b8a9d45..b655b11f707449 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -7,6 +7,7 @@ from pathlib import Path import argparse import json +import re def extract_warnings_from_compiler_output(compiler_output: str) -> list[dict]: """ @@ -15,26 +16,17 @@ def extract_warnings_from_compiler_output(compiler_output: str) -> list[dict]: Compiler output as a whole is not a valid json document, but includes many json objects and may include other output that is not json. """ - compiler_output_json_objects = [] - stack = [] - start_index = None - for index, char in enumerate(compiler_output): - if char == '[': - if len(stack) == 0: - start_index = index # Start of a new JSON array - stack.append(char) - elif char == ']': - if len(stack) > 0: - stack.pop() - if len(stack) == 0 and start_index is not None: - try: - json_data = json.loads(compiler_output[start_index:index+1]) - compiler_output_json_objects.extend(json_data) - start_index = None - except json.JSONDecodeError: - continue # Skip malformed JSON - - compiler_warnings = [entry for entry in compiler_output_json_objects if entry.get('kind') == 'warning'] + + # Regex to find json arrays at the top level of the file in the compiler output + json_arrays = re.findall(r'\[(?:[^\[\]]|\[(?:[^\[\]]|\[[^\[\]]*\])*\])*\]', compiler_output) + compiler_warnings = [] + for array in json_arrays: + try: + json_data = json.loads(array) + json_objects_in_array = [entry for entry in json_data] + compiler_warnings.extend([entry for entry in json_objects_in_array if entry.get('kind') == 'warning']) + except json.JSONDecodeError: + continue # Skip malformed JSON return compiler_warnings @@ -52,8 +44,9 @@ def get_unexpected_warnings( for location in locations: for key in ['caret', 'start', 'end']: if key in location: - filename = location[key]['file'] - if filename not in files_with_expected_warnings: + file = location[key]['file'] + file = file.lstrip('./') # Remove leading curdir if present + if file not in files_with_expected_warnings: unexpected_warnings.append(warning) if unexpected_warnings: @@ -81,18 +74,19 @@ def get_unexpected_improvements( for location in locations: for key in ['caret', 'start', 'end']: if key in location: - filename = location[key]['file'] - files_with_warnings.add(filename) + file = location[key]['file'] + file = file.lstrip('./') # Remove leading curdir if present + files_with_warnings.add(file) unexpected_improvements = [] - for filename in files_with_expected_warnings: - if filename not in files_with_warnings: - unexpected_improvements.append(filename) + for file in files_with_expected_warnings: + if file not in files_with_warnings: + unexpected_improvements.append(file) if unexpected_improvements: print("Unexpected improvements:") - for filename in unexpected_improvements: - print(filename) + for file in unexpected_improvements: + print(file) return 1 return 0 @@ -143,13 +137,11 @@ def main(argv: list[str] | None = None) -> int: with Path(args.warning_ignore_file_path).open(encoding="UTF-8") as clean_files: files_with_expected_warnings = { - filename.strip() - for filename in clean_files - if filename.strip() and not filename.startswith("#") + file.strip() + for file in clean_files + if file.strip() and not file.startswith("#") } - if not len(compiler_output_file_contents) > 0:exit_code = 1 - warnings = extract_warnings_from_compiler_output(compiler_output_file_contents) status = get_unexpected_warnings(warnings, files_with_expected_warnings) @@ -157,8 +149,7 @@ def main(argv: list[str] | None = None) -> int: status = get_unexpected_improvements(warnings, files_with_expected_warnings) if args.fail_on_improvement: exit_code |= status - - print("Returning exit code: ", exit_code) + return exit_code if __name__ == "__main__": From fc0a60ba7e6a902b0abd17faa30b18c97f1ad90c Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Sat, 13 Jul 2024 16:16:29 -0500 Subject: [PATCH 06/46] Trim whitespace --- Tools/build/check_warnings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index b655b11f707449..16075d855a6e39 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -149,7 +149,7 @@ def main(argv: list[str] | None = None) -> int: status = get_unexpected_improvements(warnings, files_with_expected_warnings) if args.fail_on_improvement: exit_code |= status - + return exit_code if __name__ == "__main__": From 7793d80836c9fa1adc7841acf25f4f135fdeaa99 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Sat, 13 Jul 2024 16:30:09 -0500 Subject: [PATCH 07/46] Test on github unexpected improvement --- Tools/build/.warningignore_ubuntu | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tools/build/.warningignore_ubuntu b/Tools/build/.warningignore_ubuntu index 8242c8d17c89fb..83dccf0eb215f6 100644 --- a/Tools/build/.warningignore_ubuntu +++ b/Tools/build/.warningignore_ubuntu @@ -1,3 +1,5 @@ # Files listed will be ignored by the compiler warning checker # for the Ubuntu/build and test job. # Keep lines sorted lexicographically to help avoid merge conflicts. + +Modules/_decimal/libmpdec/memory.c \ No newline at end of file From 99715d235cc437e6b5ce7cb194e34559149296c2 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Sat, 13 Jul 2024 16:36:42 -0500 Subject: [PATCH 08/46] Add config for improve fail check --- .github/workflows/reusable-ubuntu.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/reusable-ubuntu.yml b/.github/workflows/reusable-ubuntu.yml index 3a6f2b910d0da0..9b05e1141d357c 100644 --- a/.github/workflows/reusable-ubuntu.yml +++ b/.github/workflows/reusable-ubuntu.yml @@ -79,7 +79,7 @@ jobs: working-directory: ${{ env.CPYTHON_BUILDDIR }} run: make pythoninfo - name: Check compiler warnings - run: python Tools/build/check_warnings.py --compiler-output-file-path=${{ env.CPYTHON_BUILDDIR }}/compiler_output.txt --warning-ignore-file-path ${GITHUB_WORKSPACE}/Tools/build/.warningignore_ubuntu + run: python Tools/build/check_warnings.py --compiler-output-file-path=${{ env.CPYTHON_BUILDDIR }}/compiler_output.txt --warning-ignore-file-path ${GITHUB_WORKSPACE}/Tools/build/.warningignore_ubuntu --fail-on-improvement - name: Remount sources writable for tests # some tests write to srcdir, lack of pyc files slows down testing run: sudo mount $CPYTHON_RO_SRCDIR -oremount,rw From bc44ec223af3ed552b0d769ffd5629f8a540ebf2 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Sat, 13 Jul 2024 16:38:27 -0500 Subject: [PATCH 09/46] Revert to prod check warning state --- .github/workflows/reusable-ubuntu.yml | 2 +- Tools/build/.warningignore_ubuntu | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/reusable-ubuntu.yml b/.github/workflows/reusable-ubuntu.yml index 9b05e1141d357c..3a6f2b910d0da0 100644 --- a/.github/workflows/reusable-ubuntu.yml +++ b/.github/workflows/reusable-ubuntu.yml @@ -79,7 +79,7 @@ jobs: working-directory: ${{ env.CPYTHON_BUILDDIR }} run: make pythoninfo - name: Check compiler warnings - run: python Tools/build/check_warnings.py --compiler-output-file-path=${{ env.CPYTHON_BUILDDIR }}/compiler_output.txt --warning-ignore-file-path ${GITHUB_WORKSPACE}/Tools/build/.warningignore_ubuntu --fail-on-improvement + run: python Tools/build/check_warnings.py --compiler-output-file-path=${{ env.CPYTHON_BUILDDIR }}/compiler_output.txt --warning-ignore-file-path ${GITHUB_WORKSPACE}/Tools/build/.warningignore_ubuntu - name: Remount sources writable for tests # some tests write to srcdir, lack of pyc files slows down testing run: sudo mount $CPYTHON_RO_SRCDIR -oremount,rw diff --git a/Tools/build/.warningignore_ubuntu b/Tools/build/.warningignore_ubuntu index 83dccf0eb215f6..8242c8d17c89fb 100644 --- a/Tools/build/.warningignore_ubuntu +++ b/Tools/build/.warningignore_ubuntu @@ -1,5 +1,3 @@ # Files listed will be ignored by the compiler warning checker # for the Ubuntu/build and test job. # Keep lines sorted lexicographically to help avoid merge conflicts. - -Modules/_decimal/libmpdec/memory.c \ No newline at end of file From e2ca75f3991e20b453622b9917fc6182938ed57b Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sat, 13 Jul 2024 21:55:58 +0000 Subject: [PATCH 10/46] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Tests/2024-07-13-21-55-58.gh-issue-112301.YJS1dl.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Tests/2024-07-13-21-55-58.gh-issue-112301.YJS1dl.rst diff --git a/Misc/NEWS.d/next/Tests/2024-07-13-21-55-58.gh-issue-112301.YJS1dl.rst b/Misc/NEWS.d/next/Tests/2024-07-13-21-55-58.gh-issue-112301.YJS1dl.rst new file mode 100644 index 00000000000000..27ccd8e63d5d9e --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2024-07-13-21-55-58.gh-issue-112301.YJS1dl.rst @@ -0,0 +1 @@ +Add tooling to check for changes in compiler warnings. From bd1634e3c3504b4b79423c9f65a50bbf30205241 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 16 Jul 2024 22:32:12 -0500 Subject: [PATCH 11/46] Refactor creating set of files with warnings to a dedicated function --- ...-07-13-21-55-58.gh-issue-112301.YJS1dl.rst | 1 + Tools/build/check_warnings.py | 61 ++++++++++--------- 2 files changed, 34 insertions(+), 28 deletions(-) diff --git a/Misc/NEWS.d/next/Tests/2024-07-13-21-55-58.gh-issue-112301.YJS1dl.rst b/Misc/NEWS.d/next/Tests/2024-07-13-21-55-58.gh-issue-112301.YJS1dl.rst index 27ccd8e63d5d9e..4dd10fb746d44d 100644 --- a/Misc/NEWS.d/next/Tests/2024-07-13-21-55-58.gh-issue-112301.YJS1dl.rst +++ b/Misc/NEWS.d/next/Tests/2024-07-13-21-55-58.gh-issue-112301.YJS1dl.rst @@ -1 +1,2 @@ Add tooling to check for changes in compiler warnings. +Patch by Nate Ohlson \ No newline at end of file diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 16075d855a6e39..6c048ecd845dfa 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -3,11 +3,11 @@ Parses compiler output with -fdiagnostics-format=json and checks that warnings exist only in files that are expected to have warnings. """ -import sys -from pathlib import Path import argparse import json import re +import sys +from pathlib import Path def extract_warnings_from_compiler_output(compiler_output: str) -> list[dict]: """ @@ -24,63 +24,67 @@ def extract_warnings_from_compiler_output(compiler_output: str) -> list[dict]: try: json_data = json.loads(array) json_objects_in_array = [entry for entry in json_data] - compiler_warnings.extend([entry for entry in json_objects_in_array if entry.get('kind') == 'warning']) + compiler_warnings.extend([entry for entry in json_objects_in_array + if entry.get('kind') == 'warning']) except json.JSONDecodeError: continue # Skip malformed JSON return compiler_warnings +# Create a function that returns a dictionary where the key is the file and the data is the warnings in that file +def get_warnings_by_file(warnings: list[dict]) -> dict[str, list[dict]]: + """ + Returns a dictionary where the key is the file and the data is the warnings in that file + """ + warnings_by_file = {} + for warning in warnings: + locations = warning['locations'] + for location in locations: + for key in ['caret', 'start', 'end']: + if key in location: + file = location[key]['file'] + file = file.lstrip('./') # Remove leading current directory if present + if file not in warnings_by_file: + warnings_by_file[file] = [] + warnings_by_file[file].append(warning) + + return warnings_by_file + def get_unexpected_warnings( warnings: list[dict], files_with_expected_warnings: set[str], + files_with_warnings: set[str], ) -> int: """ Returns failure status if warnings discovered in list of warnings are associated with a file that is not found in the list of files with expected warnings """ unexpected_warnings = [] - for warning in warnings: - locations = warning['locations'] - for location in locations: - for key in ['caret', 'start', 'end']: - if key in location: - file = location[key]['file'] - file = file.lstrip('./') # Remove leading curdir if present - if file not in files_with_expected_warnings: - unexpected_warnings.append(warning) + for file in files_with_warnings.keys(): + if file not in files_with_expected_warnings: + unexpected_warnings.extend(files_with_warnings[file]) if unexpected_warnings: print("Unexpected warnings:") for warning in unexpected_warnings: print(warning) return 1 - + return 0 def get_unexpected_improvements( warnings: list[dict], files_with_expected_warnings: set[str], + files_with_warnings: set[str], ) -> int: """ Returns failure status if there are no warnings in the list of warnings for a file that is in the list of files with expected warnings """ - - # Create set of files with warnings - files_with_warnings = set() - for warning in warnings: - locations = warning['locations'] - for location in locations: - for key in ['caret', 'start', 'end']: - if key in location: - file = location[key]['file'] - file = file.lstrip('./') # Remove leading curdir if present - files_with_warnings.add(file) - unexpected_improvements = [] for file in files_with_expected_warnings: - if file not in files_with_warnings: + if file not in files_with_warnings.keys(): unexpected_improvements.append(file) if unexpected_improvements: @@ -143,11 +147,12 @@ def main(argv: list[str] | None = None) -> int: } warnings = extract_warnings_from_compiler_output(compiler_output_file_contents) + files_with_warnings = get_warnings_by_file(warnings) - status = get_unexpected_warnings(warnings, files_with_expected_warnings) + status = get_unexpected_warnings(warnings, files_with_expected_warnings, files_with_warnings) if args.fail_on_regression: exit_code |= status - status = get_unexpected_improvements(warnings, files_with_expected_warnings) + status = get_unexpected_improvements(warnings, files_with_expected_warnings, files_with_warnings) if args.fail_on_improvement: exit_code |= status return exit_code From b07b1d6241840639a5c2d90a31644e02d245b273 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 23 Jul 2024 02:41:34 -0500 Subject: [PATCH 12/46] Move cflags configure option to top level build configuration --- Tools/build/check_warnings.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 6c048ecd845dfa..1330a14f91ba5d 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -69,9 +69,8 @@ def get_unexpected_warnings( for warning in unexpected_warnings: print(warning) return 1 - - return 0 + return 0 def get_unexpected_improvements( warnings: list[dict], From e1954a53dd2b25098ebeb5b301cdb3188a61a5aa Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 23 Jul 2024 03:20:24 -0500 Subject: [PATCH 13/46] Add json diagnostics to ubuntu configuration as first argument --- .github/workflows/reusable-ubuntu.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/reusable-ubuntu.yml b/.github/workflows/reusable-ubuntu.yml index 3a6f2b910d0da0..c6289a74e9a5f6 100644 --- a/.github/workflows/reusable-ubuntu.yml +++ b/.github/workflows/reusable-ubuntu.yml @@ -67,6 +67,7 @@ jobs: working-directory: ${{ env.CPYTHON_BUILDDIR }} run: >- ../cpython-ro-srcdir/configure + CFLAGS="-fdiagnostics-format=json" --config-cache --with-pydebug --enable-slower-safety From 59351430878fb1195d5a1d309f2ae6f16c384c91 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 23 Jul 2024 15:44:55 -0500 Subject: [PATCH 14/46] Add newline to news --- .../next/Tests/2024-07-13-21-55-58.gh-issue-112301.YJS1dl.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Tests/2024-07-13-21-55-58.gh-issue-112301.YJS1dl.rst b/Misc/NEWS.d/next/Tests/2024-07-13-21-55-58.gh-issue-112301.YJS1dl.rst index 4dd10fb746d44d..4e8a8b6dd36ba6 100644 --- a/Misc/NEWS.d/next/Tests/2024-07-13-21-55-58.gh-issue-112301.YJS1dl.rst +++ b/Misc/NEWS.d/next/Tests/2024-07-13-21-55-58.gh-issue-112301.YJS1dl.rst @@ -1,2 +1,2 @@ Add tooling to check for changes in compiler warnings. -Patch by Nate Ohlson \ No newline at end of file +Patch by Nate Ohlson From 7f1a238b3a26fc8fcee4629dcb62753f369b7fa2 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Sat, 13 Jul 2024 12:50:17 -0500 Subject: [PATCH 15/46] Create simple warning check tool and add to ubuntu build and test job --- Tools/build/.warnignore_ubuntu | 3 +++ Tools/build/check_warnings.py | 2 -- 2 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 Tools/build/.warnignore_ubuntu diff --git a/Tools/build/.warnignore_ubuntu b/Tools/build/.warnignore_ubuntu new file mode 100644 index 00000000000000..8242c8d17c89fb --- /dev/null +++ b/Tools/build/.warnignore_ubuntu @@ -0,0 +1,3 @@ +# Files listed will be ignored by the compiler warning checker +# for the Ubuntu/build and test job. +# Keep lines sorted lexicographically to help avoid merge conflicts. diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 1330a14f91ba5d..712a505d057f4c 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -129,12 +129,10 @@ def main(argv: list[str] | None = None) -> int: if not Path(args.compiler_output_file_path).is_file(): print(f"Compiler output file does not exist: {args.compiler_output_file_path}") return 1 - # Check that the warning ignore file is a valid path if not Path(args.warning_ignore_file_path).is_file(): print(f"Warning ignore file does not exist: {args.warning_ignore_file_path}") return 1 - with Path(args.compiler_output_file_path).open(encoding="UTF-8") as f: compiler_output_file_contents = f.read() From 144136e8c7b3a4a8b846954e20f1be038bfbb52a Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Mon, 22 Jul 2024 13:59:08 -0500 Subject: [PATCH 16/46] Add macos warning checks to GitHub actions --- .github/workflows/reusable-macos.yml | 10 +++++++--- Tools/build/.warningignore_macos | 3 +++ 2 files changed, 10 insertions(+), 3 deletions(-) create mode 100644 Tools/build/.warningignore_macos diff --git a/.github/workflows/reusable-macos.yml b/.github/workflows/reusable-macos.yml index 0f189960dbea61..16e7bc71ff5dfd 100644 --- a/.github/workflows/reusable-macos.yml +++ b/.github/workflows/reusable-macos.yml @@ -26,13 +26,15 @@ jobs: strategy: fail-fast: false matrix: - os: ${{fromJson(inputs.os-matrix)}} + os: + - "macos-10.15" + - "macos-11" is-fork: - ${{ github.repository_owner != 'python' }} exclude: - os: "ghcr.io/cirruslabs/macos-runner:sonoma" is-fork: true - - os: "macos-14" + - os: "macos-10.15" is-fork: false runs-on: ${{ matrix.os }} steps: @@ -58,8 +60,10 @@ jobs: --prefix=/opt/python-dev \ --with-openssl="$(brew --prefix openssl@3.0)" - name: Build CPython - run: make -j8 + run: make -j8 &> compiler_output.txt - name: Display build info run: make pythoninfo + - name: Check compiler warnings + run: python Tools/build/check_warnings.py --compiler-output-file-path=${{ env.CPYTHON_BUILDDIR }}/compiler_output.txt --warning-ignore-file-path ${GITHUB_WORKSPACE}/Tools/build/.warningignore_macos - name: Tests run: make test diff --git a/Tools/build/.warningignore_macos b/Tools/build/.warningignore_macos new file mode 100644 index 00000000000000..1b504dfc54000f --- /dev/null +++ b/Tools/build/.warningignore_macos @@ -0,0 +1,3 @@ +# Files listed will be ignored by the compiler warning checker +# for the macOS/build and test job. +# Keep lines sorted lexicographically to help avoid merge conflicts. From 3dd40fd960d086ad41f57d1a03d5e83f63095dd6 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Mon, 22 Jul 2024 14:32:34 -0500 Subject: [PATCH 17/46] Revert reusable-macos.yml for environment variables --- .github/workflows/reusable-macos.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/reusable-macos.yml b/.github/workflows/reusable-macos.yml index 16e7bc71ff5dfd..e54de82a9ecf4a 100644 --- a/.github/workflows/reusable-macos.yml +++ b/.github/workflows/reusable-macos.yml @@ -26,15 +26,13 @@ jobs: strategy: fail-fast: false matrix: - os: - - "macos-10.15" - - "macos-11" + os: ${{fromJson(inputs.os-matrix)}} is-fork: - ${{ github.repository_owner != 'python' }} exclude: - os: "ghcr.io/cirruslabs/macos-runner:sonoma" is-fork: true - - os: "macos-10.15" + - os: "macos-14" is-fork: false runs-on: ${{ matrix.os }} steps: From 1158f53bc5e4976fc18fef9b460ede66c2bca3d2 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Mon, 22 Jul 2024 14:46:16 -0500 Subject: [PATCH 18/46] Update paths --- .github/workflows/reusable-macos.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/reusable-macos.yml b/.github/workflows/reusable-macos.yml index e54de82a9ecf4a..0751f2f63a564b 100644 --- a/.github/workflows/reusable-macos.yml +++ b/.github/workflows/reusable-macos.yml @@ -62,6 +62,6 @@ jobs: - name: Display build info run: make pythoninfo - name: Check compiler warnings - run: python Tools/build/check_warnings.py --compiler-output-file-path=${{ env.CPYTHON_BUILDDIR }}/compiler_output.txt --warning-ignore-file-path ${GITHUB_WORKSPACE}/Tools/build/.warningignore_macos + run: python Tools/build/check_warnings.py --compiler-output-file-path=compiler_output.txt --warning-ignore-file-path=Tools/build/.warningignore_macos - name: Tests run: make test From cb51b4f854cf2c8c65c5f65b6c24d89b3c17abeb Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Mon, 22 Jul 2024 15:03:30 -0500 Subject: [PATCH 19/46] Test unexpected improvement --- Tools/build/.warningignore_macos | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tools/build/.warningignore_macos b/Tools/build/.warningignore_macos index 1b504dfc54000f..ec6dc2c54367e9 100644 --- a/Tools/build/.warningignore_macos +++ b/Tools/build/.warningignore_macos @@ -1,3 +1,5 @@ # Files listed will be ignored by the compiler warning checker # for the macOS/build and test job. # Keep lines sorted lexicographically to help avoid merge conflicts. + +Modules/_abc.c \ No newline at end of file From 77e0f6e7850d3696fffaa4c08e73053a9f8922ef Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Mon, 22 Jul 2024 16:08:53 -0500 Subject: [PATCH 20/46] Remove warning ignore --- Tools/build/.warningignore_macos | 2 -- 1 file changed, 2 deletions(-) diff --git a/Tools/build/.warningignore_macos b/Tools/build/.warningignore_macos index ec6dc2c54367e9..1b504dfc54000f 100644 --- a/Tools/build/.warningignore_macos +++ b/Tools/build/.warningignore_macos @@ -1,5 +1,3 @@ # Files listed will be ignored by the compiler warning checker # for the macOS/build and test job. # Keep lines sorted lexicographically to help avoid merge conflicts. - -Modules/_abc.c \ No newline at end of file From 3e1d75f14c87505055e4b5a9c269068253a29b76 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Mon, 22 Jul 2024 16:57:19 -0500 Subject: [PATCH 21/46] Add json output option to macos configure job --- .github/workflows/reusable-macos.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/reusable-macos.yml b/.github/workflows/reusable-macos.yml index 0751f2f63a564b..6241b11fa6106f 100644 --- a/.github/workflows/reusable-macos.yml +++ b/.github/workflows/reusable-macos.yml @@ -51,6 +51,7 @@ jobs: GDBM_CFLAGS="-I$(brew --prefix gdbm)/include" \ GDBM_LIBS="-L$(brew --prefix gdbm)/lib -lgdbm" \ ./configure \ + CFLAGS="--fdiagnostics-format=json" \ --config-cache \ --with-pydebug \ --enable-slower-safety \ From b5cd58ae585710776f2d48410f2146e3a45fc065 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 23 Jul 2024 00:09:14 -0500 Subject: [PATCH 22/46] Add common dictionary format when parsing warnings --- .github/workflows/reusable-macos.yml | 2 +- .github/workflows/reusable-ubuntu.yml | 2 +- Tools/build/check_warnings.py | 70 ++++++++++++++++++++------- 3 files changed, 55 insertions(+), 19 deletions(-) diff --git a/.github/workflows/reusable-macos.yml b/.github/workflows/reusable-macos.yml index 6241b11fa6106f..9adda38cf165d2 100644 --- a/.github/workflows/reusable-macos.yml +++ b/.github/workflows/reusable-macos.yml @@ -63,6 +63,6 @@ jobs: - name: Display build info run: make pythoninfo - name: Check compiler warnings - run: python Tools/build/check_warnings.py --compiler-output-file-path=compiler_output.txt --warning-ignore-file-path=Tools/build/.warningignore_macos + run: python Tools/build/check_warnings.py --compiler-output-file-path=compiler_output.txt --warning-ignore-file-path=Tools/build/.warningignore_macos --compiler-output-type=clang - name: Tests run: make test diff --git a/.github/workflows/reusable-ubuntu.yml b/.github/workflows/reusable-ubuntu.yml index c6289a74e9a5f6..670dec62c4ea30 100644 --- a/.github/workflows/reusable-ubuntu.yml +++ b/.github/workflows/reusable-ubuntu.yml @@ -80,7 +80,7 @@ jobs: working-directory: ${{ env.CPYTHON_BUILDDIR }} run: make pythoninfo - name: Check compiler warnings - run: python Tools/build/check_warnings.py --compiler-output-file-path=${{ env.CPYTHON_BUILDDIR }}/compiler_output.txt --warning-ignore-file-path ${GITHUB_WORKSPACE}/Tools/build/.warningignore_ubuntu + run: python Tools/build/check_warnings.py --compiler-output-file-path=${{ env.CPYTHON_BUILDDIR }}/compiler_output.txt --warning-ignore-file-path ${GITHUB_WORKSPACE}/Tools/build/.warningignore_ubuntu --compiler-output-type=json - name: Remount sources writable for tests # some tests write to srcdir, lack of pyc files slows down testing run: sudo mount $CPYTHON_RO_SRCDIR -oremount,rw diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 712a505d057f4c..5c359a69462c9f 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -9,7 +9,27 @@ import sys from pathlib import Path -def extract_warnings_from_compiler_output(compiler_output: str) -> list[dict]: +def extract_warnings_from_compiler_output_clang(compiler_output: str) -> list[dict]: + """ + Extracts warnings from the compiler output when using clang + """ + + # Regex to find warnings in the compiler output + clang_warning_regex = re.compile(r'(?P.*):(?P\d+):(?P\d+): warning: (?P.*)') + compiler_warnings = [] + for line in compiler_output.splitlines(): + match = clang_warning_regex.match(line) + if match: + compiler_warnings.append({ + 'file': match.group('file'), + 'line': match.group('line'), + 'column': match.group('column'), + 'message': match.group('message'), + }) + + return compiler_warnings + +def extract_warnings_from_compiler_output_json(compiler_output: str) -> list[dict]: """ Extracts warnings from the compiler output when using -fdiagnostics-format=json @@ -24,8 +44,20 @@ def extract_warnings_from_compiler_output(compiler_output: str) -> list[dict]: try: json_data = json.loads(array) json_objects_in_array = [entry for entry in json_data] - compiler_warnings.extend([entry for entry in json_objects_in_array - if entry.get('kind') == 'warning']) + warning_list = [entry for entry in json_objects_in_array if entry.get('kind') == 'warning'] + for warning in warning_list: + locations = warning['locations'] + for location in locations: + for key in ['caret', 'start', 'end']: + if key in location: + compiler_warnings.append({ + 'file': location[key]['file'].lstrip('./'), # Remove leading current directory if present + 'line': location[key]['line'], + 'column': location[key]['column'], + 'message': warning['message'], + }) + + except json.JSONDecodeError: continue # Skip malformed JSON @@ -38,20 +70,14 @@ def get_warnings_by_file(warnings: list[dict]) -> dict[str, list[dict]]: """ warnings_by_file = {} for warning in warnings: - locations = warning['locations'] - for location in locations: - for key in ['caret', 'start', 'end']: - if key in location: - file = location[key]['file'] - file = file.lstrip('./') # Remove leading current directory if present - if file not in warnings_by_file: - warnings_by_file[file] = [] - warnings_by_file[file].append(warning) + file = warning['file'] + if file not in warnings_by_file: + warnings_by_file[file] = [] + warnings_by_file[file].append(warning) return warnings_by_file def get_unexpected_warnings( - warnings: list[dict], files_with_expected_warnings: set[str], files_with_warnings: set[str], ) -> int: @@ -73,7 +99,6 @@ def get_unexpected_warnings( return 0 def get_unexpected_improvements( - warnings: list[dict], files_with_expected_warnings: set[str], files_with_warnings: set[str], ) -> int: @@ -120,6 +145,13 @@ def main(argv: list[str] | None = None) -> int: default=False, help="Flag to fail if files that were expected to have warnings have no warnings" ) + parser.add_argument( + "--compiler-output-type", + type=str, + required=True, + choices=["json", "clang"], + help="Type of compiler output file (json or clang)" + ) args = parser.parse_args(argv) @@ -143,13 +175,17 @@ def main(argv: list[str] | None = None) -> int: if file.strip() and not file.startswith("#") } - warnings = extract_warnings_from_compiler_output(compiler_output_file_contents) + if args.compiler_output_type == "json": + warnings = extract_warnings_from_compiler_output_json(compiler_output_file_contents) + elif args.compiler_output_type == "clang": + warnings = extract_warnings_from_compiler_output_clang(compiler_output_file_contents) + files_with_warnings = get_warnings_by_file(warnings) - status = get_unexpected_warnings(warnings, files_with_expected_warnings, files_with_warnings) + status = get_unexpected_warnings(files_with_expected_warnings, files_with_warnings) if args.fail_on_regression: exit_code |= status - status = get_unexpected_improvements(warnings, files_with_expected_warnings, files_with_warnings) + status = get_unexpected_improvements(files_with_expected_warnings, files_with_warnings) if args.fail_on_improvement: exit_code |= status return exit_code From 02f313e37bbf052fa1e7a09cb2c29d38abf9d095 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 23 Jul 2024 00:11:42 -0500 Subject: [PATCH 23/46] Remove configure option for macos job --- .github/workflows/reusable-macos.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/reusable-macos.yml b/.github/workflows/reusable-macos.yml index 9adda38cf165d2..530855212ba33e 100644 --- a/.github/workflows/reusable-macos.yml +++ b/.github/workflows/reusable-macos.yml @@ -51,7 +51,6 @@ jobs: GDBM_CFLAGS="-I$(brew --prefix gdbm)/include" \ GDBM_LIBS="-L$(brew --prefix gdbm)/lib -lgdbm" \ ./configure \ - CFLAGS="--fdiagnostics-format=json" \ --config-cache \ --with-pydebug \ --enable-slower-safety \ From 83d1ed7100f45d0d43bf823dc9087107b93a8272 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 23 Jul 2024 01:55:22 -0500 Subject: [PATCH 24/46] Print out json version of compiler output --- Tools/build/check_warnings.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 5c359a69462c9f..c6061085a12bd3 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -36,6 +36,9 @@ def extract_warnings_from_compiler_output_json(compiler_output: str) -> list[dic Compiler output as a whole is not a valid json document, but includes many json objects and may include other output that is not json. """ + print("######") + print(compiler_output) + print("&&&&&") # Regex to find json arrays at the top level of the file in the compiler output json_arrays = re.findall(r'\[(?:[^\[\]]|\[(?:[^\[\]]|\[[^\[\]]*\])*\])*\]', compiler_output) From 08a6f6d1d06ecf81fae7258adfebee164ae1f0a5 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 23 Jul 2024 16:22:50 -0500 Subject: [PATCH 25/46] Remove old version of warning ignore file --- Tools/build/.warnignore_ubuntu | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 Tools/build/.warnignore_ubuntu diff --git a/Tools/build/.warnignore_ubuntu b/Tools/build/.warnignore_ubuntu deleted file mode 100644 index 8242c8d17c89fb..00000000000000 --- a/Tools/build/.warnignore_ubuntu +++ /dev/null @@ -1,3 +0,0 @@ -# Files listed will be ignored by the compiler warning checker -# for the Ubuntu/build and test job. -# Keep lines sorted lexicographically to help avoid merge conflicts. From 7be8ee6b1ea327c161f7d3f268ac169d726add1b Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Wed, 24 Jul 2024 00:15:26 -0500 Subject: [PATCH 26/46] Remove compiler output print diagnostic --- Tools/build/check_warnings.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index c6061085a12bd3..f5e8ec8efdd943 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -13,7 +13,6 @@ def extract_warnings_from_compiler_output_clang(compiler_output: str) -> list[di """ Extracts warnings from the compiler output when using clang """ - # Regex to find warnings in the compiler output clang_warning_regex = re.compile(r'(?P.*):(?P\d+):(?P\d+): warning: (?P.*)') compiler_warnings = [] @@ -36,10 +35,6 @@ def extract_warnings_from_compiler_output_json(compiler_output: str) -> list[dic Compiler output as a whole is not a valid json document, but includes many json objects and may include other output that is not json. """ - print("######") - print(compiler_output) - print("&&&&&") - # Regex to find json arrays at the top level of the file in the compiler output json_arrays = re.findall(r'\[(?:[^\[\]]|\[(?:[^\[\]]|\[[^\[\]]*\])*\])*\]', compiler_output) compiler_warnings = [] From 8b0a2eeaec843dbb5ba5defcf43acd37cf28a0fa Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Wed, 24 Jul 2024 05:18:26 +0000 Subject: [PATCH 27/46] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../Security/2024-07-24-05-18-25.gh-issue-112301.lfINgZ.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Security/2024-07-24-05-18-25.gh-issue-112301.lfINgZ.rst diff --git a/Misc/NEWS.d/next/Security/2024-07-24-05-18-25.gh-issue-112301.lfINgZ.rst b/Misc/NEWS.d/next/Security/2024-07-24-05-18-25.gh-issue-112301.lfINgZ.rst new file mode 100644 index 00000000000000..bc9f6758108ce1 --- /dev/null +++ b/Misc/NEWS.d/next/Security/2024-07-24-05-18-25.gh-issue-112301.lfINgZ.rst @@ -0,0 +1,2 @@ +Add macOS warning tracking to warning check tooling. +Patch by Nate Ohlson From 49cbd87cc6c4c88bf5721c601e36e50bbb51cbae Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Wed, 24 Jul 2024 00:21:37 -0500 Subject: [PATCH 28/46] Remove superfluous comment --- Tools/build/check_warnings.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index f5e8ec8efdd943..9771977d6e705c 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -61,7 +61,6 @@ def extract_warnings_from_compiler_output_json(compiler_output: str) -> list[dic return compiler_warnings -# Create a function that returns a dictionary where the key is the file and the data is the warnings in that file def get_warnings_by_file(warnings: list[dict]) -> dict[str, list[dict]]: """ Returns a dictionary where the key is the file and the data is the warnings in that file From b654a84c46debebe2edbb63f19d646a8ff0928d8 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 30 Jul 2024 10:26:34 -0500 Subject: [PATCH 29/46] Add period to news --- .../Security/2024-07-24-05-18-25.gh-issue-112301.lfINgZ.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Security/2024-07-24-05-18-25.gh-issue-112301.lfINgZ.rst b/Misc/NEWS.d/next/Security/2024-07-24-05-18-25.gh-issue-112301.lfINgZ.rst index bc9f6758108ce1..81237e735ebdb7 100644 --- a/Misc/NEWS.d/next/Security/2024-07-24-05-18-25.gh-issue-112301.lfINgZ.rst +++ b/Misc/NEWS.d/next/Security/2024-07-24-05-18-25.gh-issue-112301.lfINgZ.rst @@ -1,2 +1,2 @@ -Add macOS warning tracking to warning check tooling. -Patch by Nate Ohlson +Add macOS warning tracking to warning check tooling. +Patch by Nate Ohlson. From 3688c5c2639d22b28c15a74b83ce912a775c2201 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 30 Jul 2024 22:20:43 -0500 Subject: [PATCH 30/46] Make warning ignore file optional --- Tools/build/check_warnings.py | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 77347c752ef6e9..e936bc372f5cc1 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -151,7 +151,6 @@ def main(argv: list[str] | None = None) -> int: "-i", "--warning-ignore-file-path", type=str, - required=True, help="Path to the warning ignore file", ) parser.add_argument( @@ -187,24 +186,31 @@ def main(argv: list[str] | None = None) -> int: f"Compiler output file does not exist: {args.compiler_output_file_path}" ) return 1 - # Check that the warning ignore file is a valid path - if not Path(args.warning_ignore_file_path).is_file(): + + # Check that a warning ignore file was specified and if so is a valid path + if not args.warning_ignore_file_path: print( - f"Warning ignore file does not exist: {args.warning_ignore_file_path}" + "Warning ignore file not specified. Continuing without it (no warnings ignored)." ) - return 1 + files_with_expected_warnings = set() + else: + if not Path(args.warning_ignore_file_path).is_file(): + print( + f"Warning ignore file does not exist: {args.warning_ignore_file_path}" + ) + return 1 + with Path(args.warning_ignore_file_path).open( + encoding="UTF-8" + ) as clean_files: + files_with_expected_warnings = { + file.strip() + for file in clean_files + if file.strip() and not file.startswith("#") + } + with Path(args.compiler_output_file_path).open(encoding="UTF-8") as f: compiler_output_file_contents = f.read() - with Path(args.warning_ignore_file_path).open( - encoding="UTF-8" - ) as clean_files: - files_with_expected_warnings = { - file.strip() - for file in clean_files - if file.strip() and not file.startswith("#") - } - if args.compiler_output_type == "json": warnings = extract_warnings_from_compiler_output_json( compiler_output_file_contents From cb1f276f5e30a8f05a4199a82afa64a8e56e2394 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 30 Jul 2024 22:49:35 -0500 Subject: [PATCH 31/46] Add write compiler output to log and file --- .github/workflows/reusable-macos.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/reusable-macos.yml b/.github/workflows/reusable-macos.yml index 8e3f7e52615478..d77723ef27c2dc 100644 --- a/.github/workflows/reusable-macos.yml +++ b/.github/workflows/reusable-macos.yml @@ -48,10 +48,10 @@ jobs: --prefix=/opt/python-dev \ --with-openssl="$(brew --prefix openssl@3.0)" - name: Build CPython - run: make -j8 &> compiler_output.txt + run: set -o pipefail; make -j8 2>&1 | tee compiler_output.txt - name: Display build info run: make pythoninfo - name: Check compiler warnings - run: python Tools/build/check_warnings.py --compiler-output-file-path=compiler_output.txt --warning-ignore-file-path=Tools/build/.warningignore_macos --compiler-output-type=clang + run: python3 Tools/build/check_warnings.py --compiler-output-file-path=compiler_output.txt --warning-ignore-file-path=Tools/build/.warningignore_macos --compiler-output-type=clang - name: Tests run: make test From fb91e3e2d38aa89b2779e18f94645733915b01f1 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Wed, 31 Jul 2024 12:02:40 -0500 Subject: [PATCH 32/46] Fix formatting and update regex --- Tools/build/check_warnings.py | 47 +++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index e936bc372f5cc1..ddadde6583264b 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -2,7 +2,9 @@ Parses compiler output with -fdiagnostics-format=json and checks that warnings exist only in files that are expected to have warnings. """ + import argparse +from collections import defaultdict import json import re import sys @@ -21,8 +23,7 @@ def extract_warnings_from_compiler_output_clang( ) compiler_warnings = [] for line in compiler_output.splitlines(): - match = clang_warning_regex.match(line) - if match: + if match := clang_warning_regex.match(line): compiler_warnings.append( { "file": match.group("file"), @@ -39,14 +40,16 @@ def extract_warnings_from_compiler_output_json( compiler_output: str, ) -> list[dict]: """ - Extracts warnings from the compiler output when using -fdiagnostics-format=json + Extracts warnings from the compiler output when using + -fdiagnostics-format=json. - Compiler output as a whole is not a valid json document, but includes many json - objects and may include other output that is not json. + Compiler output as a whole is not a valid json document, + but includes many json objects and may include other output + that is not json. """ # Regex to find json arrays at the top level of the file in the compiler output json_arrays = re.findall( - r"\[(?:[^\[\]]|\[(?:[^\[\]]|\[[^\[\]]*\])*\])*\]", compiler_output + r"\[(?:[^[\]]|\[[^\]]*\])*\]", compiler_output ) compiler_warnings = [] for array in json_arrays: @@ -61,18 +64,24 @@ def extract_warnings_from_compiler_output_json( for warning in warning_list: locations = warning["locations"] for location in locations: + found_location = False for key in ["caret", "start", "end"]: if key in location: compiler_warnings.append( { - "file": location[key]["file"].lstrip( - "./" - ), # Remove leading current directory if present + # Remove leading current directory if present + "file": location[key]["file"].lstrip("./"), "line": location[key]["line"], "column": location[key]["column"], "message": warning["message"], } ) + # Found a caret, start, or end in location so + # break out completely to address next warning + break + else: + continue + break except json.JSONDecodeError: continue # Skip malformed JSON @@ -84,19 +93,16 @@ def get_warnings_by_file(warnings: list[dict]) -> dict[str, list[dict]]: """ Returns a dictionary where the key is the file and the data is the warnings in that file """ - warnings_by_file = {} + warnings_by_file = defaultdict(list) for warning in warnings: - file = warning["file"] - if file not in warnings_by_file: - warnings_by_file[file] = [] - warnings_by_file[file].append(warning) + warnings_by_file[warning["file"]].append(warning) return warnings_by_file def get_unexpected_warnings( files_with_expected_warnings: set[str], - files_with_warnings: set[str], + files_with_warnings: dict[str, list[dict]], ) -> int: """ Returns failure status if warnings discovered in list of warnings are associated with a file @@ -118,7 +124,7 @@ def get_unexpected_warnings( def get_unexpected_improvements( files_with_expected_warnings: set[str], - files_with_warnings: set[str], + files_with_warnings: dict[str, list[dict]], ) -> int: """ Returns failure status if there are no warnings in the list of warnings for a file @@ -183,20 +189,23 @@ def main(argv: list[str] | None = None) -> int: # Check that the compiler output file is a valid path if not Path(args.compiler_output_file_path).is_file(): print( - f"Compiler output file does not exist: {args.compiler_output_file_path}" + f"Compiler output file does not exist:" + f" {args.compiler_output_file_path}" ) return 1 # Check that a warning ignore file was specified and if so is a valid path if not args.warning_ignore_file_path: print( - "Warning ignore file not specified. Continuing without it (no warnings ignored)." + "Warning ignore file not specified." + " Continuing without it (no warnings ignored)." ) files_with_expected_warnings = set() else: if not Path(args.warning_ignore_file_path).is_file(): print( - f"Warning ignore file does not exist: {args.warning_ignore_file_path}" + f"Warning ignore file does not exist:" + f" {args.warning_ignore_file_path}" ) return 1 with Path(args.warning_ignore_file_path).open( From f35ba6013af9db280073b0f23f5449f922f5c00a Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Mon, 5 Aug 2024 15:00:54 -0500 Subject: [PATCH 33/46] Fix comment formatting --- Tools/build/check_warnings.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index ddadde6583264b..31258932dbd4ca 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -47,7 +47,8 @@ def extract_warnings_from_compiler_output_json( but includes many json objects and may include other output that is not json. """ - # Regex to find json arrays at the top level of the file in the compiler output + # Regex to find json arrays at the top level of the file + # in the compiler output json_arrays = re.findall( r"\[(?:[^[\]]|\[[^\]]*\])*\]", compiler_output ) @@ -64,7 +65,6 @@ def extract_warnings_from_compiler_output_json( for warning in warning_list: locations = warning["locations"] for location in locations: - found_location = False for key in ["caret", "start", "end"]: if key in location: compiler_warnings.append( @@ -91,7 +91,8 @@ def extract_warnings_from_compiler_output_json( def get_warnings_by_file(warnings: list[dict]) -> dict[str, list[dict]]: """ - Returns a dictionary where the key is the file and the data is the warnings in that file + Returns a dictionary where the key is the file and the data is the warnings + in that file """ warnings_by_file = defaultdict(list) for warning in warnings: @@ -105,8 +106,9 @@ def get_unexpected_warnings( files_with_warnings: dict[str, list[dict]], ) -> int: """ - Returns failure status if warnings discovered in list of warnings are associated with a file - that is not found in the list of files with expected warnings + Returns failure status if warnings discovered in list of warnings + are associated with a file that is not found in the list of files + with expected warnings """ unexpected_warnings = [] for file in files_with_warnings.keys(): @@ -127,8 +129,8 @@ def get_unexpected_improvements( files_with_warnings: dict[str, list[dict]], ) -> int: """ - Returns failure status if there are no warnings in the list of warnings for a file - that is in the list of files with expected warnings + Returns failure status if there are no warnings in the list of warnings + for a file that is in the list of files with expected warnings """ unexpected_improvements = [] for file in files_with_expected_warnings: @@ -171,7 +173,8 @@ def main(argv: list[str] | None = None) -> int: "--fail-on-improvement", action="store_true", default=False, - help="Flag to fail if files that were expected to have warnings have no warnings", + help="Flag to fail if files that were expected " + "to have warnings have no warnings", ) parser.add_argument( "-t", From 294dd6163472312ecd0fb4c851cc48f434f99248 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Mon, 5 Aug 2024 18:20:24 -0500 Subject: [PATCH 34/46] Add warning count to check warnigns tooling --- Tools/build/.warningignore_macos | 2 ++ Tools/build/.warningignore_ubuntu | 2 ++ Tools/build/check_warnings.py | 18 ++++++++++++++---- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/Tools/build/.warningignore_macos b/Tools/build/.warningignore_macos index 1b504dfc54000f..67f50119db7310 100644 --- a/Tools/build/.warningignore_macos +++ b/Tools/build/.warningignore_macos @@ -1,3 +1,5 @@ # Files listed will be ignored by the compiler warning checker # for the macOS/build and test job. # Keep lines sorted lexicographically to help avoid merge conflicts. +# Format example: +# /path/to/file (number of warnings in file) diff --git a/Tools/build/.warningignore_ubuntu b/Tools/build/.warningignore_ubuntu index 8242c8d17c89fb..469c727abfb11c 100644 --- a/Tools/build/.warningignore_ubuntu +++ b/Tools/build/.warningignore_ubuntu @@ -1,3 +1,5 @@ # Files listed will be ignored by the compiler warning checker # for the Ubuntu/build and test job. # Keep lines sorted lexicographically to help avoid merge conflicts. +# Format example: +# /path/to/file (number of warnings in file) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 31258932dbd4ca..7a0932dc017c53 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -74,6 +74,7 @@ def extract_warnings_from_compiler_output_json( "line": location[key]["line"], "column": location[key]["column"], "message": warning["message"], + "option": warning["option"], } ) # Found a caret, start, or end in location so @@ -112,6 +113,11 @@ def get_unexpected_warnings( """ unexpected_warnings = [] for file in files_with_warnings.keys(): + for ignore_file in files_with_expected_warnings: + if file == ignore_file[0]: + if len(files_with_warnings[file]) > ignore_file[1]: + unexpected_warnings.extend(files_with_warnings[file]) + break if file not in files_with_expected_warnings: unexpected_warnings.extend(files_with_warnings[file]) @@ -134,8 +140,11 @@ def get_unexpected_improvements( """ unexpected_improvements = [] for file in files_with_expected_warnings: - if file not in files_with_warnings.keys(): + if file[0] not in files_with_warnings.keys(): unexpected_improvements.append(file) + else: + if (len(files_with_warnings[file[0]]) < file[1]): + unexpected_improvements.append(file) if unexpected_improvements: print("Unexpected improvements:") @@ -214,11 +223,12 @@ def main(argv: list[str] | None = None) -> int: with Path(args.warning_ignore_file_path).open( encoding="UTF-8" ) as clean_files: - files_with_expected_warnings = { - file.strip() + files_with_expected_warnings = [ + (file.strip().split()[0], int(file.strip().split()[1])) for file in clean_files if file.strip() and not file.startswith("#") - } + ] + with Path(args.compiler_output_file_path).open(encoding="UTF-8") as f: compiler_output_file_contents = f.read() From 405698517dbc4491300b33562ae4d4af29e469f6 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Mon, 5 Aug 2024 18:53:52 -0500 Subject: [PATCH 35/46] Remove duplicate warnings from check warning tooling --- Tools/build/check_warnings.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 7a0932dc017c53..442ed9b4458a2f 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -93,12 +93,17 @@ def extract_warnings_from_compiler_output_json( def get_warnings_by_file(warnings: list[dict]) -> dict[str, list[dict]]: """ Returns a dictionary where the key is the file and the data is the warnings - in that file + in that file. Does not include duplicate warnings for a file from list of + provided warnings. """ warnings_by_file = defaultdict(list) + warnings_added = set() for warning in warnings: - warnings_by_file[warning["file"]].append(warning) - + warning_key = f"{warning['file']}-{warning['line']}-{warning['column']}-{warning['option']}" + if warning_key not in warnings_added: + warnings_added.add(warning_key) + warnings_by_file[warning["file"]].append(warning) + return warnings_by_file @@ -113,12 +118,14 @@ def get_unexpected_warnings( """ unexpected_warnings = [] for file in files_with_warnings.keys(): + found_file_in_ignore_list = False for ignore_file in files_with_expected_warnings: if file == ignore_file[0]: if len(files_with_warnings[file]) > ignore_file[1]: unexpected_warnings.extend(files_with_warnings[file]) + found_file_in_ignore_list = True break - if file not in files_with_expected_warnings: + if not found_file_in_ignore_list: unexpected_warnings.extend(files_with_warnings[file]) if unexpected_warnings: From 4164b81406e11618f664f73a4cae4378e245f531 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Mon, 5 Aug 2024 19:04:17 -0500 Subject: [PATCH 36/46] Trim trailing whitespace --- Tools/build/check_warnings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 442ed9b4458a2f..b9e80f3bcfea9e 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -103,7 +103,7 @@ def get_warnings_by_file(warnings: list[dict]) -> dict[str, list[dict]]: if warning_key not in warnings_added: warnings_added.add(warning_key) warnings_by_file[warning["file"]].append(warning) - + return warnings_by_file From 8b07ae8845cf74879eaa3722e9499b461663bb12 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Tue, 6 Aug 2024 00:06:24 +0000 Subject: [PATCH 37/46] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Security/2024-08-06-00-06-23.gh-issue-112301.4k4lw6.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Security/2024-08-06-00-06-23.gh-issue-112301.4k4lw6.rst diff --git a/Misc/NEWS.d/next/Security/2024-08-06-00-06-23.gh-issue-112301.4k4lw6.rst b/Misc/NEWS.d/next/Security/2024-08-06-00-06-23.gh-issue-112301.4k4lw6.rst new file mode 100644 index 00000000000000..83fcc91cd09cb0 --- /dev/null +++ b/Misc/NEWS.d/next/Security/2024-08-06-00-06-23.gh-issue-112301.4k4lw6.rst @@ -0,0 +1 @@ +Add ability to ignore warnings per file with warning count in warning checking tooling. Patch by Nate Ohlson. From 17858b9d61b3613aafa674c6e5cfa76211de0483 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Mon, 5 Aug 2024 19:07:38 -0500 Subject: [PATCH 38/46] Reformat news --- .../Security/2024-08-06-00-06-23.gh-issue-112301.4k4lw6.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Security/2024-08-06-00-06-23.gh-issue-112301.4k4lw6.rst b/Misc/NEWS.d/next/Security/2024-08-06-00-06-23.gh-issue-112301.4k4lw6.rst index 83fcc91cd09cb0..0bd2f4d7810a78 100644 --- a/Misc/NEWS.d/next/Security/2024-08-06-00-06-23.gh-issue-112301.4k4lw6.rst +++ b/Misc/NEWS.d/next/Security/2024-08-06-00-06-23.gh-issue-112301.4k4lw6.rst @@ -1 +1,2 @@ -Add ability to ignore warnings per file with warning count in warning checking tooling. Patch by Nate Ohlson. +Add ability to ignore warnings per file with warning count in warning checking tooling. +Patch by Nate Ohlson. From 55e20bdaa8913c4c416f1173587bdd3e60cf04f6 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 6 Aug 2024 14:05:40 -0500 Subject: [PATCH 39/46] Update helpers for function argument types --- Tools/build/check_warnings.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index b9e80f3bcfea9e..8d0dd2218ce475 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -108,7 +108,7 @@ def get_warnings_by_file(warnings: list[dict]) -> dict[str, list[dict]]: def get_unexpected_warnings( - files_with_expected_warnings: set[str], + files_with_expected_warnings: set[tuple[str, int]], files_with_warnings: dict[str, list[dict]], ) -> int: """ @@ -138,7 +138,7 @@ def get_unexpected_warnings( def get_unexpected_improvements( - files_with_expected_warnings: set[str], + files_with_expected_warnings: set[tuple[str, int]], files_with_warnings: dict[str, list[dict]], ) -> int: """ @@ -230,13 +230,15 @@ def main(argv: list[str] | None = None) -> int: with Path(args.warning_ignore_file_path).open( encoding="UTF-8" ) as clean_files: + # Files with expected warnings are stored as a set of tuples + # where the first element is the file name and the second element + # is the number of warnings expected in that file files_with_expected_warnings = [ (file.strip().split()[0], int(file.strip().split()[1])) for file in clean_files if file.strip() and not file.startswith("#") ] - with Path(args.compiler_output_file_path).open(encoding="UTF-8") as f: compiler_output_file_contents = f.read() From 566e8cd313f1d7589851b9bdca6fd5183d4effa6 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 6 Aug 2024 14:06:37 -0500 Subject: [PATCH 40/46] Format line length --- Tools/build/check_warnings.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 8d0dd2218ce475..b57603efb0a64a 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -49,9 +49,7 @@ def extract_warnings_from_compiler_output_json( """ # Regex to find json arrays at the top level of the file # in the compiler output - json_arrays = re.findall( - r"\[(?:[^[\]]|\[[^\]]*\])*\]", compiler_output - ) + json_arrays = re.findall(r"\[(?:[^[\]]|\[[^\]]*\])*\]", compiler_output) compiler_warnings = [] for array in json_arrays: try: @@ -150,7 +148,7 @@ def get_unexpected_improvements( if file[0] not in files_with_warnings.keys(): unexpected_improvements.append(file) else: - if (len(files_with_warnings[file[0]]) < file[1]): + if len(files_with_warnings[file[0]]) < file[1]: unexpected_improvements.append(file) if unexpected_improvements: From f8ae75ea892eaa5314e0c645bad13cfc0cb2bc51 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Mon, 12 Aug 2024 17:00:15 -0500 Subject: [PATCH 41/46] Use named tuple for warning filename and count --- Tools/build/check_warnings.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index b57603efb0a64a..72fb8a6178d294 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -9,6 +9,11 @@ import re import sys from pathlib import Path +from typing import NamedTuple + +class FileWarnings(NamedTuple): + name: str + count: int def extract_warnings_from_compiler_output_clang( @@ -49,7 +54,7 @@ def extract_warnings_from_compiler_output_json( """ # Regex to find json arrays at the top level of the file # in the compiler output - json_arrays = re.findall(r"\[(?:[^[\]]|\[[^\]]*\])*\]", compiler_output) + json_arrays = re.findall(r"\[(?:[^[\]]|\[[^]]*])*]", compiler_output) compiler_warnings = [] for array in json_arrays: try: @@ -97,7 +102,10 @@ def get_warnings_by_file(warnings: list[dict]) -> dict[str, list[dict]]: warnings_by_file = defaultdict(list) warnings_added = set() for warning in warnings: - warning_key = f"{warning['file']}-{warning['line']}-{warning['column']}-{warning['option']}" + warning_key = ( + f"{warning['file']}-{warning['line']}-" + f"{warning['column']}-{warning['option']}" + ) if warning_key not in warnings_added: warnings_added.add(warning_key) warnings_by_file[warning["file"]].append(warning) @@ -118,8 +126,8 @@ def get_unexpected_warnings( for file in files_with_warnings.keys(): found_file_in_ignore_list = False for ignore_file in files_with_expected_warnings: - if file == ignore_file[0]: - if len(files_with_warnings[file]) > ignore_file[1]: + if file == ignore_file.name: + if len(files_with_warnings[file]) > ignore_file.count: unexpected_warnings.extend(files_with_warnings[file]) found_file_in_ignore_list = True break @@ -145,10 +153,10 @@ def get_unexpected_improvements( """ unexpected_improvements = [] for file in files_with_expected_warnings: - if file[0] not in files_with_warnings.keys(): + if file.name not in files_with_warnings.keys(): unexpected_improvements.append(file) else: - if len(files_with_warnings[file[0]]) < file[1]: + if len(files_with_warnings[file.name]) < file.count: unexpected_improvements.append(file) if unexpected_improvements: @@ -232,7 +240,7 @@ def main(argv: list[str] | None = None) -> int: # where the first element is the file name and the second element # is the number of warnings expected in that file files_with_expected_warnings = [ - (file.strip().split()[0], int(file.strip().split()[1])) + FileWarning(file.strip().split()[0], int(file.strip().split()[1])) for file in clean_files if file.strip() and not file.startswith("#") ] From cbf069b461c5b32996aff6d6946a21fdbb85b4d1 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Mon, 12 Aug 2024 18:10:30 -0500 Subject: [PATCH 42/46] Use proper type --- Tools/build/check_warnings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 72fb8a6178d294..2052b42d529672 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -162,7 +162,7 @@ def get_unexpected_improvements( if unexpected_improvements: print("Unexpected improvements:") for file in unexpected_improvements: - print(file) + print(file.name) return 1 return 0 @@ -240,7 +240,7 @@ def main(argv: list[str] | None = None) -> int: # where the first element is the file name and the second element # is the number of warnings expected in that file files_with_expected_warnings = [ - FileWarning(file.strip().split()[0], int(file.strip().split()[1])) + FileWarnings(file.strip().split()[0], int(file.strip().split()[1])) for file in clean_files if file.strip() and not file.startswith("#") ] From 26b40d3727b5bab6c88a37838b7d80f038f4a8a8 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 13 Aug 2024 11:54:43 -0500 Subject: [PATCH 43/46] Update function signatures and make files with expected warnings list a true set --- Tools/build/check_warnings.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 2052b42d529672..3cedd92c411314 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -115,7 +115,7 @@ def get_warnings_by_file(warnings: list[dict]) -> dict[str, list[dict]]: def get_unexpected_warnings( files_with_expected_warnings: set[tuple[str, int]], - files_with_warnings: dict[str, list[dict]], + files_with_warnings: set[FileWarnings], ) -> int: """ Returns failure status if warnings discovered in list of warnings @@ -145,7 +145,7 @@ def get_unexpected_warnings( def get_unexpected_improvements( files_with_expected_warnings: set[tuple[str, int]], - files_with_warnings: dict[str, list[dict]], + files_with_warnings: set[FileWarnings], ) -> int: """ Returns failure status if there are no warnings in the list of warnings @@ -155,8 +155,7 @@ def get_unexpected_improvements( for file in files_with_expected_warnings: if file.name not in files_with_warnings.keys(): unexpected_improvements.append(file) - else: - if len(files_with_warnings[file.name]) < file.count: + elif len(files_with_warnings[file.name]) < file.count: unexpected_improvements.append(file) if unexpected_improvements: @@ -239,11 +238,11 @@ def main(argv: list[str] | None = None) -> int: # Files with expected warnings are stored as a set of tuples # where the first element is the file name and the second element # is the number of warnings expected in that file - files_with_expected_warnings = [ + files_with_expected_warnings = { FileWarnings(file.strip().split()[0], int(file.strip().split()[1])) for file in clean_files if file.strip() and not file.startswith("#") - ] + } with Path(args.compiler_output_file_path).open(encoding="UTF-8") as f: compiler_output_file_contents = f.read() From ca519a3736553bc78de99b693fe6d01a72f8f3b0 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Wed, 14 Aug 2024 10:54:14 -0500 Subject: [PATCH 44/46] Update Tools/build/check_warnings.py Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> --- Tools/build/check_warnings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 3cedd92c411314..39433557b1a367 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -144,7 +144,7 @@ def get_unexpected_warnings( def get_unexpected_improvements( - files_with_expected_warnings: set[tuple[str, int]], + files_with_expected_warnings: set[FileWarnings], files_with_warnings: set[FileWarnings], ) -> int: """ From f370442f938cc72f6b9a2ca7a429cf28b0883890 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Wed, 14 Aug 2024 10:54:25 -0500 Subject: [PATCH 45/46] Update Tools/build/check_warnings.py Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> --- Tools/build/check_warnings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 39433557b1a367..f62f966c6aab48 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -114,7 +114,7 @@ def get_warnings_by_file(warnings: list[dict]) -> dict[str, list[dict]]: def get_unexpected_warnings( - files_with_expected_warnings: set[tuple[str, int]], + files_with_expected_warnings: set[FileWarnings], files_with_warnings: set[FileWarnings], ) -> int: """ From 644c11709ee5528d02f7437f24ad7f1d0ccb1c8a Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Wed, 14 Aug 2024 14:38:52 -0500 Subject: [PATCH 46/46] Add parsing of option from clang output --- Tools/build/check_warnings.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index f62f966c6aab48..1ed83447b6b668 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -24,7 +24,8 @@ def extract_warnings_from_compiler_output_clang( """ # Regex to find warnings in the compiler output clang_warning_regex = re.compile( - r"(?P.*):(?P\d+):(?P\d+): warning: (?P.*)" + r"(?P.*):(?P\d+):(?P\d+): warning: " + r"(?P.*) (?P