diff --git a/derive/src/codegen.rs b/derive/src/codegen.rs index 8118861ae..889a4ca65 100644 --- a/derive/src/codegen.rs +++ b/derive/src/codegen.rs @@ -1304,10 +1304,6 @@ fn flag_arm(cli: &Cli, i: usize, field: &Field) -> TokenStream { } } -/// Whether another occurrence is a command-line mistake rather than another value. -/// -/// Counts and collections repeat by definition, and `var` explicitly opts a value-taking flag -/// into repetition. Every other flag matches clap's default of one occurrence. /// Whether a repeat of this flag is only a duplicate *within one command*. /// /// A `global` flag is in scope for every descendant, and clap lets it be given again on a @@ -1331,6 +1327,26 @@ fn reset_per_level(cli: &Cli) -> TokenStream { quote!(#(#resets)*) } +/// Whether another occurrence is a command-line mistake rather than another value. +/// +/// Counts and collections repeat by definition, and `var` explicitly opts a value-taking flag +/// into repetition. Every other flag matches clap's default of one occurrence. +/// +/// **This is stricter than the grammar, deliberately.** The corpus specifies the opposite for a +/// *spec-driven* parse — `long-repeated-keeps-the-last` says "a repeat is a correction, typically +/// a wrapper appending to a command line it did not write" — and usage-argv's parser and +/// usage-lib both honour that. The rule lives here, in the derive's post-binding layer, and +/// nowhere else: `Error::DuplicateFlag` is constructed at one call site, in this file. +/// +/// Kept, because the two cases are not the same case. A spec parsed at run time may well be +/// describing someone else's command line, which is the wrapper the corpus has in mind. A +/// derived CLI is an authored, fixed surface, where `--jobs 1 --jobs 2` is far likelier to be a +/// mistake than an amendment — and it is what clap does, so an adopter replacing clap sees no +/// change. Dropping it would make a derived binary quietly accept a line clap rejects today, +/// which is the direction that costs an adopter rather than the one that helps. +/// +/// Recorded because it looks like an inconsistency and is not one. `differential.rs` in the gate +/// carries the same note from the other end. fn rejects_duplicate(field: &Field) -> bool { matches!(field.kind, Kind::Flag { .. }) && !matches!(field.shape, Shape::Count | Shape::Many) diff --git a/tasks/perf-shadow.sh b/tasks/perf-shadow.sh index 57c0cdff7..e891bbb35 100755 --- a/tasks/perf-shadow.sh +++ b/tasks/perf-shadow.sh @@ -66,6 +66,19 @@ verify() { fi } +# The ratio against clap this file expects to hold, and below which it says so. +# +# Not a gate — the workflow runs this with `|| true`, and a shared runner is no place to fail a +# build on an instruction count. It is an annotation, because the alternative turned out to be +# nobody noticing: the number went from 117x to 97x across a run of feature work, unremarked, +# because the only place it appeared was one line of a report nobody diffs. clap is flat, so the +# movement was ours. +# +# 80 rather than 117: the point is to catch a slide, not to pin the current figure. A parse that +# is only 80x cheaper than clap's has stopped being the thing this project claims, and that is +# worth a sentence in the log before it is worth an argument about the target. +CLAP_RATIO_FLOOR=80 + # Two significant figures either side of the decimal point, so a framework within a factor # of usage reads as 1.5x rather than 1x and one three orders away still reads cleanly. ratio() { @@ -119,3 +132,20 @@ bpaf_cold=$(cold parse-n-bpaf) } >"$tmp" cat "$tmp" >"$out" + +# Said after the report is written, so a failure here cannot leave the comment truncated — the +# same reason the table is built in a temporary file. +# +# `::warning::` when running under GitHub Actions, a plain line otherwise, so the same script is +# useful at a terminal. +clap_ratio=$(awk -v a="$clap_cold" -v b="$usage_cold" 'BEGIN { printf "%d", a / b }') +if [ "$clap_ratio" -lt "$CLAP_RATIO_FLOOR" ]; then + message="the parse is ${clap_ratio}x cheaper than clap's, under the ${CLAP_RATIO_FLOOR}x this\ + file expects. clap is flat, so a fall here is usage getting more expensive rather than clap\ + getting cheaper. See tasks/perf-shadow.sh." + if [ -n "${GITHUB_ACTIONS:-}" ]; then + echo "::warning title=Shadow ratio below floor::$message" + else + echo "warning: $message" >&2 + fi +fi