Skip to content

Commit 4fd12c6

Browse files
etrclaude
etr
and
claude
committed
TASK-060: broaden suppression gate + single-pass awk + unit test
Review follow-up to the initial TASK-060 commit. scripts/check-warning-suppressions.sh - Replace the narrow two-file WATCHED_FILES list with a runtime scan of all .cpp files under src/. Safe because no other TU in src/ carries an unscoped -Warray-bounds pragma today, and the broad scan means future TUs are guarded without anyone having to remember to update a static list. - Collapse the two per-candidate awk invocations (one for nearest push-before, one for nearest pop-after) into a single awk pass that emits both line numbers. Halves the work per candidate and keeps the bracketing logic in one place. - Document the known limitation of the "nearest push before / nearest pop after" heuristic: an interleaved shape (push, pop, pragma, pop) would slip through. Not present in the codebase; upgrade to a depth counter if nested patterns ever appear. Makefile.am - Wire lint-warning-suppressions into check-local. Unlike the other lint-* gates it has no external-tool dependency (bash/awk/grep only), so it can run in every developer build and CI lane, not just the dedicated lint lane. Updated the surrounding comment to record that asymmetry and its rationale. scripts/test_check_warning_suppressions.sh (new) - Self-contained bash unit test that fixtures clean / bracketed / unbracketed / mixed cases in a tmpdir, runs the script against each, and asserts the expected exit code. Not wired into `make check` (it builds throwaway src/ trees that would confuse a parallel `make check` run); intended for direct invocation by contributors editing the gate itself. specs/tasks/M7-v2-cleanup/TASK-060.md / _index.md - Mark all action items checked and flip status to Done. specs/unworked_review_issues/2026-06-04_105130_task-060.md - Reviewer log preserved for the audit trail; the one MAJOR finding (single-pass awk) is addressed by this commit, the 37 MINORs are catalogued for future passes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 9ef16bb commit 4fd12c6

6 files changed

Lines changed: 317 additions & 37 deletions

File tree

‎Makefile.am‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ check-hygiene:
358358

359359
# check-local runs the following static (no-install) checks first:
360360
# check-headers, check-examples, check-readme, check-release-notes,
361-
# check-doxygen, check-hooks-doc-spotcheck
361+
# check-doxygen, check-hooks-doc-spotcheck, lint-warning-suppressions
362362
# Then runs check-install-layout and check-hygiene against a single shared
363363
# staged install to avoid paying two full `make install` costs on every
364364
# `make check`. Both install-backed sub-checks can still be invoked standalone
@@ -368,7 +368,7 @@ check-hygiene:
368368
# staged the install; sub-check skips the install step to avoid double cost.
369369
# This knob is set only by check-local; do not set it when invoking sub-checks
370370
# standalone.
371-
check-local: check-headers check-examples check-readme check-release-notes check-doxygen check-hooks-doc-spotcheck
371+
check-local: check-headers check-examples check-readme check-release-notes check-doxygen check-hooks-doc-spotcheck lint-warning-suppressions
372372
@echo "=== Shared staged install for check-install-layout and check-hygiene ==="
373373
@rm -rf "$(abs_top_builddir)/.shared-check-stage"
374374
@$(MAKE) $(AM_MAKEFLAGS) install DESTDIR="$(abs_top_builddir)/.shared-check-stage" >check-shared-install.log 2>&1 || { \
@@ -464,12 +464,12 @@ lint-file-size:
464464
@echo "=== lint-file-size: enforce per-file line-count ceiling ==="
465465
@$(top_srcdir)/scripts/check-file-size.sh
466466

467-
# TASK-060: file-scoped warning-suppression gate. Fails if any of the
468-
# watched library TUs (currently src/http_utils.cpp and
469-
# src/detail/ip_representation.cpp) regains a top-level
470-
# `#pragma GCC diagnostic ignored "-Warray-bounds"` that is not
471-
# bracketed by a push/pop pair. Not wired into `make check` — invoked
472-
# directly by CI's `lint` lane alongside the other lint-* gates.
467+
# TASK-060: file-scoped warning-suppression gate. Fails if any .cpp
468+
# file under src/ carries a top-level `#pragma GCC diagnostic ignored
469+
# "-Warray-bounds"` that is not bracketed by a push/pop pair. Wired
470+
# into `make check` via check-local (requires only bash/awk/grep, so
471+
# runs in every CI lane and local developer builds without needing
472+
# external tools). Also invoked directly by CI's `lint` lane.
473473
lint-warning-suppressions:
474474
@echo "=== lint-warning-suppressions: forbid file-scoped GCC warning suppressions ==="
475475
@$(top_srcdir)/scripts/check-warning-suppressions.sh

‎scripts/check-warning-suppressions.sh‎

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,14 @@
88
# ignored "-Warray-bounds"` directives that sat at file scope and
99
# silenced the warning for entire translation units. TASK-060 either
1010
# removes them or scopes them to a `push`/`pop` block; this gate makes
11-
# the regression visible by failing if any of the watched files grows a
12-
# new file-scoped suppression.
11+
# the regression visible by failing if any TU in src/ grows a new
12+
# file-scoped suppression.
1313
#
14-
# Watched files: kept narrow on purpose. Future tasks that fix
15-
# additional suppression sites should add their files to WATCHED_FILES
16-
# below; broadening the scan to all of src/ is a separate decision
17-
# (currently other TUs in src/ legitimately carry no such pragmas, so
18-
# the gate would still pass — but adding files here is the conscious,
19-
# auditable choice).
14+
# Watched files: all .cpp files under src/, discovered at runtime via
15+
# `find`. This is safe because at the time TASK-060 landed no TU in
16+
# src/ carried an unscoped -Warray-bounds pragma; the broad scan ensures
17+
# that future work which introduces new TUs is also guarded without
18+
# needing to remember to update a static list.
2019
#
2120
# Detection logic:
2221
# 1. Any `#pragma GCC diagnostic ignored "-Warray-bounds"` at the
@@ -29,6 +28,14 @@
2928
# 3. Any candidate that fails the push/pop bracketing check is
3029
# reported and the script exits 1.
3130
#
31+
# Known limitation: the push/pop check uses "nearest push before" /
32+
# "nearest pop after" heuristics. An interleaved pattern such as
33+
# push@5, pop@8, pragma@10, pop@15 would be incorrectly allowed because
34+
# push_before=5 (non-zero) and pop_after=15 (non-zero). This shape is
35+
# not present in the codebase and is extremely unlikely in practice; if
36+
# the project ever adopts nested or interleaved push/pop patterns,
37+
# upgrade the detection to track bracket depth with a counter.
38+
#
3239
# Exit codes:
3340
# 0 no violations
3441
# 1 one or more watched files carries a file-scoped suppression
@@ -37,11 +44,13 @@ set -euo pipefail
3744
REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)"
3845
cd "$REPO_ROOT"
3946

40-
# Files watched by this gate. Narrow on purpose; see header comment.
41-
WATCHED_FILES=(
42-
"src/http_utils.cpp"
43-
"src/detail/ip_representation.cpp"
44-
)
47+
# Discover all .cpp files under src/ at runtime so new TUs are
48+
# automatically included without requiring a manual list update.
49+
# Use a while-read loop for bash 3.x / macOS compatibility (no mapfile).
50+
WATCHED_FILES=()
51+
while IFS= read -r f; do
52+
WATCHED_FILES+=("$f")
53+
done < <(find src -name '*.cpp' | sort)
4554

4655
echo "check-warning-suppressions: scanning ${#WATCHED_FILES[@]} file(s)"
4756

@@ -56,19 +65,16 @@ for file in "${WATCHED_FILES[@]}"; do
5665
# Each line that begins with the warning-suppression pragma is a
5766
# candidate. We then verify it is bracketed by push/pop.
5867
while IFS=: read -r lineno _; do
59-
# Search backward for the nearest `#pragma GCC diagnostic push`
60-
# before this line, and forward for the nearest `pop` after.
61-
push_before=$(awk -v target="$lineno" '
68+
# Single-pass awk: find the nearest push before and pop after
69+
# the candidate line in one read of the file.
70+
read -r push_before pop_after < <(awk -v target="$lineno" '
6271
/^#pragma[[:space:]]+GCC[[:space:]]+diagnostic[[:space:]]+push/ {
63-
if (NR < target) last = NR
72+
if (NR < target) last_push = NR
6473
}
65-
END { print (last ? last : 0) }
66-
' "$file")
67-
pop_after=$(awk -v target="$lineno" '
6874
/^#pragma[[:space:]]+GCC[[:space:]]+diagnostic[[:space:]]+pop/ {
69-
if (NR > target && first == 0) first = NR
75+
if (NR > target && first_pop == 0) first_pop = NR
7076
}
71-
END { print (first ? first : 0) }
77+
END { print (last_push ? last_push : 0), (first_pop ? first_pop : 0) }
7278
' "$file")
7379

7480
if [ "$push_before" = "0" ] || [ "$pop_after" = "0" ]; then

‎scripts/test_check_warning_suppressions.sh‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
#!/usr/bin/env bash
2+
# Unit test for check-warning-suppressions.sh
3+
# Tests that the push/pop bracketing detection works correctly.
4+
# Run from any directory; uses a temporary directory for fixtures.
5+
set -euo pipefail
6+
7+
PASS=0
8+
FAIL=0
9+
10+
ok() { echo " OK: $1"; PASS=$((PASS + 1)); }
11+
fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); }
12+
13+
SCRIPT="$(cd "$(dirname "$0")" && pwd)/check-warning-suppressions.sh"
14+
15+
# Create a temp work area and override the src/ search inside the script.
16+
# We do this by creating a temp directory tree that mimics src/ and then
17+
# running the script with REPO_ROOT overridden via a symlink approach.
18+
TMPDIR_BASE="$(mktemp -d)"
19+
trap 'rm -rf "$TMPDIR_BASE"' EXIT
20+
21+
FAKE_REPO="$TMPDIR_BASE/repo"
22+
mkdir -p "$FAKE_REPO/scripts" "$FAKE_REPO/src"
23+
24+
# Copy the script so it can be run from the fake repo root.
25+
cp "$SCRIPT" "$FAKE_REPO/scripts/check-warning-suppressions.sh"
26+
chmod +x "$FAKE_REPO/scripts/check-warning-suppressions.sh"
27+
28+
# Test 1: No pragmas — should exit 0
29+
cat > "$FAKE_REPO/src/clean.cpp" <<'EOF'
30+
// no pragmas here
31+
int main() { return 0; }
32+
EOF
33+
if (cd "$FAKE_REPO" && bash scripts/check-warning-suppressions.sh 2>/dev/null); then
34+
ok "no pragmas exits 0"
35+
else
36+
fail "no pragmas should exit 0"
37+
fi
38+
39+
# Test 2: Properly bracketed pragma — should exit 0
40+
cat > "$FAKE_REPO/src/bracketed.cpp" <<'EOF'
41+
// Some code
42+
#pragma GCC diagnostic push
43+
#pragma GCC diagnostic ignored "-Warray-bounds"
44+
int x = arr[5];
45+
#pragma GCC diagnostic pop
46+
int main() { return 0; }
47+
EOF
48+
if (cd "$FAKE_REPO" && bash scripts/check-warning-suppressions.sh 2>/dev/null); then
49+
ok "bracketed pragma exits 0"
50+
else
51+
fail "bracketed pragma should exit 0"
52+
fi
53+
54+
# Test 3: File-scoped pragma (no push/pop) — should exit 1
55+
cat > "$FAKE_REPO/src/unbracketed.cpp" <<'EOF'
56+
#pragma GCC diagnostic ignored "-Warray-bounds"
57+
int arr[5];
58+
int x = arr[10];
59+
int main() { return 0; }
60+
EOF
61+
if ! (cd "$FAKE_REPO" && bash scripts/check-warning-suppressions.sh 2>/dev/null); then
62+
ok "unbracketed pragma exits 1"
63+
else
64+
fail "unbracketed pragma should exit 1"
65+
fi
66+
67+
# Test 4: push before but no pop after — should exit 1
68+
cat > "$FAKE_REPO/src/push_no_pop.cpp" <<'EOF'
69+
#pragma GCC diagnostic push
70+
#pragma GCC diagnostic ignored "-Warray-bounds"
71+
int arr[5];
72+
int main() { return 0; }
73+
EOF
74+
if ! (cd "$FAKE_REPO" && bash scripts/check-warning-suppressions.sh 2>/dev/null); then
75+
ok "push-no-pop exits 1"
76+
else
77+
fail "push-no-pop should exit 1"
78+
fi
79+
80+
# Test 5: pop after but no push before — should exit 1
81+
cat > "$FAKE_REPO/src/no_push_pop.cpp" <<'EOF'
82+
#pragma GCC diagnostic ignored "-Warray-bounds"
83+
int arr[5];
84+
#pragma GCC diagnostic pop
85+
int main() { return 0; }
86+
EOF
87+
if ! (cd "$FAKE_REPO" && bash scripts/check-warning-suppressions.sh 2>/dev/null); then
88+
ok "no-push-pop exits 1"
89+
else
90+
fail "no-push-pop should exit 1"
91+
fi
92+
93+
echo ""
94+
echo "Results: $PASS passed, $FAIL failed"
95+
[ "$FAIL" -eq 0 ]

‎specs/tasks/M7-v2-cleanup/TASK-060.md‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@
88
Eliminate the two unscoped `#pragma GCC diagnostic ignored "-Warray-bounds"` directives flagged as the worst-shaped suppressions in `specs/tasks/v2-branch-gap-audit.md` §1 "HIGH — Unscoped warning suppressions". Either localize each to the minimum offending line range with a `push`/`pop` pair plus a comment explaining the underlying false-positive, or investigate and remove the suppression entirely once the warning is silenced at the source.
99

1010
**Action Items:**
11-
- [ ] Reproduce the `-Warray-bounds` warning at `src/http_utils.cpp:62` under the supported GCC versions used in CI (matrix lanes in `.github/workflows/verify-build.yml`). Capture the exact diagnostic text and offending construct in the commit message.
12-
- [ ] Reproduce the same at `src/detail/ip_representation.cpp:55`. Capture diagnostic text.
13-
- [ ] For each site: either (a) rewrite the source so the warning no longer fires and delete the pragma, or (b) replace the file-scoped pragma with a tightly scoped `#pragma GCC diagnostic push` / `#pragma GCC diagnostic ignored "-Warray-bounds"` / `#pragma GCC diagnostic pop` block around the minimum offending lines, with a comment naming the GCC version range and the upstream report (if any).
14-
- [ ] If the suppression is kept, add a `TODO(TASK-NNN-followup)` style comment only if a concrete follow-up exists; otherwise the scoping comment is sufficient.
15-
- [ ] Add a guard test or static-analysis spot-check that fails if either file regains a file-scoped suppression.
11+
- [x] Reproduce the `-Warray-bounds` warning at `src/http_utils.cpp:62` under the supported GCC versions used in CI (matrix lanes in `.github/workflows/verify-build.yml`). Capture the exact diagnostic text and offending construct in the commit message.
12+
- [x] Reproduce the same at `src/detail/ip_representation.cpp:55`. Capture diagnostic text.
13+
- [x] For each site: either (a) rewrite the source so the warning no longer fires and delete the pragma, or (b) replace the file-scoped pragma with a tightly scoped `#pragma GCC diagnostic push` / `#pragma GCC diagnostic ignored "-Warray-bounds"` / `#pragma GCC diagnostic pop` block around the minimum offending lines, with a comment naming the GCC version range and the upstream report (if any).
14+
- [x] If the suppression is kept, add a `TODO(TASK-NNN-followup)` style comment only if a concrete follow-up exists; otherwise the scoping comment is sufficient.
15+
- [x] Add a guard test or static-analysis spot-check that fails if either file regains a file-scoped suppression.
1616

1717
**Dependencies:**
1818
- Blocked by: None
@@ -28,4 +28,4 @@ Eliminate the two unscoped `#pragma GCC diagnostic ignored "-Warray-bounds"` dir
2828
**Related Requirements:** PRD §2 code quality NFR
2929
**Related Decisions:** None new
3030

31-
**Status:** Backlog
31+
**Status:** Done

‎specs/tasks/M7-v2-cleanup/_index.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ TASK-093).
2424

2525
| ID | Name | Audit grade | Estimate | Status |
2626
|---|---|---|---|---|
27-
| TASK-060 | Scope or remove file-scoped `-Warray-bounds` suppressions | HIGH | S | Backlog |
27+
| TASK-060 | Scope or remove file-scoped `-Warray-bounds` suppressions | HIGH | S | Done |
2828
| TASK-061 | Mechanical cleanup sweep — unfinished prose, orphan comments, stale doc refs | HIGH | S | Backlog |
2929
| TASK-062 | RFC-7616-compliant Digest auth response factory | HIGH | L | Backlog |
3030
| TASK-063 | Honor or remove `http_response::pipe` `size_hint` parameter | HIGH | S | Backlog |

0 commit comments

Comments
 (0)