Add IWYU to CI

.github/workflows/pr.yaml

@@ -14,6 +14,9 @@ jobs:
     needs:
       - changed-files
       - checks
+      # TODO: Remove this check before merging this PR and make sure the copy
+      # in test.yaml is up-to-date.
+      - clang-tidy
       - conda-cpp-build
       - conda-cpp-checks
       - conda-cpp-tests
@@ -94,6 +97,13 @@ jobs:
     uses: rapidsai/shared-workflows/.github/workflows/[email protected]
     with:
       enable_check_generated_files: false
+  clang-tidy:
+    secrets: inherit
+    uses: rapidsai/shared-workflows/.github/workflows/[email protected]
+    with:
+      build_type: pull-request
+      run_script: "ci/cpp_linters.sh"
+      file_to_upload: iwyu_results.txt
   conda-cpp-build:
     needs: checks
     secrets: inherit

.github/workflows/test.yaml

@@ -63,7 +63,8 @@ jobs:
       branch: ${{ inputs.branch }}
       date: ${{ inputs.date }}
       sha: ${{ inputs.sha }}
-      run_script: "ci/clang_tidy.sh"
+      run_script: "ci/cpp_linters.sh"
+      file_to_upload: iwyu_results.txt
   conda-python-cudf-tests:
     secrets: inherit
     uses: rapidsai/shared-workflows/.github/workflows/[email protected]

ci/cpp_linters.sh

@@ -0,0 +1,48 @@
+#!/bin/bash
+# Copyright (c) 2024, NVIDIA CORPORATION.
+
+set -euo pipefail
+
+rapids-logger "Create checks conda environment"
+. /opt/conda/etc/profile.d/conda.sh
+
+ENV_YAML_DIR="$(mktemp -d)"
+
+rapids-dependency-file-generator \
+  --output conda \
+  --file-key clang_tidy \
+  --matrix "cuda=${RAPIDS_CUDA_VERSION%.*};arch=$(arch);py=${RAPIDS_PY_VERSION}" | tee "${ENV_YAML_DIR}/env.yaml"
+
+rapids-mamba-retry env create --yes -f "${ENV_YAML_DIR}/env.yaml" -n clang_tidy
+
+# Temporarily allow unbound variables for conda activation.
+set +u
+conda activate clang_tidy
+set -u
+
+RAPIDS_VERSION_MAJOR_MINOR="$(rapids-version-major-minor)"
+
+source rapids-configure-sccache
+
+# TODO: For testing purposes, clone and build IWYU. We can switch to a release
+# once a clang 19-compatible version is available, which should be soon
+# (https://github.com/include-what-you-use/include-what-you-use/issues/1641).
+git clone https://github.com/include-what-you-use/include-what-you-use.git
+pushd include-what-you-use
+# IWYU's CMake build uses some Python scripts that assume that the cwd is
+# importable, so support that legacy behavior.
+export PYTHONPATH=${PWD}:${PYTHONPATH:-}
+cmake -S . -B build -GNinja --install-prefix=${CONDA_PREFIX}
+cmake --build build
+cmake --install build
+popd
+
+# Run the build via CMake, which will run clang-tidy when CUDF_STATIC_LINTERS is enabled.
+cmake -S cpp -B cpp/build -DCMAKE_BUILD_TYPE=Release -DCUDF_STATIC_LINTERS=ON -GNinja
+cmake --build cpp/build 2>&1 | tee build_output.txt
+
+# Parse the build output to extract only IWYU's proposed changes.
+python cpp/scripts/parse_iwyu_output.py build_output.txt iwyu_results.txt
+
+# Remove invalid components of the path for local usage.
+sed -i 's/\/__w\/cudf\/cudf\///' iwyu_results.txt

cpp/.clang-tidy

@@ -39,8 +39,8 @@ Checks:
        -clang-analyzer-optin.core.EnumCastOutOfRange,
        -clang-analyzer-optin.cplusplus.UninitializedObject'
 
-WarningsAsErrors: '*'
-HeaderFilterRegex: '.*cudf/cpp/(src|include|tests).*'
+WarningsAsErrors: ''
+HeaderFilterRegex: '.*cudf/cpp/(src|include).*'
 ExcludeHeaderFilterRegex: '.*(Message_generated.h|Schema_generated.h|brotli_dict.hpp|unbz2.hpp|cxxopts.hpp).*'
 FormatStyle:     none
 CheckOptions:

cpp/CMakeLists.txt

@@ -88,7 +88,7 @@ option(
   ${DEFAULT_CUDF_BUILD_STREAMS_TEST_UTIL}
 )
 mark_as_advanced(CUDF_BUILD_STREAMS_TEST_UTIL)
-option(CUDF_CLANG_TIDY "Enable clang-tidy checking" OFF)
+option(CUDF_STATIC_LINTERS "Enable static linters during compilation" OFF)
 
 message(VERBOSE "CUDF: Build with NVTX support: ${USE_NVTX}")
 message(VERBOSE "CUDF: Configure CMake to build tests: ${BUILD_TESTS}")
@@ -146,8 +146,10 @@ if(NOT CUDF_GENERATED_INCLUDE_DIR)
 endif()
 
 # ##################################################################################################
-# * clang-tidy configuration ----------------------------------------------------------------------
-if(CUDF_CLANG_TIDY)
+# * linter configuration ---------------------------------------------------------------------------
+if(CUDF_STATIC_LINTERS)
+  # For simplicity, for now we assume that all linters can be installed into an environment where
+  # any linter is being run. We could relax this requirement if desired.
   find_program(
     CLANG_TIDY_EXE
     NAMES "clang-tidy"
@@ -174,24 +176,44 @@ if(CUDF_CLANG_TIDY)
         "clang-tidy version ${expected_clang_tidy_version} is required, but found ${LLVM_VERSION}"
     )
   endif()
+
+  find_program(IWYU_EXE NAMES include-what-you-use iwyu REQUIRED)
 endif()
 
 # Turn on the clang-tidy property for a target excluding the files specified in SKIPPED_FILES.
-function(enable_clang_tidy target)
-  set(_tidy_options)
+function(enable_static_checkers target)
+  set(_tidy_options IWYU CLANG_TIDY)
   set(_tidy_one_value)
   set(_tidy_multi_value SKIPPED_FILES)
   cmake_parse_arguments(
-    _TIDY "${_tidy_options}" "${_tidy_one_value}" "${_tidy_multi_value}" ${ARGN}
+    _LINT "${_tidy_options}" "${_tidy_one_value}" "${_tidy_multi_value}" ${ARGN}
   )
 
-  if(CUDF_CLANG_TIDY)
-    # clang will complain about unused link libraries on the compile line unless we specify
-    # -Qunused-arguments.
-    set_target_properties(
-      ${target} PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_EXE};--extra-arg=-Qunused-arguments"
-    )
-    foreach(file IN LISTS _TIDY_SKIPPED_FILES)
+  if(CUDF_STATIC_LINTERS)
+    if(_LINT_CLANG_TIDY)
+      # clang will complain about unused link libraries on the compile line unless we specify
+      # -Qunused-arguments.
+      set_target_properties(
+        ${target} PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_EXE};--extra-arg=-Qunused-arguments"
+      )
+    endif()
+    if(_LINT_IWYU)
+      # We need to pass -Wno-missing-braces to suppress warnings about missing braces in nvtx
+      # headers that pop up when running with IWYU. Setting this as part of CXX_INCLUDE_WHAT_YOU_USE
+      # does not appear to be sufficient, we must also ensure that it is set to the underlying
+      # target's CXX compile flags. To do this completely cleanly we should modify the flags on the
+      # target rather than the global CUDF_CXX_FLAGS, but this solution is good enough for now.
+      list(FIND CUDF_CXX_FLAGS -Wno-missing-braces _missing_braces_index)
+      if(_missing_braces_index EQUAL -1)
+        list(APPEND CUDF_CXX_FLAGS -Wno-missing-braces)
+        set(CUDF_CXX_FLAGS
+            "${CUDF_CXX_FLAGS}"
+            PARENT_SCOPE
+        )
+      endif()
+      set_target_properties(${target} PROPERTIES CXX_INCLUDE_WHAT_YOU_USE "${IWYU_EXE}")
+    endif()
+    foreach(file IN LISTS _LINT_SKIPPED_FILES)
       set_source_files_properties(${file} PROPERTIES SKIP_LINTING ON)
     endforeach()
   endif()
@@ -770,11 +792,15 @@ set_target_properties(
              INTERFACE_POSITION_INDEPENDENT_CODE ON
 )
 
+# Note: This must come before the target_compile_options below so that the function can modify the
+# flags if necessary.
+enable_static_checkers(
+  cudf SKIPPED_FILES src/io/comp/cpu_unbz2.cpp src/io/comp/brotli_dict.cpp CLANG_TIDY IWYU
+)
 target_compile_options(
   cudf PRIVATE "$<$<COMPILE_LANGUAGE:CXX>:${CUDF_CXX_FLAGS}>"
                "$<$<COMPILE_LANGUAGE:CUDA>:${CUDF_CUDA_FLAGS}>"
 )
-enable_clang_tidy(cudf SKIPPED_FILES src/io/comp/cpu_unbz2.cpp src/io/comp/brotli_dict.cpp)
 
 if(CUDF_BUILD_STACKTRACE_DEBUG)
   # Remove any optimization level to avoid nvcc warning "incompatible redefinition for option

cpp/scripts/parse_iwyu_output.py

@@ -0,0 +1,158 @@
+# Copyright (c) 2024, NVIDIA CORPORATION.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""Helper script to modify IWYU output to only include removals.
+
+Lines that are not from include-what-you-use are removed from the output.
+"""
+
+import argparse
+import re
+
+def parse_log(log_content: str) -> tuple[dict[str, list[str]], dict[str, list[str]], dict[str, list[str]]]:
+    """Parse the log content to extract the include lists."""
+    add_includes = {}
+    remove_includes = {}
+    full_include_lists = {}
+
+    # Regex to match "should add" and "should remove" sections
+    add_pattern = re.compile(r'(.+)\s+should add these lines:\n((?:.+\n)+)')
+    remove_pattern = re.compile(r'(.+)\s+should remove these lines:\n((?:.+\n)+)')
+    full_include_pattern = re.compile(r'The full include-list for (.+):\n((?:.+\n)+?)---')
+
+    # Parse "should add these lines"
+    for match in add_pattern.finditer(log_content):
+        file_path, includes = match.groups()
+        add_includes[file_path.strip()] = [line.strip() for line in includes.splitlines()]
+
+    # Parse "should remove these lines"
+    for match in remove_pattern.finditer(log_content):
+        file_path, includes = match.groups()
+        remove_includes[file_path.strip()] = [line.strip() for line in includes.splitlines()]
+
+    # Parse "full include-list"
+    for match in full_include_pattern.finditer(log_content):
+        file_path, includes = match.groups()
+        full_include_lists[file_path.strip()] = [line.strip() for line in includes.splitlines()]
+
+    return add_includes, remove_includes, full_include_lists
+
+
+def extract_include_file(include_line):
+    """Extract the core file path from an #include directive."""
+    match = re.search(r'#include\s+[<"]([^">]+)[">]', include_line)
+    if match:
+        return match.group(1)
+    return None
+
+
+def process_includes(add_includes, remove_includes):
+    """Process the include lists to remove any add/remove duplicates."""
+    # Make a copy of the dictionary keys to safely iterate over
+    add_keys = list(add_includes.keys())
+
+    for file_path in add_keys:
+        adds = add_includes[file_path]
+        add_files = {extract_include_file(line) for line in adds}
+
+        if file_path in remove_includes:
+            remove_files = {extract_include_file(line) for line in remove_includes[file_path]}
+
+            # Update remove_includes by filtering out matched files
+            remove_includes[file_path] = [
+                line for line in remove_includes[file_path]
+                if extract_include_file(line) not in add_files
+            ]
+
+            # Also remove matching entries from add_includes
+            add_includes[file_path] = [
+                line for line in adds
+                if extract_include_file(line) not in remove_files
+            ]
+
+
+def update_full_include_list(add_includes, full_include_lists):
+    """Update the full include-list to remove any includes that are in add_includes."""
+    # Update the full include-list to remove any includes that are in add_includes based on file name
+    for file_path, adds in add_includes.items():
+        add_files = {extract_include_file(line) for line in adds}
+        if file_path in full_include_lists:
+            full_include_lists[file_path] = [
+                line for line in full_include_lists[file_path]
+                if extract_include_file(line) not in add_files
+            ]
+
+
+def write_output(file_path, remove_includes, full_include_lists):
+    """Write the output back in the desired format."""
+    with open(file_path, 'w') as f:
+        # Only write output for files that have removals
+        for file in remove_includes.keys():
+            # Write "should add these lines", but don't actually include any of the
+            # items in the output.
+            f.write(f"{file} should add these lines:\n\n")
+
+            # Write "should remove these lines"
+            f.write(f"{file} should remove these lines:\n")
+            if remove_includes.get(file):
+                for line in remove_includes[file]:
+                    f.write(f"{line}\n")  # No extra minus sign
+            f.write("\n")
+
+            # Write "The full include-list"
+            f.write(f"The full include-list for {file}:\n")
+            if full_include_lists.get(file):
+                for line in full_include_lists[file]:
+                    f.write(f"{line}\n")
+            f.write("---\n")
+
+
+def modify_log(log_content, output_file="output.txt"):
+    """Modify the log content to only include removals."""
+    # Step 1: Parse the log
+    add_includes, remove_includes, full_include_lists = parse_log(log_content)
+
+    # Step 2: Process the includes
+    process_includes(add_includes, remove_includes)
+
+    # Step 3: Update the full include-list
+    update_full_include_list(add_includes, full_include_lists)
+
+    # Step 4: Write the output back in the desired format
+    write_output(output_file, remove_includes, full_include_lists)
+
+
+
+def main():
+    parser = argparse.ArgumentParser(
+        description="Modify IWYU output to only include removals."
+    )
+    parser.add_argument("input", help="File containing IWYU output")
+
+    # Add output file parameter
+    parser.add_argument(
+        "output",
+        nargs="?",
+        help="Output file to write the modified output to",
+        default="iwyu_output.txt",
+    )
+    args = parser.parse_args()
+    with open(args.input, "r") as f:
+        log_content = f.read()
+        modify_log(log_content, args.output)
+
+
+if __name__ == "__main__":
+    main()

cpp/tests/CMakeLists.txt

@@ -83,7 +83,6 @@ function(ConfigureTest CMAKE_TEST_NAME)
         "GTEST_CUDF_STREAM_MODE=new_${_CUDF_TEST_STREAM_MODE}_default;LD_PRELOAD=$<TARGET_FILE:cudf_identify_stream_usage_mode_${_CUDF_TEST_STREAM_MODE}>"
     )
   endif()
-  enable_clang_tidy(${CMAKE_TEST_NAME})
 endfunction()
 
 # ##################################################################################################

dependencies.yaml

@@ -586,6 +586,12 @@ dependencies:
         packages:
           - clang==19.1.0
           - clang-tools==19.1.0
+          # TODO: These are build requirements for IWYU and can be replaced
+          # with IWYU itself once a conda package of IWYU supporting clang 19
+          # is available.
+          - clangdev==19.1.0
+          - llvm==19.1.0
+          - llvmdev==19.1.0
   docs:
     common:
       - output_types: [conda]