fix(pre-commit): normalize SPDX handling#3081
Conversation
Signed-off-by: Bradley Dice <bdice@bradleydice.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesClang-format configuration
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cpp/.clang-format`:
- Line 59: Update the CommentPragmas regex in the clang-format configuration to
anchor both recognized pragmas at the start of the comment text, preserving the
existing IWYU pragma and SPDX matches while preventing incidental prose from
being treated as a pragma.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c8113cb7-4cf1-426d-9764-e0ecee927503
📒 Files selected for processing (1)
cpp/.clang-format
| BreakStringLiterals: true | ||
| ColumnLimit: 100 | ||
| CommentPragmas: '^ ?[*]? ?(IWYU pragma:|SPDX-)' | ||
| CommentPragmas: '(IWYU pragma:|SPDX-)' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
tmp=$(mktemp --suffix=.cc)
trap 'rm -f "$tmp"' EXIT
cat >"$tmp" <<'EOF'
/* This ordinary comment mentions SPDX- in prose and should remain reflowable. */
void f();
EOF
clang-format --version
clang-format -style=file -assume-filename=cpp/comment-pragmas.cc "$tmp"Repository: NVIDIA/raft
Length of output: 200
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '--- cpp/.clang-format ---\n'
sed -n '1,120p' cpp/.clang-format
printf '\n--- references to CommentPragmas / clang-format ---\n'
rg -n "CommentPragmas|clang-format|-style=file" -S . --glob '!**/.git/**'Repository: NVIDIA/raft
Length of output: 240
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
import re
samples = [
"/* This ordinary comment mentions SPDX- in prose and should remain reflowable. */",
"// SPDX-123",
"// This documents SPDX- headers",
"/* IWYU pragma: keep */",
"/* note: IWYU pragma: keep in prose */",
]
patterns = {
"unanchored": r"(IWYU pragma:|SPDX-)",
"anchored_original": r"^(IWYU pragma:|SPDX-)",
"proposed": r"^ *[*]? *(IWYU pragma:|SPDX-)",
}
for name, pat in patterns.items():
rx = re.compile(pat)
print(f"[{name}] {pat}")
for s in samples:
print(f" {bool(rx.search(s))} {s}")
print()
PYRepository: NVIDIA/raft
Length of output: 966
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '--- cpp/.clang-format ---\n'
cat -n cpp/.clang-format | sed -n '1,120p'
printf '\n--- references to CommentPragmas / clang-format ---\n'
rg -n "CommentPragmas|clang-format|-style=file" -S . --glob '!**/.git/**'Repository: NVIDIA/raft
Length of output: 240
🌐 Web query:
clang-format CommentPragmas regex matched against comment text leading // stripped before matching
💡 Result:
In clang-format, the CommentPragmas regular expression is matched against the text of the comment after the leading comment markers (such as // or /*) and any associated whitespace have been stripped [1][2]. When clang-format processes comments, it uses the CommentPragmas regex to identify lines that should not be reflowed or split into multiple lines [3][4][5]. The source code implementation of the breakable token logic indicates that for line comments, the content is analyzed after the comment marker is removed [1][2]. Specifically, internal logic such as that found in BreakableComment and related classes handles the matching process by evaluating the content of the comment line against the provided regex, effectively ignoring the comment indicator itself [1][6]. It is important to note that while CommentPragmas prevents clang-format from breaking or reflowing these comments, it does not prevent the formatter from adjusting the indentation of the comment to align with the surrounding code [5]. As a result, even if a comment matches the CommentPragmas regex, it may still be moved or re-indented by clang-format if the surrounding code structure changes [5].
Citations:
- 1: https://github.com/llvm/llvm-project/blob/d5f223da/clang/lib/Format/BreakableToken.cpp
- 2: llvm/llvm-project@17725d8
- 3: https://releases.llvm.org/16.0.0/tools/clang/docs/ClangFormatStyleOptions.html
- 4: https://releases.llvm.org/10.0.0/tools/clang/docs/ClangFormatStyleOptions.html
- 5: https://stackoverflow.com/questions/51944227/clang-format-seems-to-ignore-commentpragmas
- 6: https://clang.llvm.org/doxygen/classclang_1_1format_1_1BreakableComment.html
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Read the relevant formatter setting and any in-repo pragma examples.
sed -n '50,70p' cpp/.clang-format
printf '\n--- pragma examples ---\n'
rg -n "IWYU pragma:|SPDX-" cpp -SRepository: NVIDIA/raft
Length of output: 50368
Keep CommentPragmas anchored to the start of the comment text. Clang-format strips the comment marker before applying this regex, so removing ^ makes incidental prose containing SPDX- or IWYU pragma: act like a pragma and skip reflow.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cpp/.clang-format` at line 59, Update the CommentPragmas regex in the
clang-format configuration to anchor both recognized pragmas at the start of the
comment text, preserving the existing IWYU pragma and SPDX matches while
preventing incidental prose from being treated as a pragma.
1 participant
Description
Contributes to rapidsai/build-planning#303.
Standardizes clang-format's SPDX protection and ensures the repository uses
rapidsai/pre-commit-hooksv1.6.0. It also restores malformed SPDX headers where present.Validated with
pre-commit run --all-files.