From 8ac5565be2e5a11fad643c2fe9cbf16d2ddb95cd Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Sat, 27 Jul 2024 04:57:44 -0500 Subject: [PATCH] gh-112301: Compiler warning management tooling (#121730) Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> --- .github/workflows/build.yml | 2 +- .github/workflows/reusable-ubuntu.yml | 5 +- ...-07-13-21-55-58.gh-issue-112301.YJS1dl.rst | 2 + Tools/build/.warningignore_ubuntu | 3 + Tools/build/check_warnings.py | 195 ++++++++++++++++++ 5 files changed, 205 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Tests/2024-07-13-21-55-58.gh-issue-112301.YJS1dl.rst create mode 100644 Tools/build/.warningignore_ubuntu create mode 100644 Tools/build/check_warnings.py diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 613578ae176..6568b50c3a6 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -348,7 +348,7 @@ jobs: with: save: false - name: Configure CPython - run: ./configure --config-cache --enable-slower-safety --with-pydebug --with-openssl=$OPENSSL_DIR + run: ./configure CFLAGS="-fdiagnostics-format=json" --config-cache --enable-slower-safety --with-pydebug --with-openssl=$OPENSSL_DIR - name: Build CPython run: make -j4 - name: Display build info diff --git a/.github/workflows/reusable-ubuntu.yml b/.github/workflows/reusable-ubuntu.yml index 54d7765d159..c6289a74e9a 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 @@ -74,10 +75,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/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 00000000000..d5718ed4be7 --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2024-07-13-21-55-58.gh-issue-112301.YJS1dl.rst @@ -0,0 +1,2 @@ +Add tooling to check for changes in compiler warnings. +Patch by Nate Ohlson. diff --git a/Tools/build/.warningignore_ubuntu b/Tools/build/.warningignore_ubuntu new file mode 100644 index 00000000000..8242c8d17c8 --- /dev/null +++ b/Tools/build/.warningignore_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 00000000000..f0c0067f4ab --- /dev/null +++ b/Tools/build/check_warnings.py @@ -0,0 +1,195 @@ +#!/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 argparse +import json +import re +import sys +from pathlib import Path + + +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. + """ + + # 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 + + +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 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 + """ + unexpected_improvements = [] + for file in files_with_expected_warnings: + if file not in files_with_warnings.keys(): + unexpected_improvements.append(file) + + if unexpected_improvements: + print("Unexpected improvements:") + for file in unexpected_improvements: + print(file) + 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 compiler output file", + ) + parser.add_argument( + "--warning-ignore-file-path", + type=str, + 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) + + exit_code = 0 + + # Check that the compiler output file is a valid path + if not Path(args.compiler_output_file_path).is_file(): + print( + "Compiler output file does not exist: " + f"{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( + "Warning ignore file does not exist: " + f"{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 = { + file.strip() + for file in clean_files + if file.strip() and not file.startswith("#") + } + + 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, files_with_warnings + ) + if args.fail_on_regression: + exit_code |= status + + status = get_unexpected_improvements( + warnings, files_with_expected_warnings, files_with_warnings + ) + if args.fail_on_improvement: + exit_code |= status + + return exit_code + + +if __name__ == "__main__": + sys.exit(main())