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 <[email protected]>

Author: 3 people

.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

.github/workflows/reusable-ubuntu.yml

@@ -67,17 +67,20 @@ jobs:
       working-directory: ${{ env.CPYTHON_BUILDDIR }}
       run: >-
         ../cpython-ro-srcdir/configure
+        CFLAGS="-fdiagnostics-format=json"
         --config-cache
         --with-pydebug
         --enable-slower-safety
         --with-openssl=$OPENSSL_DIR
         ${{ 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

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.

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.

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())