From 7128373ce2c041974b43e18dd987b5931fbd280a Mon Sep 17 00:00:00 2001 From: Christian Vetter Date: Wed, 12 Aug 2026 19:11:22 +0200 Subject: [PATCH 01/19] tests: fail the gate on an nsedge fixture with no goldens --- .github/workflows/ci.yml | 2 +- CONTRIBUTING.md | 7 ++++ check.sh | 17 ++++++--- tests/check_fixture_coverage.sh | 68 +++++++++++++++++++++++++++++++++ tests/regen_goldens.sh | 5 +++ 5 files changed, 93 insertions(+), 6 deletions(-) create mode 100755 tests/check_fixture_coverage.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 790add3..adc621a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,7 +28,7 @@ jobs: - uses: actions/checkout@v4 - uses: ./.github/actions/setup-toolchain - name: Quality gate minus tidy (./check.sh) - run: RAPIDPROTO_GATE_STAGES='format docs gcc clang cf fuzz differential' ./check.sh + run: RAPIDPROTO_GATE_STAGES='format docs fixtures gcc clang cf fuzz cxx20 differential' ./check.sh # The real-world compatibility check (see tests/corpus_gate.py for what it covers and why). # Its own job because it needs a ~100 MB fetch the other jobs don't, keeping that off the main diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1fc207e..e3b7380 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -146,6 +146,13 @@ arena layout plan — all dumped to text and compared byte-for-byte). After an * to a generator or a dumper, regenerate with `tests/regen_goldens.sh`, then run `./check.sh` and review the diff by hand. Never hand-edit a file under `tests/*_golden/`. +Adding a schema to `tests/corpus/nsedge/` needs one extra step: the regen scripts only *overwrite* +goldens that already exist, so seed each one once by running `rapidprotoc` directly, and add the +fixture to `tests/regen_goldens.sh`, `tests/regen_arenagen_goldens.sh`, and the case list plus +`#include` in the matching test file. `tests/check_fixture_coverage.sh` (a gate stage) fails until +the fixture is referenced by both regen scripts and both goldens exist — without it a new fixture +is silently unpinned, since a package/namespace shape can fail in ways no compiler reports. + ## Style & scope - All hand-written code is `clang-format`ed; the gate enforces it, and nothing is exempt. Comments diff --git a/check.sh b/check.sh index e80a15e..5c4aef2 100755 --- a/check.sh +++ b/check.sh @@ -14,8 +14,8 @@ # # (three decode paths + the schema front-end). Slow (three instrumented builds). # # Override: FUZZ_TIME=120 COV_FLOOR=88. # -# The independent stages (format, doc-links, gcc build+test, clang build+test, compile-fail, -# fuzz-compile, clang-tidy) run concurrently; each build is a parallel build and clang-tidy is +# The independent stages (format, doc-links, fixture-coverage, gcc build+test, clang build+test, +# compile-fail, fuzz-compile, clang-tidy) run concurrently; each build is a parallel build and clang-tidy is # parallelized across files. The corpus stage is the exception: it consumes the gcc stage's # rapidprotoc, so it runs after them. Per-stage output is captured and printed in a fixed # order so nothing interleaves. Exits non-zero if anything is not clean. @@ -240,6 +240,9 @@ job_doc_links() { python3 tests/check_doc_links.py } +job_fixtures() { + tests/check_fixture_coverage.sh +} job_build_test() { # $1 = preset; parallel build, then run the test binary local preset=$1 build_out test_out rc @@ -429,10 +432,10 @@ job_differential() { python3 tests/differential.py --build-dir ./build/gcc } -# Which of the ten gate stages run (default: all). CI splits them across runner jobs -- the +# Which of the eleven gate stages run (default: all). CI splits them across runner jobs -- the # build/test stages in one, tidy shards in a matrix -- so wall-clock is the slowest runner. stage_enabled() { - [[ " ${RAPIDPROTO_GATE_STAGES:-format docs gcc clang cf fuzz tidy corpus cxx20 differential} " == *" $1 "* ]] + [[ " ${RAPIDPROTO_GATE_STAGES:-format docs fixtures gcc clang cf fuzz tidy corpus cxx20 differential} " == *" $1 "* ]] } run_stage() { # $1 stage key, $2 log name, rest: the job command local key=$1 log=$2; shift 2 @@ -454,6 +457,7 @@ if [[ "${RAPIDPROTO_GATE_SERIAL:-${GITHUB_ACTIONS:+1}}" == "1" ]]; then # job anyway, the last line names the guilty stage. echo "serial gate: format"; run_stage format "format" job_format; rc_format=$? echo "serial gate: doc-links"; run_stage docs "docs" job_doc_links; rc_docs=$? + echo "serial gate: fixtures"; run_stage fixtures "fixtures" job_fixtures; rc_fixtures=$? echo "serial gate: build gcc"; run_stage gcc "gcc" job_build_test gcc; rc_gcc=$? echo "serial gate: build clang"; run_stage clang "clang" job_build_test clang; rc_clang=$? echo "serial gate: compile-fail"; run_stage cf "cf" job_compile_fail; rc_cf=$? @@ -462,6 +466,7 @@ if [[ "${RAPIDPROTO_GATE_SERIAL:-${GITHUB_ACTIONS:+1}}" == "1" ]]; then else run_stage format "format" job_format & p_format=$! run_stage docs "docs" job_doc_links & p_docs=$! + run_stage fixtures "fixtures" job_fixtures & p_fixtures=$! run_stage gcc "gcc" job_build_test gcc & p_gcc=$! run_stage clang "clang" job_build_test clang & p_clang=$! run_stage cf "cf" job_compile_fail & p_cf=$! @@ -470,6 +475,7 @@ else wait "$p_format"; rc_format=$? wait "$p_docs"; rc_docs=$? + wait "$p_fixtures"; rc_fixtures=$? wait "$p_gcc"; rc_gcc=$? wait "$p_clang"; rc_clang=$? wait "$p_cf"; rc_cf=$? @@ -489,6 +495,7 @@ run_stage differential "differential" job_differential; rc_differential=$? section "generated headers at c++20/c++23"; cat "$LOG/cxx20" section "clang-format (check)"; cat "$LOG/format" section "doc links"; cat "$LOG/docs" +section "corpus fixture coverage"; cat "$LOG/fixtures" section "build + test (gcc)"; cat "$LOG/gcc" section "build + test (clang)"; cat "$LOG/clang" section "compile-fail (generated decoder rejects misuse)"; cat "$LOG/cf" @@ -498,7 +505,7 @@ section "real-world schema corpus"; cat "$LOG/corpus" section "randomized differential vs protobuf"; cat "$LOG/differential" fail=0 -for rc in "$rc_format" "$rc_docs" "$rc_gcc" "$rc_clang" "$rc_cf" "$rc_fuzz" "$rc_tidy" \ +for rc in "$rc_format" "$rc_docs" "$rc_fixtures" "$rc_gcc" "$rc_clang" "$rc_cf" "$rc_fuzz" "$rc_tidy" \ "$rc_corpus" "$rc_cxx20" "$rc_differential"; do [[ "$rc" -ne 0 ]] && fail=1 done diff --git a/tests/check_fixture_coverage.sh b/tests/check_fixture_coverage.sh new file mode 100755 index 0000000..bc381d5 --- /dev/null +++ b/tests/check_fixture_coverage.sh @@ -0,0 +1,68 @@ +#!/usr/bin/env bash +# +# Every tests/corpus/nsedge/*.proto must be regenerated by the golden scripts and have a checked-in +# streamgen AND arenagen golden. +# +# Why a dedicated check: nothing else notices a fixture that has NO golden. regen_goldens.sh only +# flags a golden that exists but was not regenerated -- the reverse direction -- and the per-model +# case lists in test_streamgen.cpp / test_arenagen.cpp / test_dumpgen.cpp are hand-maintained. So a +# .proto dropped into nsedge/ silently gets no golden and no streaming coverage at all. That has +# happened twice. (The differential does pick up any corpus schema by rglob and compiles it under +# --arena --dump, so the gap is narrower than "untested" -- but it pins no generated text, and it +# skips itself when protoc is absent.) +# +# nsedge is where package/namespace SHAPES are pinned, and several of them fail in ways no compiler +# can see: a package named `std` emits `namespace std { ... }`, undefined behaviour that compiles +# without a diagnostic. A byte-compared golden is the only thing that catches a regression there. +# +# Checking the regen reference, not just the file, is what makes this more than a touch-two-files +# test: it ties the fixture to the script that rebuilds its golden, and it stops an unrelated +# directory's golden of the same stem from satisfying the rule (golden paths are a flat namespace, +# so tests/corpus/nsedge/proto3.proto would otherwise be "covered" by tests/corpus/proto3.proto's). +# +# The dump model is deliberately not required. Its namespace comes from the same shared +# CppNameTable the arena golden pins, so a dump-only namespace regression is not reachable. Add a +# dump golden anyway when a fixture's dump OUTPUT is the point (stdpkg/rppkg have one). +set -euo pipefail +cd "$(dirname "$0")/.." + +readonly NSEDGE=tests/corpus/nsedge + +mapfile -t protos < <(find "$NSEDGE" -name '*.proto' | sort) +if [[ ${#protos[@]} -eq 0 ]]; then + echo ">> no fixtures found under $NSEDGE/ -- this check is looking in the wrong place" + exit 1 +fi + +missing=0 +for proto in "${protos[@]}"; do + stem="$(basename "$proto" .proto)" + + # -s, not -f: an empty golden is not a golden. + for want in "tests/streamgen_golden/$stem.rp.stream.hpp|--stream" \ + "tests/arenagen_golden/$stem.rp.hpp|--arena"; do + if [[ ! -s "${want%%|*}" ]]; then + echo ">> $proto has no ${want##*|} golden (${want%%|*})" + missing=1 + fi + done + + # Regenerated either directly, or transitively as an import of a fixture that is (deep.proto is + # pulled in by xpkg.proto and has no entry of its own). + imported_by="$(grep -l "import \"$stem.proto\"" "$NSEDGE"/*.proto 2>/dev/null || true)" + for script in tests/regen_goldens.sh tests/regen_arenagen_goldens.sh; do + if ! grep -q "$NSEDGE/$stem.proto" "$script" && [[ -z "$imported_by" ]]; then + echo ">> $proto is not regenerated by $script (no entry, and nothing imports it)" + missing=1 + fi + done +done + +if [[ $missing -ne 0 ]]; then + echo ">> Add the fixture to tests/regen_goldens.sh and tests/regen_arenagen_goldens.sh, plus its" + echo ">> case list and #include in the matching test file. Seed each golden once by running" + echo ">> rapidprotoc directly (the regen scripts only overwrite goldens that already exist)," + echo ">> then re-run tests/regen_goldens.sh." + exit 1 +fi +echo "fixture coverage: ${#protos[@]} nsedge schemas, each regenerated and pinned by stream + arena goldens" diff --git a/tests/regen_goldens.sh b/tests/regen_goldens.sh index d9ccb2e..01768d9 100755 --- a/tests/regen_goldens.sh +++ b/tests/regen_goldens.sh @@ -95,4 +95,9 @@ echo "[5/5] regenerating AST + wire + arena-layout + common goldens via the test RAPIDPROTO_REGEN_GOLDEN=1 ./build/gcc/rapidproto_tests "[golden],[wire-golden],[arena-layout],[common]" 2>&1 | grep -i "regenerated" || true +# The reverse of the orphan check at [2/5], which only flags a golden that exists but was not +# regenerated: this catches a fixture with no golden at all. Shared with check.sh (the `fixtures` +# gate stage, which CI runs) so it holds even when nobody runs this script. +tests/check_fixture_coverage.sh + echo "done -- review the diff (git diff), then run ./check.sh to confirm." From a700c33c24f3367e25f2afe68200247d8746c6e5 Mon Sep 17 00:00:00 2001 From: Christian Vetter Date: Wed, 12 Aug 2026 20:01:02 +0200 Subject: [PATCH 02/19] check.sh: drive stages from one table, keep logs, name failures --- check.sh | 170 +++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 116 insertions(+), 54 deletions(-) diff --git a/check.sh b/check.sh index 5c4aef2..31c5e9f 100755 --- a/check.sh +++ b/check.sh @@ -14,6 +14,13 @@ # # (three decode paths + the schema front-end). Slow (three instrumented builds). # # Override: FUZZ_TIME=120 COV_FLOOR=88. # +# Every stage's output is captured to build/gate-logs/ and kept after the run, so a failure +# you piped past can be read back instead of re-running the gate. The summary names the stages that +# failed, how long each took, and how many ran -- `./check.sh | tail -20` is enough to triage. +# +# RAPIDPROTO_GATE_STAGES='gcc tidy' runs a subset (space-separated; an unknown key is an error). +# RAPIDPROTO_GATE_SERIAL=1 runs stages one at a time (the default under GITHUB_ACTIONS). +# # The independent stages (format, doc-links, fixture-coverage, gcc build+test, clang build+test, # compile-fail, fuzz-compile, clang-tidy) run concurrently; each build is a parallel build and clang-tidy is # parallelized across files. The corpus stage is the exception: it consumes the gcc stage's @@ -204,8 +211,11 @@ if [[ "${1:-}" == "deep" ]]; then exit "$deep_fail" fi -LOG="$(mktemp -d)" -trap 'rm -rf "$LOG"' EXIT +# Kept, not deleted: after a long run the useful next step is almost always "read the stage that +# failed", and a deleted log means re-running the whole gate to see it again. One fixed path so the +# summary can name it (and so a second run does not accumulate directories). +LOG="build/gate-logs" +rm -rf "$LOG"; mkdir -p "$LOG" # A CI kill (OOM, preemption, cancellation) arrives as SIGTERM and would discard every buffered # stage log -- exactly when the logs matter most. Dump whatever was captured before dying. trap 'echo ">> check.sh: killed (SIGTERM/SIGINT) -- dumping captured stage logs"; for f in "$LOG"/*; do [[ -f "$f" ]] && { echo "--- ${f##*/} ---"; cat "$f"; }; done; exit 143' TERM INT @@ -432,18 +442,79 @@ job_differential() { python3 tests/differential.py --build-dir ./build/gcc } -# Which of the eleven gate stages run (default: all). CI splits them across runner jobs -- the -# build/test stages in one, tidy shards in a matrix -- so wall-clock is the slowest runner. +# THE stage table. Adding a gate stage means one key here, one title, one job -- and nothing else: +# the run loops, the log capture, the printing and the pass/fail aggregation are all driven off this +# list. The previous shape needed six separate edits, two of which (the default allow-list and the +# rc_ aggregation) failed SILENTLY when missed, leaving a stage that could only report success. +readonly STAGE_KEYS=(format docs fixtures gcc clang cf fuzz tidy corpus cxx20 differential) + +stage_title() { + case $1 in + format) echo "clang-format (check)" ;; + docs) echo "doc links" ;; + fixtures) echo "corpus fixture coverage" ;; + gcc) echo "build + test (gcc)" ;; + clang) echo "build + test (clang)" ;; + cf) echo "compile-fail (generated decoder rejects misuse)" ;; + fuzz) echo "fuzz harness compile-check" ;; + tidy) echo "clang-tidy (library = strict, tests = relaxed)" ;; + corpus) echo "real-world schema corpus" ;; + cxx20) echo "generated headers at c++20/c++23" ;; + differential) echo "randomized differential vs protobuf" ;; + esac +} + +stage_job() { + case $1 in + format) job_format ;; + docs) job_doc_links ;; + fixtures) job_fixtures ;; + gcc) job_build_test gcc ;; + clang) job_build_test clang ;; + cf) job_compile_fail ;; + fuzz) job_fuzz_compile ;; + tidy) job_tidy ;; + corpus) job_corpus ;; + cxx20) job_cxx20_smoke ;; + differential) job_differential ;; + esac +} + +# Which stages run (default: all). CI splits them across runner jobs -- the build/test stages in one, +# tidy shards in a matrix -- so wall-clock is the slowest runner. An unknown key is a hard error: a +# comma instead of a space, or one typo, used to skip EVERY stage and report ALL GREEN in a second. +if [[ -n "${RAPIDPROTO_GATE_STAGES:-}" ]]; then + for want in $RAPIDPROTO_GATE_STAGES; do + known=0 + for key in "${STAGE_KEYS[@]}"; do [[ "$want" == "$key" ]] && known=1; done + if [[ $known -eq 0 ]]; then + echo ">> unknown gate stage '$want' in RAPIDPROTO_GATE_STAGES" >&2 + echo ">> valid stages (space-separated): ${STAGE_KEYS[*]}" >&2 + exit 2 + fi + done +fi + stage_enabled() { - [[ " ${RAPIDPROTO_GATE_STAGES:-format docs fixtures gcc clang cf fuzz tidy corpus cxx20 differential} " == *" $1 "* ]] + [[ " ${RAPIDPROTO_GATE_STAGES:-${STAGE_KEYS[*]}} " == *" $1 "* ]] } -run_stage() { # $1 stage key, $2 log name, rest: the job command - local key=$1 log=$2; shift 2 - if stage_enabled "$key"; then - "$@" >"$LOG/$log" 2>&1 - else - echo "stage skipped (RAPIDPROTO_GATE_STAGES)" >"$LOG/$log" + +# Records the outcome and duration BESIDE the log, because a concurrent stage runs in a subshell and +# cannot assign to a parent variable -- which is exactly how a hand-written rc_ came to be +# forgotten. Reading it back from disk means an unrecorded stage is visibly absent, not silently 0. +run_stage() { # $1 = stage key + local key=$1 start rc + if ! stage_enabled "$key"; then + echo "stage skipped (RAPIDPROTO_GATE_STAGES)" >"$LOG/$key" + echo skipped >"$LOG/$key.rc" + return 0 fi + start=$SECONDS + stage_job "$key" >"$LOG/$key" 2>&1 + rc=$? + echo "$rc" >"$LOG/$key.rc" + echo "$((SECONDS - start))" >"$LOG/$key.dur" + return "$rc" } # --- run all stages, capturing each to its own log ------------------------------------------------ @@ -455,65 +526,56 @@ run_stage() { # $1 stage key, $2 log name, rest: the job command if [[ "${RAPIDPROTO_GATE_SERIAL:-${GITHUB_ACTIONS:+1}}" == "1" ]]; then # Progress lines go straight to stdout (stage output stays buffered): if the runner kills the # job anyway, the last line names the guilty stage. - echo "serial gate: format"; run_stage format "format" job_format; rc_format=$? - echo "serial gate: doc-links"; run_stage docs "docs" job_doc_links; rc_docs=$? - echo "serial gate: fixtures"; run_stage fixtures "fixtures" job_fixtures; rc_fixtures=$? - echo "serial gate: build gcc"; run_stage gcc "gcc" job_build_test gcc; rc_gcc=$? - echo "serial gate: build clang"; run_stage clang "clang" job_build_test clang; rc_clang=$? - echo "serial gate: compile-fail"; run_stage cf "cf" job_compile_fail; rc_cf=$? - echo "serial gate: fuzz-compile"; run_stage fuzz "fuzz" job_fuzz_compile; rc_fuzz=$? - echo "serial gate: tidy"; run_stage tidy "tidy" job_tidy; rc_tidy=$? + for key in format docs fixtures gcc clang cf fuzz tidy; do + echo "serial gate: $key" + run_stage "$key" + done else - run_stage format "format" job_format & p_format=$! - run_stage docs "docs" job_doc_links & p_docs=$! - run_stage fixtures "fixtures" job_fixtures & p_fixtures=$! - run_stage gcc "gcc" job_build_test gcc & p_gcc=$! - run_stage clang "clang" job_build_test clang & p_clang=$! - run_stage cf "cf" job_compile_fail & p_cf=$! - run_stage fuzz "fuzz" job_fuzz_compile & p_fuzz=$! - run_stage tidy "tidy" job_tidy & p_tidy=$! - - wait "$p_format"; rc_format=$? - wait "$p_docs"; rc_docs=$? - wait "$p_fixtures"; rc_fixtures=$? - wait "$p_gcc"; rc_gcc=$? - wait "$p_clang"; rc_clang=$? - wait "$p_cf"; rc_cf=$? - wait "$p_fuzz"; rc_fuzz=$? - wait "$p_tidy"; rc_tidy=$? + stage_pids=() + for key in format docs fixtures gcc clang cf fuzz tidy; do + run_stage "$key" & stage_pids+=("$!") + done + # Outcomes come from $LOG/.rc, not from wait: each stage records its result where every + # consumer reads it, so there is no second place to keep in sync. + for pid in "${stage_pids[@]}"; do wait "$pid" || true; done fi # After the build stages, never alongside them: this one consumes build/gcc's rapidprotoc. -run_stage corpus "corpus" job_corpus; rc_corpus=$? +run_stage corpus # Needs the goldens on disk (not a build product), so it can run any time after them. -run_stage cxx20 "cxx20" job_cxx20_smoke; rc_cxx20=$? +run_stage cxx20 # Also consumes build/gcc's binaries, and compiles a harness per schema, so it runs alone at the end. -run_stage differential "differential" job_differential; rc_differential=$? +run_stage differential # --- print each stage's output in a fixed order (already captured, so never interleaved) ---------- -section "generated headers at c++20/c++23"; cat "$LOG/cxx20" -section "clang-format (check)"; cat "$LOG/format" -section "doc links"; cat "$LOG/docs" -section "corpus fixture coverage"; cat "$LOG/fixtures" -section "build + test (gcc)"; cat "$LOG/gcc" -section "build + test (clang)"; cat "$LOG/clang" -section "compile-fail (generated decoder rejects misuse)"; cat "$LOG/cf" -section "fuzz harness compile-check"; cat "$LOG/fuzz" -section "clang-tidy (library = strict, tests = relaxed)"; cat "$LOG/tidy" -section "real-world schema corpus"; cat "$LOG/corpus" -section "randomized differential vs protobuf"; cat "$LOG/differential" +for key in cxx20 format docs fixtures gcc clang cf fuzz tidy corpus differential; do + section "$(stage_title "$key")" + cat "$LOG/$key" +done fail=0 -for rc in "$rc_format" "$rc_docs" "$rc_fixtures" "$rc_gcc" "$rc_clang" "$rc_cf" "$rc_fuzz" "$rc_tidy" \ - "$rc_corpus" "$rc_cxx20" "$rc_differential"; do - [[ "$rc" -ne 0 ]] && fail=1 +failed_stages=(); ran=0; skipped=() +for key in "${STAGE_KEYS[@]}"; do + rc="$(cat "$LOG/$key.rc" 2>/dev/null || echo missing)" + case "$rc" in + 0) ran=$((ran + 1)) ;; + skipped) skipped+=("$key") ;; + # "missing" lands here too: a stage that never recorded a result must not read as a pass. + *) ran=$((ran + 1)); fail=1; failed_stages+=("$key") ;; + esac done section "summary" +for key in "${STAGE_KEYS[@]}"; do + dur="$(cat "$LOG/$key.dur" 2>/dev/null || true)" + [[ -n "$dur" ]] && printf ' %-13s %4ss\n' "$key" "$dur" +done +echo "ran $ran/${#STAGE_KEYS[@]} stages${skipped:+ (skipped: ${skipped[*]})}" +echo "stage logs: $LOG" if [[ "$fail" == "0" ]]; then echo "ALL GREEN" else - echo "FAILURES above" + echo "FAILURES: ${failed_stages[*]}" fi exit "$fail" From 59a47fecf94d2258ace83937a4eb582aebccdf91 Mon Sep 17 00:00:00 2001 From: Christian Vetter Date: Wed, 12 Aug 2026 20:48:47 +0200 Subject: [PATCH 03/19] gate: move the corpus sweep to deep, parallelise the differential --- check.sh | 23 ++++++++++++-- tests/differential.py | 74 +++++++++++++++++++++++++++++-------------- 2 files changed, 72 insertions(+), 25 deletions(-) diff --git a/check.sh b/check.sh index 31c5e9f..1ff37e4 100755 --- a/check.sh +++ b/check.sh @@ -206,6 +206,19 @@ if [[ "${1:-}" == "deep" ]]; then fi done + # Moved out of the default gate (see DEFAULT_STAGES). Needs a rapidprotoc: the sanitizer build is + # the wrong binary to sweep 8000 schemas with, so build the plain one if this is a bare deep run. + section "real-world schema corpus" + if [[ ! -x ./build/gcc/rapidprotoc ]]; then + cmake --preset gcc >/dev/null 2>&1 + cmake --build --preset gcc --target rapidprotoc -j"$JOBS" >/dev/null 2>&1 + fi + if [[ -x ./build/gcc/rapidprotoc ]]; then + python3 tests/corpus_gate.py --rapidprotoc ./build/gcc/rapidprotoc --jobs "$JOBS" || deep_fail=1 + else + echo ">> could not build build/gcc/rapidprotoc for the corpus sweep"; deep_fail=1 + fi + section "deep summary" if [[ "$deep_fail" == 0 ]]; then echo "DEEP ALL GREEN"; else echo "DEEP FAILURES above"; fi exit "$deep_fail" @@ -439,7 +452,7 @@ job_differential() { echo " (run ./check.sh, or include 'gcc' in RAPIDPROTO_GATE_STAGES)" return 1 fi - python3 tests/differential.py --build-dir ./build/gcc + python3 tests/differential.py --build-dir ./build/gcc --jobs "$JOBS" } # THE stage table. Adding a gate stage means one key here, one title, one job -- and nothing else: @@ -448,6 +461,12 @@ job_differential() { # rc_ aggregation) failed SILENTLY when missed, leaving a stage that could only report success. readonly STAGE_KEYS=(format docs fixtures gcc clang cf fuzz tidy corpus cxx20 differential) +# What a bare ./check.sh runs. `corpus` is deliberately absent: sweeping ~8000 third-party schemas is +# a COMPATIBILITY check, not a fast-feedback one -- the library's own behaviour is covered by the +# explicit tests -- and at ~163s it was 30% of the gate. It moved to the deep tier (which gates every +# PR) and keeps its own CI runner, so nothing stopped watching it; it just left the inner loop. +readonly DEFAULT_STAGES=(format docs fixtures gcc clang cf fuzz tidy cxx20 differential) + stage_title() { case $1 in format) echo "clang-format (check)" ;; @@ -496,7 +515,7 @@ if [[ -n "${RAPIDPROTO_GATE_STAGES:-}" ]]; then fi stage_enabled() { - [[ " ${RAPIDPROTO_GATE_STAGES:-${STAGE_KEYS[*]}} " == *" $1 "* ]] + [[ " ${RAPIDPROTO_GATE_STAGES:-${DEFAULT_STAGES[*]}} " == *" $1 "* ]] } # Records the outcome and duration BESIDE the log, because a concurrent stage runs in a subshell and diff --git a/tests/differential.py b/tests/differential.py index 9c19d8a..67019e7 100644 --- a/tests/differential.py +++ b/tests/differential.py @@ -45,14 +45,17 @@ from __future__ import annotations import argparse +import concurrent.futures import json import math import random import shutil import struct +import os import subprocess import sys import tempfile +from itertools import repeat from pathlib import Path REPO = Path(__file__).resolve().parent.parent @@ -494,6 +497,37 @@ def describe_difference(wanted, got, path: str = "") -> str: return f"{path or ''}: protobuf={wanted!r} rapidproto={got!r}" +def check_schema(schema: Path, tools: dict[str, Path], cxx: str, seed: int, messages: int, + seed_dir: Path | None) -> tuple[int, str | None, list[str]]: + """One schema end to end: (messages checked, skip reason or None, mismatch descriptions). + + Runs in a worker process, so every argument is picklable and nothing is shared but `seed_dir` + (whose files are named after the message FQN, unique across schemas). + """ + from google.protobuf import message_factory # re-imported here: this runs in a worker process + + rng = random.Random(f"{seed}:{schema.name}") # per schema, so one file's set is stable + with tempfile.TemporaryDirectory(prefix="rpdiff-") as directory: + work = Path(directory) + try: + harness, pool, meta = build_schema(schema, work, tools, cxx) + except Skip as reason: + return 0, f"{schema.name}: {reason}", [] + except HarnessError as error: + return 0, None, [f"{schema.name}: {error}"] + factory = message_factory.MessageFactory(pool) + checked = 0 + failures: list[str] = [] + for fqn in meta["messages"]: + descriptor = pool.FindMessageTypeByName(fqn.lstrip(".")) + if descriptor.GetOptions().map_entry: + continue # synthesized map entries are not decoded on their own + checked += 1 + failures += check_message(harness, work, factory, descriptor, meta, messages, rng, + seed_dir) + return checked, None, failures + + def main() -> int: parser = argparse.ArgumentParser( description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) @@ -508,10 +542,12 @@ def main() -> int: parser.add_argument("--write-seeds", type=Path, default=None, metavar="DIR", help="also write every generated payload into DIR, as a fuzzer seed corpus") parser.add_argument("--verbose", action="store_true", help="name every schema and skip reason") + parser.add_argument("--jobs", type=int, default=os.cpu_count() or 4, + help="schemas to check in parallel (default: CPU count)") args = parser.parse_args() try: - from google.protobuf import message_factory + import google.protobuf.message_factory # noqa: F401 (presence check; workers re-import) except ImportError: print("differential: protobuf Python bindings not installed; skipping") return 0 @@ -548,31 +584,23 @@ def main() -> int: for schema in schemas: if not schema.is_file(): raise SystemExit(f"--schema {schema} does not exist") + # One worker per schema. Each compiles a harness and runs it, so the work is dominated by + # subprocesses rather than Python -- but the payload generation and field comparison ARE Python, + # so processes (not threads) to escape the GIL. map() yields in input order, which keeps the + # skip/failure lines identical run to run regardless of who finishes first. checked = 0 skipped: list[str] = [] failures: list[str] = [] - for schema in schemas: - rng = random.Random(f"{args.seed}:{schema.name}") # per schema, so one file's set is stable - with tempfile.TemporaryDirectory(prefix="rpdiff-") as directory: - work = Path(directory) - try: - harness, pool, meta = build_schema(schema, work, tools, args.cxx) - except Skip as reason: - skipped.append(f"{schema.name}: {reason}") - continue - except HarnessError as error: - failures.append(f"{schema.name}: {error}") - continue - factory = message_factory.MessageFactory(pool) - for fqn in meta["messages"]: - descriptor = pool.FindMessageTypeByName(fqn.lstrip(".")) - if descriptor.GetOptions().map_entry: - continue # synthesized map entries are not decoded on their own - checked += 1 - failures += check_message(harness, work, factory, descriptor, meta, - args.messages, rng, seed_dir) - if args.verbose: - print(f" {schema.name}: done") + with concurrent.futures.ProcessPoolExecutor(max_workers=args.jobs) as pool_exec: + results = pool_exec.map(check_schema, schemas, repeat(tools), repeat(args.cxx), + repeat(args.seed), repeat(args.messages), repeat(seed_dir)) + for schema, (count, skip, schema_failures) in zip(schemas, results): + checked += count + if skip is not None: + skipped.append(skip) + failures += schema_failures + if args.verbose: + print(f" {schema.name}: done") # Skips are printed every run, not just under --verbose: they are how a schema silently leaves # coverage, and the count drifting is the thing worth noticing. From 156fb02d8c61c600919e0d5358b9a9bc4414ab7c Mon Sep 17 00:00:00 2001 From: Christian Vetter Date: Wed, 12 Aug 2026 21:17:11 +0200 Subject: [PATCH 04/19] parser: split the enum/reserved grammar into its own TU --- CMakeLists.txt | 1 + check.sh | 4 +- src/parser.cpp | 183 ++++------------------------------------ src/parser_enum.cpp | 146 ++++++++++++++++++++++++++++++++ src/parser_internal.hpp | 89 +++++++++++++++++++ 5 files changed, 256 insertions(+), 167 deletions(-) create mode 100644 src/parser_enum.cpp create mode 100644 src/parser_internal.hpp diff --git a/CMakeLists.txt b/CMakeLists.txt index d0e2c46..9c83a23 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -97,6 +97,7 @@ add_library(rapidproto_lib STATIC src/lexer.cpp src/interpret.cpp src/parser.cpp + src/parser_enum.cpp src/features.cpp src/resolve.cpp src/resolver.cpp diff --git a/check.sh b/check.sh index 1ff37e4..6cdfe35 100755 --- a/check.sh +++ b/check.sh @@ -44,7 +44,7 @@ export CLANG_TIDY # are formatted but NOT tidied -- their argv / measurement / harness patterns trip strict checks for # no real-bug gain. HEADERS=(include/rapidproto/*.hpp include/rapidproto/streamgen/*.hpp include/rapidproto/arenagen/*.hpp include/rapidproto/dumpgen/*.hpp include/rapidproto/codegen/*.hpp include/rapidproto/cli/*.hpp) -LIB_SRC=(src/lexer.cpp src/interpret.cpp src/parser.cpp src/features.cpp src/resolve.cpp src/resolver.cpp src/source.cpp src/streamgen/generator.cpp src/codegen/naming.cpp src/arenagen/layout.cpp src/arenagen/modes.cpp src/arenagen/generator.cpp src/dumpgen/generator.cpp src/header_self_contained.cpp) +LIB_SRC=(src/lexer.cpp src/interpret.cpp src/parser.cpp src/parser_enum.cpp src/features.cpp src/resolve.cpp src/resolver.cpp src/source.cpp src/streamgen/generator.cpp src/codegen/naming.cpp src/arenagen/layout.cpp src/arenagen/modes.cpp src/arenagen/generator.cpp src/dumpgen/generator.cpp src/header_self_contained.cpp) TEST_SRC=(tests/test_*.cpp) CLI_SRC=(src/main.cpp src/rapidprotoc/main.cpp tests/diffgen/main.cpp) EXTRA_SRC=(tests/bench_streamgen.cpp tests/bench_stream_isolated.cpp tests/bench_arena.cpp tests/fuzz/*.cpp examples/*/*.cpp) @@ -177,7 +177,7 @@ if [[ "${1:-}" == "deep" ]]; then section "fuzz smoke (${FUZZ_TIME}s per target)" mkdir -p build/fuzz # The front-end target links the library TUs; the three decode targets are header-only. - parser_tus=(src/lexer.cpp src/parser.cpp src/features.cpp src/resolve.cpp src/interpret.cpp + parser_tus=(src/lexer.cpp src/parser.cpp src/parser_enum.cpp src/features.cpp src/resolve.cpp src/interpret.cpp src/source.cpp src/resolver.cpp src/wellknown_generated.cpp) for f in wire arena stream parser; do extra_tus=() diff --git a/src/parser.cpp b/src/parser.cpp index dda4ecd..340dd60 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -1,4 +1,5 @@ #include "rapidproto/parser.hpp" +#include "parser_internal.hpp" #include #include @@ -34,6 +35,7 @@ // conversion is always memory-safe. namespace rapidproto { +using namespace parse_detail; // NOLINT(google-build-using-namespace): the parser's own split namespace { // --- recursion-depth guard -------------------------------------------------- @@ -78,25 +80,6 @@ Error too_deep(Range in) { // --- token matchers --------------------------------------------------------- -bool is_keyword(TokenKind k) { - return k >= TokenKind::KwSyntax && k <= TokenKind::KwNan; -} - -// A name position accepts an identifier or a keyword (proto allows keywords as -// names — e.g. a field named `message`). -bool is_name_token(const Token& t) { - return t.kind == TokenKind::Identifier || is_keyword(t.kind); -} - -// Match one token of the given kind; produces the Token. -auto kind(TokenKind k) { - return one([k](const Token& t) { return t.kind == k; }); -} - -auto name_token() { - return one([](const Token& t) { return is_name_token(t); }); -} - auto sign_token() { return one( [](const Token& t) { return t.kind == TokenKind::Minus || t.kind == TokenKind::Plus; }); @@ -390,137 +373,6 @@ Result> parse_list_literal(Range in) { // out-of-range magnitude (beyond uint64, or outside int32 once signed) silently yields an unspecified // in-range value rather than erroring: a value-range concern for invalid input only, and // memory-safe. -std::int32_t parse_int32(std::string_view text, bool negative) { - const auto [base, digits] = split_int_literal(text); - std::uint64_t mag = 0; - // NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage): paired with size below - const char* first = digits.data(); - // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic): from_chars needs a range - const char* last = first + digits.size(); - std::from_chars(first, last, mag, base); - // Negate in the unsigned domain, where wraparound is well-defined: negating the int64 cast - // would be signed-overflow UB for a magnitude of exactly 2^63 (e.g. `-9223372036854775808`). - const std::uint64_t value = negative ? std::uint64_t{0} - mag : mag; - return static_cast(value); -} - -template -std::vector prepend(T first, std::vector rest) { - std::vector out; - out.reserve(rest.size() + 1); - out.push_back(std::move(first)); - for (auto& item : rest) { - out.push_back(std::move(item)); - } - return out; -} - -// ["-"] intLit -> int32 -auto signed_int() { - return map(seq(opt(kind(TokenKind::Minus)), kind(TokenKind::IntLiteral)), [](auto parts) { - return parse_int32(std::get<1>(parts).text, std::get<0>(parts).has_value()); - }); -} - -// Range = ["-"] intLit [ "to" ( ["-"] intLit | "max" ) ]; `max` -> the given sentinel. -// Function boundary: reserved_range appears twice inside both parse_reserved and -// extension_range, so its signed_int/kind chain otherwise re-spells into every enclosing name. -Result> reserved_range(Range in, std::int32_t max_sentinel) { - auto bound = - alt(map(kind(TokenKind::KwMax), [max_sentinel](const Token&) { return max_sentinel; }), - signed_int()); - return map(seq(signed_int(), opt(preceded(kind(TokenKind::KwTo), bound))), [](auto parts) { - NumberRange range; - range.start = std::get<0>(parts); - range.end = std::get<1>(parts).has_value() ? *std::get<1>(parts) : range.start; - return range; - })(in); -} -// The lambda-wrapped spelling that combinator call sites consume. -auto reserved_range(std::int32_t max_sentinel) { - return [max_sentinel](Range in) { return reserved_range(in, max_sentinel); }; -} - -// A reserved name is a string literal (proto2/proto3) or an identifier (editions). -auto reserved_name() { - return alt(map(kind(TokenKind::StringLiteral), [](Token t) { return std::move(t.str_value); }), - map(name_token(), [](const Token& t) { return std::string(t.text); })); -} - -// ReservedDecl = "reserved" ( Range {"," Range} | Name {"," Name} ) ";" -Result> parse_reserved(Range in, std::int32_t max_sentinel) { - auto ranges = map(seq(reserved_range(max_sentinel), - many(preceded(kind(TokenKind::Comma), reserved_range(max_sentinel)))), - [](auto parts) { - ReservedNode node; - node.ranges = - prepend(std::move(std::get<0>(parts)), std::move(std::get<1>(parts))); - return node; - }); - auto names = map(seq(reserved_name(), many(preceded(kind(TokenKind::Comma), reserved_name()))), - [](auto parts) { - ReservedNode node; - node.names = - prepend(std::move(std::get<0>(parts)), std::move(std::get<1>(parts))); - return node; - }); - return delimited(kind(TokenKind::KwReserved), alt(ranges, names), - cut(kind(TokenKind::Semicolon)))(in); -} - -// EnumValueDecl = ident "=" ["-"] intLit [ CompactOptions ] ";" -auto enum_value_decl() { - return map(seq(name_token(), cut(kind(TokenKind::Equals)), cut(signed_int()), - opt(parse_compact_options), cut(kind(TokenKind::Semicolon))), - [](auto parts) { - EnumValueNode value; - value.name = std::string(std::get<0>(parts).text); - value.number = std::get<2>(parts); - if (std::get<3>(parts).has_value()) { - value.options = std::move(*std::get<3>(parts)); - } - return value; - }); -} - -// [ "export" | "local" ] -> optional -auto visibility_modifier() { - return opt(alt(map(kind(TokenKind::KwExport), [](const Token&) { return Visibility::Export; }), - map(kind(TokenKind::KwLocal), [](const Token&) { return Visibility::Local; }))); -} - -// One enum body element. `option`/`reserved` are matched before a bare value so those -// keywords aren't mistaken for value names. monostate represents an empty ";". -using EnumElement = std::variant; - -auto enum_body() { - return many( - alt(map(parse_option_decl, [](Option o) { return EnumElement{std::move(o)}; }), - map([](Range i) { return parse_reserved(i, kMaxEnumNumber); }, - [](ReservedNode r) { return EnumElement{std::move(r)}; }), - map(enum_value_decl(), [](EnumValueNode v) { return EnumElement{std::move(v)}; }), - map(kind(TokenKind::Semicolon), - [](const Token&) { return EnumElement{std::monostate{}}; }))); -} - -EnumNode assemble_enum(std::string_view name, std::vector& elements, - SyntaxLevel syntax) { - EnumNode node; - node.name = std::string(name); - // proto2 enums are closed; proto3 and editions default open (editions refined by - // the feature pass). - node.openness = syntax == SyntaxLevel::Proto2 ? EnumOpenness::Closed : EnumOpenness::Open; - for (auto& element : elements) { - if (auto* value = std::get_if(&element)) { - node.values.push_back(std::move(*value)); - } else if (auto* option = std::get_if