Skip to content

feat(const-param): experimental Clang check of const suggestions, version 2 - #239

Draft
SizzleUnrlsd wants to merge 164 commits into
mainfrom
217-t-v2
Draft

SizzleUnrlsd wants to merge 164 commits into
mainfrom
217-t-v2

Conversation

@SizzleUnrlsd

@SizzleUnrlsd SizzleUnrlsd commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Part of #217. Fixes #237, through #238's d711321, carried here.

The second version of the experimental Clang check of ConstParameterNotModified suggestions (--experimental-const-suggestion-check). #221 stays open as a draft and is not to be merged: it is the reference of the first version (217-t-v1 at 2a5b0c1).

This branch carries d711321 from #238 (#237), merged in 46c036f, and 6a91a79 and 270955e from #241 (#240), merged in 4fd0dae: its unit tests pass on Linux arm64, and its messages name C++23 explicit object members on macOS 14 too. #238 is closed as superseded by this PR; #237 stays open until this PR is delivered.

Depends on #241. The fallback to LLVM's demangler (#240) changes the output without --experimental-const-suggestion-check: symbols that the C++ runtime's demangler predates are now demangled. That change belongs to #241 and is reviewed and merged there on its own; it is not an exception that #239 makes to its rule that the output without the option stays the same. #239 carries 6a91a79 and 270955e only because const-suggestion-explicit-object.cpp needs them on macos-14. #258, now in main, replaces that job with macos-15: this PR's CI, which runs on its merge ref, now builds on macos-15, where the suite passes, with the fallback present. The order is #258, then #241, then #239: #258 is in main, and #241 has been updated with main (3027b09). #239 is not merged before #241; once #241 is in main, #239 is updated with main, and its diff no longer contains the change.

Status

Draft, not to be merged. Stages 1 to 3, the final phase, the fixes of the final review and those that followed its targeted review are done; 2318ea2 merges main at f71bdac (#246) and is validated. 8658cd0 adds the fixes of the two defects that were open at 2318ea2 (below), 8f875a7 formats them, and c84b959 adds a test of a defining file that imports a module. c84b959 is validated (below). 70762e7 merges main at b1228c6, which brings #263: its analyzer code differs from c84b959's by #263's diff alone, and its CI passes (below). 9304b8f and af12b11 share the compilations after a rewrite between the suggestions that have the same one; their CI passes. e1ce489 and 57aa98a limit that sharing to the defining file, which removes af12b11's memory retention on the scenario that showed it (Shared compilations, below); their CI passes. One criterion is not met: the cost. Since November 4 it is judged on Release builds, the same binary without and with the option: the option adds +70.3 % at c84b959, +62.1 % at 70762e7, +59.2 % at af12b11 and +57.9 % at 57aa98a, the head (+53.3 % or +62.7 % depending on the pass without the option). The reserves keep the first two from showing a lower cost of the option, and af12b11 and 57aa98a are not shown to differ: on the campaign's load they make the same compilations. Against 70762e7, each in its own campaign, af12b11 takes 193.2 s instead of 202.7 s with the option, and 57aa98a 193.5 s instead of 206.6 s (Cost in Release, below). The plan asks for at most 10 %; this overrun is not accepted. The earlier campaigns, up to 2318ea2 (+30.1 %, below, with their measurement reserves), measured builds without optimization.

Two defects that were open at 2318ea2 are fixed at 8658cd0. In both, a suggestion was reported checked while its rewrite breaks the program or adds a constraint violation. Each fix follows its test, which fails at 2318ea2.

  1. A callback passed through ?: (test 5dcbae0, fix af1edbe).
    • callout_enumerate_function_8's cb and callout_data, in PCRE2's pcre2test, were reported checked. The function reaches pcre2_callout_enumerate through a conditional, c ? f1 : f2.
    • With the rewrite, the two operands point to incompatible function types. C makes that a constraint violation (C11 6.5.15p3); Clang reports it only as -Wpointer-type-mismatch, a warning that the check did not track. Passed directly, the same rewrite is an error, and such suggestions were already removed.
    • That group is now tracked like the others: enabled, and kept a warning under -Werror.
    • check_const_suggestion_conditional_between_callbacks covers the default warnings and -Werror -Wno-pointer-type-mismatch, with a conditional between const T * and T *, which C allows, as its witness. In PCRE2, both suggestions are now removed for that warning (Corpora at 8658cd0 and c84b959, below).
  2. A file that imports a module (test 10adf29, fix 8658cd0).
    • The check tells whether a file of the compilation database sees the function from that file's preprocessing, which does not read a module that the file imports.
    • A file that got the function's declaration from a Clang header module (-fmodules), and never named the function in its own text, was not judged. In a minimal C++ case on macOS, the suggestion was reported checked, while the rewritten program no longer links (D17 missed).
    • The scan now records an import: an #include that becomes one, a C++20 header unit, an import declaration, or a module implementation unit, which imports its interface.
    • A file that imports a module, and does not see the change otherwise, keeps the suggestion not checked. The exception: the rewrite changes only the defining file, and the function is not a free function with external linkage. A refutation by another file still wins.
    • check_const_suggestion_module_import covers, for a function f declared in a header, each of the four forms, a precompiled header as a witness (judged, D17, as before), two refutations, and a static function s, which stays checked in every case. The exception's two conditions are not tested separately: in every case, f has external linkage and is declared in a header that the rewrite changes, and s has neither.

Limits on modules, kept:

  • The check does not rebuild modules with the rewritten files. A file that names the function and imports a Clang header module that declares it is judged: in two cases run on macOS arm64, outside the suite, at 2318ea2 and again at 8658cd0, the suggestion is not checked.
  • A file that gets the declaration through a precompiled header (-include-pch) is judged, as with a plain include, when the header that the PCH was built from is still there, unchanged.
  • Under -fmodules, an #include of a header that a module map names, system headers included, counts as an import. In such a database, a suggestion is not checked as soon as one file imports a module and does not see the change otherwise, unless the rewrite changes only the defining file and the function is not a free function with external linkage; a refutation by another file still wins. This rule applies to every free function with external linkage, and to every function whose rewrite changes a header, such as a method declared in its class's header. For example, with a file that imports the module holding the rewritten header and does not use the function, the suggestion was checked at 2318ea2 and is not checked at 8658cd0.
  • A defining file that imports a Clang header module: one targeted test, check_const_suggestion_defining_file_imports_a_module (c84b959), covers one case. The module's header uses the function in a way that the rewrite breaks. Because the module keeps the old declaration, the suggestion is not checked; without -fmodules, the same files refute the rewrite. No code changes with this test: it pins that verdict and its reason.
  • Not covered: a defining file that is a C++20 module unit, or that imports a named module or a header unit; a module whose declaration of the function the defining file does not see; a module built before the analysis from other sources than those on disk.

Shared compilations (9304b8f, af12b11; limited to the defining file by e1ce489 and 57aa98a). Suggestions with the same rewrite, such as the specializations of one template, compiled each file after it once per suggestion, with the same inputs: the same command, directory and texts. One compilation of the defining file now serves them all: CodeGen, then one inspection for each suggestion, with its own targets, plan and sites, as its own compilation would give them; each suggestion is still judged alone and in the same order, with the same trace. The later suggestions keep from it their facts and the code prepared once, not the module. The other files are compiled and released for each suggestion, as before.

  • Before the change, on the self-analysis that the cost is measured on, with a throwaway patch that only logs each compilation's inputs: 7 of the 26 compilations after a rewrite repeat another one of their module, all of a defining file (2 in StackComputation.cpp, 5 in AnalyzerApp.cpp), none across modules; that run's trace is the campaign's.
  • Tests. check_const_suggestion_compiles_a_shared_rewrite_once (9304b8f) and check_const_suggestion_shares_only_the_defining_file (9304b8f, adapted and renamed in e1ce489): a template whose first<int> has its address cast explicitly (D15), so that first<int> stays not checked where that happens and first<double> is checked. The first expects 1 compilation after the rewrite and fails at 70762e7 with 2, on macOS and Linux x86_64. The second, with two files, expects 3: the defining file once, the other file once for each suggestion. It fails on that count only: with 4 at 70762e7 on macOS, and with 2 at af12b11 on macOS and Linux x86_64 (e1ce489's suite, 2741/2742). testConstSuggestionSharedInspection (af12b11) compares, for each target, everything the judgement reads from a shared compilation with its own compilation: facts, declarations, sites, diagnostics and code, with an address cast and with a rewrite that does not compile. Mutants that give the first target's facts to every inspection, the diagnostics to the first inspection only, or the first suggestion's facts to the later ones are caught. At 57aa98a, so are mutants that give a later suggestion the first one's facts, or do not share the prepared code.
  • Validation on Linux x86_64 (Release, in the campaign's container): the suite passes 2740/2740 at 70762e7, 2740/2742 at 9304b8f (the two new checks only), 2742/2742 at af12b11. Against 70762e7, without and with the option, the output is the same on the self-analysis, zlib and tinyxml2 (SARIF byte for byte); on PCRE2 it is the same apart from the order and the ids of the diagnostics, as between two runs of 70762e7 (JSON and SARIF outputs list the same diagnostics in a different order from one run to the next #220).
  • Validation of 57aa98a on Linux x86_64 (the same container): the unit and ownership tests pass, and the suite passes 2742/2742. With the cost campaign's protocol (its tree, 4 chunks, 2 analyzers at once), the self-analysis gives each chunk the same JSON as 70762e7, without and with the option, the same trace and the same verdicts (10 suggestions shown with the option, 22 without, 22 judged). zlib and tinyxml2, without and with the option, and PCRE2 with it, are the same byte for byte; PCRE2's order and ids can differ between two runs (JSON and SARIF outputs list the same diagnostics in a different order from one run to the next #220), not this time. On that self-analysis, it compiles 19 times after a rewrite against 26 at 70762e7, chunk by chunk as af12b11 in the campaign, with one inspection per suggestion (38).
  • Memory (57aa98a). af12b11 kept a compilation made for several suggestions, module included, for each later one until it reached that file. When such suggestions rewrite a header that several files see, the modules of all those files stayed alive until the next suggestion, where they used to go file by file. The cost campaign does not exercise it: its shared compilations are all of a defining file. The scenario: a header shared by two suggestions and included by 8 other files that instantiate std::regex, where af12b11 holds 9 modules at once. 57aa98a compiles 17 times after the rewrite (18 at 70762e7, 9 at af12b11); while the other files run, it holds one entry of about 2 KB of prepared text and no module. Alternated 3 times in one run, the largest process reads 375 to 377 MiB, against 375 to 376 MiB at 70762e7 and 537 to 538 MiB at af12b11, on Linux x86_64 (Release, in the campaign's container); 345 to 348 MiB, against 344 to 353 and 431 to 452 MiB, on macOS. The 9 runs give the same JSON and the same trace. The memory figures of the campaign below hold for its load only.
    • Not validated: 4 analyzers at once (ANALYZER_JOBS=4, each analyzer taking as many workers as the host has CPUs, 32 on 4 CPUs). In an earlier validation of this point with 4, the 10 GiB container ran out of memory on the self-analysis, for 70762e7 and an earlier fix alike; the cause is not established. The validations of 57aa98a ran 2 at once, as the cost campaign does, and stop at the first OOM kill.

Cost in Release (self-analysis of c84b959's sources with their compile database, in 4 chunks on 2 jobs, cold caches, in a 4-CPU, 10 GiB container on one Linux x86_64 machine; Release builds, -O3 -DNDEBUG, GCC 13.3, LLVM 20.1.8; per binary, the mean of its 4 passes with the option over the mean of its 2 without):

  • Reference, c84b959 (November 4): 175.1 s without the option, 298.2 s with it: +123.1 s, +70.3 %; CPU +39.6 %.
  • 70762e7 (November 7), in one campaign with c84b959's binary as a control, the two binaries' passes interleaved in a balanced order:
    1. perf(resource): demangle each callee once instead of once per rule #263's gain, confirmed. Against the control, the analysis takes 206.0 s instead of 302.5 s with the option (-96.5 s, -31.9 %), and 127.1 s instead of 184.7 s without it (-31.2 %); CPU -25.6 % with the option, -37.6 % without. The verdicts do not change: on the self-analysis and the corpora, they are the control's and the reference's.
    2. The option's own cost, with no visible improvement. Its --timing counters sum to 156.8 s over the four chunks, against 153.0 s for the control. Recompiling (compile.*, their inspection included) is about 77 % of it. The CPU it adds: +153.0 s, against +141.6 s.
    3. The threshold, still exceeded. 127.1 s without the option, 206.0 s with it: +79.0 s, +62.1 %, for a threshold of 139.8 s, about 66 s short. Reserve: the two passes without the option read 135.2 and 118.9 s; against one or the other, the overhead is +52.3 % or +73.3 % (the control's: +56.8 % or +71.5 %). Both binaries' first passes without the option, the first two of the campaign, ran 9 to 14 % slower than their last ones, in CPU time too and on every chunk, for a cause that was not measured; the passes with the option stayed within 2 % of their mean. The move from +70.3 % to +62.1 % is therefore not a demonstrated improvement of the option's cost. The threshold is missed under every reading.
  • af12b11 (November 8), in one campaign with 70762e7's binary as a control, interleaved the same way, on the same inputs:
    • With the option, 202.7 s for the control and 193.2 s for af12b11: -9.6 s (-4.7 %). The four interleaved pairs read -7.8, -8.6, -12.7 and -9.3 s, each more than the spread of either binary's own passes with the option (2.1 and 2.9 s); the gain is in the fourth chunk, where the option adds 48.9 s instead of 59.9 s. The option's --timing counters sum to 138.2 s instead of 152.8 s. CPU with the option -2.6 %.
    • The verdicts do not change: on the self-analysis (10 suggestions shown with the option, 22 without, 22 judged) and on the corpora, they are the control's and those of the campaign of 70762e7.
    • The threshold, still exceeded. 121.3 s without the option, 193.2 s with it: +71.8 s, +59.2 % (the control: +69.9 %), for a threshold of 133.5 s, about 60 s short. Reserve: without the option, af12b11 read 1.1 and 2.9 s slower than the control in the two pairs, although it runs the same code there and its output is byte for byte the same; two pairs cannot tell that gap from noise. Against the control's passes without the option, af12b11's overhead would read +61.8 %.
    • The clock ran about 4 % fast, at one rate over the whole campaign: uncorrected, +59.19 %; corrected, +59.2 %.
    • Memory. The container's peak with the option is 7.18 GiB, against 7.24 GiB for the control, on this load only (Shared compilations, above).
  • 57aa98a (November 11), the head, in one campaign with 70762e7's binary as a control, interleaved the same way, on the same inputs. The campaign stops at the first chunk that fails and at the first OOM kill; neither happened, and all 64 chunks exit 0.
    • With the option, 206.6 s for the control and 193.5 s for 57aa98a: -13.1 s (-6.4 %). The four interleaved pairs read -14.2, -13.0, -11.7 and -13.6 s, each more than the spread of either binary's own passes with the option (3.4 and 2.7 s); the gain is in the fourth chunk, where the option adds 47.6 s instead of 61.0 s. The option's --timing counters sum to 138.7 s instead of 154.5 s, with 19 compilations after a rewrite instead of 26, as at af12b11. CPU with the option: 387.5 s instead of 403.0 s (-3.8 %). Without the option, the two pairs read -1.1 and +0.2 s.
    • The verdicts do not change: on the self-analysis (10 suggestions shown with the option, 22 without, 22 judged: 10 verified, 12 removed), they are the control's and those of the campaign of af12b11. The corpora are not run again: 57aa98a's were validated against 70762e7 with the same code (Shared compilations, above).
    • The threshold, still exceeded. 122.5 s without the option, 193.5 s with it: +70.9 s, +57.9 % (the control: +68.0 %), for a threshold of 134.8 s, about 59 s short. Reserve: the two passes without the option read 126.2 and 118.9 s (the campaign's first two passes, one for each binary, read about 8 s slower than its last two); against one or the other, the overhead is +53.3 % or +62.7 % (the control's: +62.4 % or +74.1 %). Against af12b11's +59.2 %, from another campaign, this is no demonstrated difference: both make the same compilations on this load, and the control read 1.9 % slower with the option and 3.0 % slower without it than in that campaign.
    • The clock ran at the outside reference's rate on this boot (1.00002), with no step: uncorrected, +57.89 %; corrected, +57.9 %.
    • Memory, on this load only. The container's peak with the option is 7.71 GiB, against 7.51 GiB for the control; without it, 5.99 and 6.02 GiB. Pass by pass with the option, 7.36 to 7.71 GiB against 7.28 to 7.51 GiB, and the four pairs differ by -0.04 to +0.44 GiB of anonymous memory, in both directions. Two analyzers run at once and no per-process peak is recorded, so the difference is not attributed.
  • Reproducibility. The control reproduces the reference within about 2 % on its stable passes (with the option, +1.4 %), but its first pass without the option reads 10.2 % slower, hence +63.8 % instead of +70.3 %. perf(resource): demangle each callee once instead of once per rule #263's effect is read against the control, in the same campaign, not against the reference.
  • The clock. The machine's monotonic clock ran about 4 to 5 % fast, at a rate that varied during the campaign. Each pass is timed on an NTP-synchronized clock outside the machine, interpolated between readings 30 s apart. A varying drift does not cancel in the ratios: uncorrected, the overhead reads +62.4 % (control +64.0 %); corrected, +62.1 % (+63.8 %). The conclusion does not change.
  • Memory. The container's peak with the option is 7.42 GiB, against 7.09 GiB for the control; without it, 6.01 GiB for both. Two analyzers run at once and no per-process peak is recorded, so the difference is not attributed to perf(resource): demangle each callee once instead of once per rule #263.

Measurement reserves (the campaign at 2318ea2, on one Linux x86_64 machine):

  • The clock. The machine's monotonic clock ran about 8 % fast, and its wall clock, which times the passes, was stepped back about 2.5 s every 32 s, very likely by systemd-timesyncd; the self passes' samples, 5 s apart, show these steps throughout each pass. If each step brought the wall clock back to real time, which measurements taken after the passes support without proving, a pass reads within 2.5 s of its real duration, and the +30.1 % lies between +29.3 and +30.8 %. The --timing figures read about 8 % high; the corpora's short passes are read in CPU time, which is real.
  • The load. The second passes of both binaries with the option ran under load from outside the container: a load average of 12.8 during 42fa604's and 4.3 during 2318ea2's, against 2.0 to 2.9 on average during the other passes, both passes without the option among them. Leaving out 2318ea2's second pass, the option adds +28.9 % against the mean of the two passes without it; first pass against first pass, +27.5 %.
  • One campaign. 42fa604 and 2318ea2 differ by 0.1 % (1041.9 and 1043.2 s), in one first pass each, within the passes' noise (2.3 % between the two passes without the option): this does not show that their costs are equal. The earlier campaigns analyzed their own head's sources and are not compared with this one.
  • Under each of these readings, the +10 % criterion is not met.

Reserves:

What changes from the first version

The first version removed a suggestion whenever the rewritten code differed, whatever the difference. This version gives each suggestion one of three verdicts, from what Clang compiles before and after the rewrite:

  • verified: kept, with the checked ... line;
  • refuted: removed. The rewrite adds an error or a tracked warning, or changes what a call or a reference targets, what a method overrides, an if constexpr condition or a _Generic selection;
  • indeterminate: kept, with not checked with Clang: <reason>. Examples: a file that cannot be checked, a rewrite that changes more than it plans, facts that cannot be compared one to one, symbols that cannot be paired one to one, a dependency that the rewrite renames and the file only declares, a difference of the generated code that the facts do not explain, a rewritten file that does not compile without an error that the rewrite adds, a file that declares the function itself or casts its address, a file that imports a module and does not see the change otherwise, a possible collision between files.

How it runs:

  1. The defining file's compilation plans each suggestion: the function, found by its symbol, the declarations to rewrite, and the type tried.
  2. Each file that sees the change is compiled as it is and with the rewrite.
  3. The symbols that the rewrite renames are paired from compilation facts only: variants, inheriting constructors, nested symbols, instantiation contexts, and (step 5) the entities nested in paired functions: members of their lambdas and local classes, specializations whose arguments or type hold them, implicit members of the classes that hold them. The comparator takes these pairs; its name heuristics are gone.
  4. Collisions are judged once every file is, and only a refutation stops the judgement.

With --verbose, the trace gives each verdict's grounds. The README section "Experimental: const parameter suggestions checked with Clang" describes the check and its limits. Of the limits on modules (Status, above), it states the rule for a file that imports a module and does not see the change; the others are stated only here. Without the option, nothing changes.

Stage 1: the first version's defects

First version This version Test
D1 vtt suggested, x (C2) removed no vtt suggestion (#233), x verified const-suggestion-virtual-base.cpp
D2 a member operator used in another file: removed verified check_const_suggestion_operator_used_elsewhere
D3 a C static function and an external homonym: removed verified, both ways check_const_suggestion_homonyms_across_files
D4 an overload defined in another file: checked indeterminate, the link assumption stated check_const_suggestion_overload_defined_elsewhere
D5 a macro argument that writes two parameters: removed indeterminate const-suggestion-macro-shared-type.c
D7 an inheriting constructor: removed verified const-suggestion-inheriting-constructor.cpp
D11 \ and spaces in a #define: the macro body rewritten, removed indeterminate check_const_suggestion_line_endings
D14 a correspondence that is not one to one: removed indeterminate const-suggestion-deduction-refused.cpp, expectation changed
D15 an explicit cast of the address: removed indeterminate, unless an error or a fact refutes check_const_suggestion_cross_tu, check_const_suggestion_compilation_database, expectations changed; check_const_suggestion_cast_in_the_defining_file
D16 sizeof(__PRETTY_FUNCTION__): removed indeterminate const-suggestion-printed-signature.c, expectation changed
D17 a redeclaration with the old type in another file: removed indeterminate, the link assumption stated the two checks of D15, expectations changed

Other tests added cover:

  • collisions: a merged declaration, a strong and a weak definition, a function that a shared header defines;
  • the units: one configuration per file, one compilation per file;
  • the scope and the inspection: the scope after the rewrite, a fatal error, an invalid declaration, two functions of one macro invocation;
  • the facts: facts without a change of code, repeated and unpaired facts;
  • the trace.

The comparator's unit tests take the established pairs. Each test failed first, on the first version, on an earlier state of this branch, or on a mutant of the code.

Stage 2: the remaining defects of the catalogue

Defect Before Now
D6: a compile command with -Werror that disables a tracked warning group not checked: the file does not compile as it is the tracked groups keep the level Clang gives them by default, warnings under -Werror and default errors as errors
Review Focus 5: -Werror=<group> on a tracked group removed for an error removed for the warning
D8: a C function that a C++ file declares through an extern "C" header not checked each declaration is compared with the candidate as its own unit's language writes it
D18: declarations that differ from the definition in a top-level qualifier, both directions not checked each declaration is compared with the candidate of its own type, qualifiers kept
D12: an rvalue reference the message offers passing by value, which is not checked consider passing by const reference (...); the "checked" line says passing by value is not checked
D13: int *const p, only read displays const int *p, checks const int *const p displays and checks const int *const p
D10: a qualifier written apart from a pointer typedef (const IntPtr p) rewritten const const int *, not checked rewritten const int *const p, verified; a qualifier written by a macro keeps the suggestion as not checked

The type displayed comes from the plan's candidate only, written as Clang writes it, with struct, union and enum where C needs them; a type without a name stays <anonymous type>. A unit test checks that a rewrite losing the pointer's own const is caught, and fails with a comparison that ignores top-level qualifiers. The corresponding limits are removed from the README.

Each test was added after its failure was shown. Over every const-suggestion fixture and the first version's review cases, with and without the option, only the planned lines change; no suggestion disappears, and the output without the option is unchanged.

Stage 3 and the final phase

Stage 3, cost. Measured first (Task 3.1): the 38 compilations of T about 270 s summed over the chunks (front-end about 166 s, inspection 87 s, CodeGen 17 s), the correspondences 80 to 90 s, the scans 31 s; the copies of the scans 2.7 s and the preparation of the commands under 1 s. 48f3ec1 builds the correspondences' index once per call (correspond 49.5 -> 23.0 s, same pairs). The other leads were measured or prototyped and not kept: a precompiled preamble (a net loss, and a crash), sharing the pipeline's compilation (not recommended); the shared scans and a single construction of the Clang invocation stay as they are, their measured cost being below the noise.

Corpora (this repository's sources, zlib, tinyxml2, PCRE2; Linux x86_64): no confirmed valid suggestion disappears against the output without the option. The comparison with the first version found two classes of difference, fixed here:

  • D19 (7010ff6): a declaration whose type is deduced from a call that the rewrite makes invalid (publishSingleStoreSlots). Step 3 now judges a type's written form, its deduced part being left to the later steps, and the types that the edits write outside the planned parameters, default arguments included. The suggestion is refuted for its error, as in the first version.
  • D20 (7511f50): six suggestions of this repository (parallelFor x2, build x2, isRecursiveCallee x2) were not checked, because the specializations that lambdas and local classes nested in a paired function parameterize had no pair. Step 5 of the correspondences identifies them from Clang's facts, read before the AST goes: the members of those nested types, the specializations whose arguments or type hold them, and the implicit members of the classes that hold them (libstdc++'s _Vector_impl and _Deque_impl destructors among them). In a key only the nested types go through the pairs; every other part must match; a key held by more than one symbol on either side gives no pair, and the ABI variants stay apart. The six are now verified, on Linux and macOS.

Against the product before these two fixes, at bed265d: only those six change (not checked -> checked); the other 92 suggestions keep their verdict and reason (src/ 4/6/12 -> 10/0/12 verified, not checked, removed; zlib 16/0/10, tinyxml2 0/0/6, PCRE2 10/31/3 unchanged; PCRE2's 31 are declarations written by a macro). At 42fa604 against bed265d, the 98 suggestions keep their verdict and reason (final review, below).

The 19 units of #215. The comparisons up to 42fa604 left out the units on which the analysis did not finish, as the first version's measurements did: 5 of zlib's 37 (crc32.c, deflate.c, infback.c, inffast.c, inflate.c) and 14 of PCRE2's 35 (pcre2_auto_possess.c, pcre2_compile.c, pcre2_compile_class.c, pcre2_convert.c, pcre2_dfa_match.c, pcre2_find_bracket.c, pcre2_match.c, pcre2_pattern_info.c, pcre2_script_run.c, pcre2_study.c, pcre2_valid_utf.c, pcre2_xclass.c, pcre2grep.c, pcre2test.c). Analyzed alone without the option, before #246, each ran past a 120 s limit. At 2318ea2 (Linux x86_64), each was analyzed alone again under the same limit: without the option, 18 end in at most 7.3 s, and pcre2_match.c again runs past 120 s (with a limit of 1800 s, it ends in 254 s); with the option, the 18 end in at most 10.6 s.

Corpora at 2318ea2, with every unit (Linux x86_64; one binary, without and with the option): each source file of the compilation databases is analyzed once, zlib's 18, tinyxml2's 2 and PCRE2's 34, the generated pcre2_chartables.c left out as before (the databases list 37 and 35 entries). Every run exits 0, and two verbose passes give the same verdicts. The 76 suggestions of the earlier comparisons keep their verdict and reason. The 54 new ones all come from the 19 units:

  • 26 verified: functions of one file (static in PCRE2, local in zlib) that no other unit declares. Two of them were false "checked": callout_enumerate_function_8's cb and callout_data (defect 1, above), removed at 8658cd0 (below);
  • 25 removed. 15 are parameters of callbacks: each callback is passed where a function pointer whose parameter has no const is expected, and with the rewrite its type no longer converts, an error (pcre2test's callouts, my_malloc, my_free, jit_callback, stack_guard, pcre2grep's callout). The other 10 are parameters whose value, or a pointer reached through it (P->ovector), is returned as, stored in, or used to initialize a pointer without const, a dropped qualifier;
  • 3 not checked: pcre2_compile_8, pcre2_dfa_match_8 and pcre2_match_8, whose declarations a macro definition of pcre2.h writes, as for PCRE2's 31 earlier ones.

Corpora at 8658cd0 and c84b959 (the CI does not run the corpora). In each run, 2318ea2 and the head were each built and run once over every unit, with the option and --verbose, in the chunks and with the flags of the campaign: 8658cd0 on Linux arm64 (Docker, Ubuntu 24.04, LLVM 20.1.8), and c84b959 on Linux x86_64, on the campaign's machine (Docker, Ubuntu 24.04, LLVM 20.1.8).

  • On both platforms, 2318ea2 gives the 130 verdicts it gave in the campaign, reasons included.
  • Against it, 126 are unchanged at the head, on both. The 4 that change, the same on both, are all in pcre2test:
    • callout_enumerate_function_8's cb and callout_data: from checked to removed, "gains a checked warning: pcre2test_inc.h:1800: pointer type mismatch".
    • callout_enumerate_function_void_8's cb and callout_data, the conditional's other operand: still removed, but now for that warning. They were removed for an error at pcre2test_inc.h:6969, where the function is passed directly. The check cites the first error or tracked warning that the rewrite adds, and Clang reports the warning first.
  • PCRE2 now shows 65 suggestions with the option, against 67: verified, removed and not checked are 31, 30 and 34, against 33, 28 and 34.
  • zlib (19 verified, 10 removed, 0 not checked) and tinyxml2 (0 verified, 6 removed, 0 not checked) are unchanged.
  • c84b959 on Linux x86_64 gives the 130 verdicts of 8658cd0 on Linux arm64, reasons included. The analyzer's code at c84b959 differs from 8658cd0's by whitespace only (on macOS, the binary is the same file).

Tests. For D19: check_const_suggestion_deduced_type_error, check_const_suggestion_written_part_outside_the_plan, check_const_suggestion_default_argument_outside_the_plan. For D20: six end-to-end checks (check_const_suggestion_p2_*; five failed at 5167998), and the unit tests testConstSuggestionNestedCorrespondences, testConstSuggestionNestedIdentities, testConstSuggestionImplicitMemberIdentities. Each was proven against mutants of the code: 12 of 13 are caught. The one that survives drops the kind and the name of an implicit member: on every real case found, its variant or its type already separates it, so they are protected only together with them, as are the name, static flag and qualifiers of a nested type's member (spec §5.7).

Cost (§8, self-analysis, Linux x86_64, 4 CPUs, alternated passes; analyzers built without CMAKE_BUILD_TYPE, so without optimization, while the CI builds Release; the Release figures are in Status, under Cost in Release): about +24 to +25 % in a 6 GiB container (first version +14.6 % in the same conditions), +25 to +31 % in a 10 GiB one; at bed265d against the product: +27.5 % and +25.3 %. Step 5 adds about 1 to 2 % to the wall time, within the passes' noise. After the final review's fixes, in one 10 GiB campaign for both: without the option 806.5 s, bed265d +22.5 %, 42fa604 +22.2 %. The two heads differ by -0.2 % in wall time and -0.7 % in CPU, within the noise: the final review's fixes do not lower the cost, and the gap with the earlier +27.5 % of the same bed265d is the variation between campaigns. At 2318ea2, in one 10 GiB campaign on one binary, on 2318ea2's sources: without the option 809.0 s, with it 1052.2 s, +30.1 % in wall time, +17.0 % summed over the chunks and +14.7 % in CPU. Wall time grows more because the option's cost falls mostly on the three chunks that one of the two jobs runs in turn (+243.2 s, against +30.2 s for the other job's chunk). On these sources, 42fa604 and 2318ea2 take 1041.9 and 1043.2 s in their first passes, their only passes without --verbose that ran without the outside load. The measurement reserves (Status, above) give the readings without the loaded passes, and the clock's bound under its condition. The earlier campaigns analyzed their own head's sources, so their figures are not compared with these. On the corpora, on the same binary, in CPU: zlib +126 %, tinyxml2 +8.3 %, PCRE2 +5.5 %. PCRE2's time is almost all the chunk that holds pcre2_match.c (about 300 s without the option); without that unit, on another machine, the option added +126 %. The +10 % criterion is not met; the overrun is not accepted.

Final review

A fresh review of the whole branch at bed265d found two critical defects, both reproduced end to end, one important point besides the dependency on #241 (I1, above), and seven minor ones. 60a9828 adds the tests first: at that commit, both checks and the two expectations of the unit test fail, and the witnesses pass. Each later commit builds without warning.

Finding Before Now Reproduced Test
C1, critical (3266376) the rewrite changes the specialization that the function calls, h<int *> to h<const int *>; h is only declared in the file, and another file instantiates h<int *>: checked, while the rewritten code fails to link not checked, naming h<int *>; with h defined in its header, still checked end to end check_const_suggestion_external_dependency_only_declared
C2, critical (a01fbf5) a free operator declared again with its old type in a file that only the compilation database lists: "checked in every file of the compilation database" that file is judged (D17): not checked; a file that writes another operator only leaves the suggestion checked end to end check_const_suggestion_operator_declared_elsewhere
M5, minor (c38fd58) the rewrite skipped a file it could not read, and threw std::out_of_range on an edit past the end of a file the file is named, and the suggestion kept as not checked in the unit test only; no input of the analysis found that reaches it: defensive testConstSuggestionRewriteFailures
M6, minor (42c9a4d) a rewritten file that does not compile, without an error that the rewrite adds: removed not checked no input found: defensive none
I2, important (42fa604) --help and include/analysis/ConstSuggestionCheck.hpp stated the first version's rule: a suggestion is removed when the code no longer compiles or generates different code they state the three verdicts text none

On the corpora, no input reaches these fixes (Stage 3 above): they are shown by their tests only, and M6 by none.

The other minor findings are followed up apart: #250 (M1, an alias of a deduced parameter type keeps the suggestion not checked), #248 (M2, not reproduced), #251 (M3), #252 (M4), #253 (M7). #249 tracks building the Clang invocation in one function, left from Task 3.2; leaving it there does not accept the overrun.

Targeted review of these fixes (a fresh reviewer, read-only, at 42fa604; the first two points reproduced on macOS): no refutation is lost, nothing changes without the option, and the order of §5.8 and the judgement of collisions (§5.10) are kept. What it found, and what was done:

  • Important, from a01fbf5 (C2): kept for now, and described in the README. A free operator is looked up by the keyword operator, which most C++ files contain after preprocessing, through the standard headers. For a suggestion on a free operator with external linkage, every such file of the compilation database is compiled as it is and with the rewrite, and one that Clang cannot compile as it is keeps the suggestion not checked: with an unrelated z.cpp that includes <string> and has an error, "not checked with Clang: z.cpp does not compile with Clang as it is"; without the include, checked. Such a file never refutes; the cost is in compilations and verdicts. No suggestion of the corpora is on an operator. c56c99d describes it in the README, which says that it can cost far more than for a function named by an identifier; no cost was measured. It is kept for now, as decided for this PR. Narrowing it, or reducing its cost, is left out of this PR; leaving system headers out is excluded, since a file whose header, included with -isystem, declares the function must still be judged.
  • Minor, from a01fbf5: fixed in 902e12d. A function whose name starts with operator followed by $ or a non-ASCII letter (operator$x, operatoré) was looked up as the keyword operator: a file of the database that declares it with its old type was not judged, and the suggestion was reported checked (D17 missed; the same case named opx was not checked). The plan now records whether Clang's kind of the name is an operator's, and the lookup uses it. 94deb0d adds check_const_suggestion_identifier_starting_with_operator (both cases checked before 902e12d, not checked since) and check_const_suggestion_declared_in_a_system_header (an operator and an identifier declared only through a header included with -isystem: not checked; it fails when the scan leaves system headers out). It also makes check_const_suggestion_operator_declared_elsewhere assert, from the trace, the files judged and their verdicts, and name b.cpp in its docstring: when the keyword operator is not recorded, its "another operator only" case now fails, although its messages are those the former check accepted.
  • Minor texts: fixed in c56c99d. --help and the header comment state the exceptions to removal (a fatal error, or a change beyond the declarations the check plans, keeps the suggestion not checked). The README lists the C1 case and M6. The comments of the check, of rewrite() (what it detects is an edit past the end of the file) and of the facts after an error say what the code does.
  • M6's reason: changed in 6db974b. It names the first error, not an earlier warning, which the code had before the rewrite.
  • Still open. M6 has no test, nor 6db974b's change of its reason, and by reading no input reaches it. C1's boundaries with one specialization defined, or with an inheriting constructor, are not tested.

Validation

At c84b959 (2318ea2, the two fixes, their formatting and the test of a defining file that imports a module), and on a first version of the fixes:

  • Linux arm64 (Docker, Ubuntu 24.04, LLVM 20.1.8, GCC 13.3, a fresh build of c84b959): no warning; unit and ownership tests pass; the suite passes 2740/2740: 2318ea2's 2737 tests and the three checks added.
  • Linux x86_64, macos-15 and macos-latest, in CI (below): the suite passes 2740/2740 on each. On Linux x86_64, the suite ran only in CI, on the merge ref, whose analyzer code is c84b959's.
  • macOS arm64, locally (LLVM 20.1.2, AppleClang 17): at 8658cd0, an incremental build reports no warning (every file that includes the changed header recompiled), unit and ownership tests pass, and the suite passes 2739/2739. The analyzer built at c84b959 is byte for byte the same file as 8658cd0's (8f875a7 changes whitespace only), and the three new checks pass with it.
  • A first version of the fixes, not these commits: on Linux x86_64 outside CI, that version passed (Docker, Ubuntu 24.04, LLVM 20.1.8, GCC 13.3, fresh build): no warning, unit and ownership tests pass, suite 2739/2739. The first fix is the same in both versions. For the second, the review that followed added, in the code, the module implementation unit to the imports that the scan records, and the exception for functions that no other file can declare; in the test, which has 5 cases in that run and 7 at 8658cd0, the implementation-unit case, the static witness, the case of a file that imports a module and reads the rewritten header, and the resolution of a symbolic link in LLVM_DIR when the test looks for the clang++ that builds its precompiled header and header unit.

At 2318ea2 (main at f71bdac, merged in 2318ea2): on macOS arm64 (LLVM 20, AppleClang 17) and Linux arm64 (Docker, Ubuntu 24.04, LLVM 20.1.8, GCC 13), unit tests (318 expectations) and ownership tests pass, and the suite passes 2737/2737. On Linux x86_64 (Docker, Ubuntu 24.04, LLVM 20.1.8, GCC 13.3), at the start of the campaign above, the build reports no warning, unit and ownership tests pass, and the suite passes 2737/2737. These local builds are not Release builds: on macOS the tests ran on an incremental build, and a separate clean build of 2318ea2 reports no warning; the Linux arm64 build is a full one, without warning. In CI, on the merge ref e4f8e55, whose tree is 2318ea2's, the suite passes 2737/2737 on ubuntu-24.04 (Linux x86_64), macos-14 (AppleClang 15) and macos-latest (AppleClang 21). The CI's Release builds report the warnings that main has too (#258's run): one -Wunused-result in test/unit/analyzer_module_unit_tests.cpp with GCC 13, and 24 -Wpreferred-type-bitfield-enum-conversion in Clang 20's clang/Basic/LangOptions.def with AppleClang 21. The merge adds exactly main's change since 172ffd7, merged in 4f95305 (#246), and leaves the branch's own change as it was. Against the tree of c56c99d, the head before the merge (unit 313, suite 2685/2685 on macOS), what changes is #246's: its 5 unit expectations, which the suite's log repeats, and the 52 tests of its 6 fixtures under test/bound-storage, with their 6 checks that the human and JSON outputs agree. Besides, the check of the coretrace-compiler revision that a build records prints other commit ids, as on every run, since it commits, at the current date, in git repositories it creates when it runs. On macOS, each state of this batch builds without warning (incremental builds); with the code of 94deb0d, which is 42fa604's, check_const_suggestion_identifier_starting_with_operator fails on both cases.

Earlier, at 42fa604 (main at 1be6d3b, merged in 72dd688), on macOS arm64 (LLVM 20), Linux arm64 (Docker, Ubuntu 24.04, LLVM 20.1.8) and Linux x86_64 (Docker, Ubuntu 24.04, LLVM 20.1.8): built without warning; unit and ownership tests pass; the suite, as CI runs it, passes 2683/2683 on each. 4f95305 merges main at 172ffd7 (#254), which changes only .github/workflows/ci.yml and .github/workflows/test-ci-integration.yml; the analyzer's sources are those of 42fa604. At bed265d: the same on the three platforms, 2681/2681.

CI and code scanning

On 57aa98a (merge ref dd55112, into main at db7fb25, which brings #267): every check passes, format-check included; the release job is skipped, as outside tags.

  • The builds pass on ubuntu-24.04, macos-15 and macos-latest. On each, the suite passes 2971/2971, with main's new tests, check_const_suggestion_shares_only_the_defining_file included.
  • Code scanning: coretrace-stack-analyzer reported 86 results (73 on main at db7fb25), coretrace-concurrency-analyzer none. The alerts open here and not on main are the same seven Recursion.Detected notes (#273, #286 to #291): the change adds or closes none.

On af12b11 (merge ref bea143c, into main at 4a8810a, which brings #262, #264 and #265): every check passes, format-check included; the release job is skipped, as outside tags.

  • The builds pass on ubuntu-24.04, macos-15 and macos-latest. On each, the suite passes 2928/2928, with main's new tests.
  • Code scanning: coretrace-stack-analyzer reported 85 results (72 on main at 4a8810a), coretrace-concurrency-analyzer none. The alerts open here and not on main are the same seven Recursion.Detected notes (#273, #286 to #291): the change adds or closes none.

On 70762e7 (merge ref 1d17e0f, into main at b1228c6; its tree is 70762e7's, which merges that main): every check passes, format-check included; the release job is skipped, as outside tags.

  • The builds pass on ubuntu-24.04, macos-15 and macos-latest. On each, the suite passes 2740/2740.
  • Code scanning: coretrace-stack-analyzer reported 83 results (70 on main at b1228c6), coretrace-concurrency-analyzer none. The alerts open here and not on main are the same seven Recursion.Detected notes (#273, #286 to #291).

On c84b959 (merge ref 26db4d4, into main at 703da23; its tree differs from c84b959's only by main's changes since f71bdac: .github/workflows/ci.yml, action.yml, the LLVM 20 floor in CMakeLists.txt and scripts/build.sh, the README, CONTRIBUTING.md and the changelog): every check passes, format-check included; the release job is skipped, as outside tags.

  • The builds pass on ubuntu-24.04, macos-15 and macos-latest. On each, the suite passes 2740/2740, every case of the three new checks included. macos-14 no longer runs (ci: build on macos-15 with Xcode 16.0 before macos-14 is retired #258).
  • Code scanning: coretrace-stack-analyzer reported 83 results, coretrace-concurrency-analyzer none. The alerts open here and not on main are the seven Recursion.Detected notes listed for bed265d below (#273, #286 to #291): the fixes add or close none.

On 8658cd0 (merge ref 0cb919d, into main at 2e3c12c): the builds passed on the same three runners, 2739/2739 each, and format-check failed on the two fixes, on whitespace only; 8f875a7 formats them.

On 2318ea2 (merge ref e4f8e55, into main at f71bdac): every check passes, format-check and macos-14 included (the release job is skipped, as outside tags); macos-14 ran before the brownout of October 31. Code scanning: coretrace-stack-analyzer reported 83 results, coretrace-concurrency-analyzer none. The alerts open here and not on main are the seven Recursion.Detected notes listed for bed265d below (#273, #286 to #291), at other lines of ConstSuggestionFacts.cpp: this batch adds or closes none. The one result fewer than on 4f95305 is #224, a Recursion.Detected note in StackBufferAnalysis.cpp that #246 fixed on main too (70 results there, against 71 before).

Earlier runs:

On 4f95305 (merge ref cd0f115, into main at 172ffd7): every check passes, format-check and macos-14 included. macos-14 first failed before any step, in a scheduled brownout of the macOS 14 image (actions/runner-images#13518; the image is retired on November 2, 2026); re-run outside the brownout, it passes (Xcode 15.4): unit tests, const-suggestion-explicit-object.cpp, suite 2683/2683. Code scanning: coretrace-stack-analyzer reported 84 results, coretrace-concurrency-analyzer none. The alerts open here and not on main are the seven Recursion.Detected notes listed for bed265d below (#273, #286 to #291), at other lines of ConstSuggestionFacts.cpp: the fixes add or close none.

On bed265d (merge ref a629028, into main at 1be6d3b): every check passes, macos-14 and format-check included. Code scanning: coretrace-stack-analyzer reported 84 results, coretrace-concurrency-analyzer none. Seven alerts are open here and not on main, all Recursion.Detected notes in ConstSuggestionFacts.cpp, intended recursions over types and the AST: #273 (Inspector::TraverseDecl, below), and #286 to #291, writtenForm and its lambda (D19, the structure of a type), TraverseParmVarDecl and TraverseTypeLoc (the AST traversal), typeKey and argumentsKey (D20, mutually recursive over types and template arguments). Kept open, like the 44 notes of this rule then open on main.

CI on fe43067 (merge ref 2555947, into main at f846712), b17ff2a with the two D10 fixtures formatted: every check passes, macos-14 and format-check included (format-check had failed on b17ff2a for those fixtures alone). Code scanning: coretrace-stack-analyzer reported 78 results, coretrace-concurrency-analyzer none; no new alert. The only alert open here and not on main is still #273 (below).

Previously, CI on 4fd0dae (merge ref 14ce152, into main at f846712): every check passes. On macos-14 (AppleClang 15, Xcode 15.4), const-suggestion-explicit-object.cpp now passes in both passes, the six checks of testSymbolDemangling pass, and the suite passes 2645/2645. It failed there on 00a21b7 and 519500a, because macOS 14's demangler does not read C++23 explicit object members (#240). The fixture fails the same way on #221 (b8538c5).

Code scanning, against main at f846712: coretrace-stack-analyzer reported 78 results (71 on main), coretrace-concurrency-analyzer none. One alert is open here and not on main: #273.

The first run, on 00a21b7 (merge ref 29d24b0), had 13 new alerts, all in the new files and all reproduced locally:

…ated code

A const suggestion must compile in every analyzed file and keep the generated code.
…adata included

A const suggestion must hold in every file of the compilation database, metadata included.
…ic metadata

Recheck every file of the compilation database and compare semantic metadata.
…ly if unique

A specialization whose deduced type changes is paired only when unique, and its code compared.
Keep a suggestion whose rewrite only changes what a forwarding reference deduces.
…e code

Collapsed specializations with the same code must still be refused.
…write

A rewrite that reaches the preamble parses the file whole, and a header's warning is not new.
Name a diagnostic's file by its real path, so a header's warning is not counted as new.
Drop the precompiled preamble, its gain does not justify its invariants.
…ption

The Clang check of const suggestions runs only with its experimental option.
Check const suggestions with Clang only under --experimental-const-suggestion-check.
…moval reasons

Tests for the review of the experimental check: a static or anonymous-namespace homonym, a macro that writes several parameters, (void)f, an explicit object parameter, CRLF continuation lines, identifiers that contain const, the reason a removal gives; every const-suggestion input with and without the option; the human/JSON parity of a fixture with its own arguments.
A homonym with internal linkage in another file is another function. A macro that writes other parameters is not rewritten. (void)f is no conversion of the address. An explicit object parameter is numbered like the others. A continuation line ending in CRLF stays in its #define. An identifier that contains const keeps it. A removal names the first diagnostic the rewrite adds, error or checked warning. The compile command of each unit is resolved once per analysis.
ci.yml runs the whole run_test.py suite on Linux and macOS, with the same toolchains and configuration, the check's tests included: the dedicated job only ran them a second time.
clang-format 20, as the format check of the CI runs it. In const-suggestion-deduction-kept.cpp, the template declaration now takes one line: its expectations move from line 25 to line 24.
Code scanning alerts #268, #269 and #270 on #221: Inspector::plan's target, writtenType's context and the list lambda's node are only read. The experimental check keeps the three suggestions.
…ders

The image of v0.7.1 had LLVM's headers but not Clang's, so
ConstSuggestionCheck.cpp did not compile in it and the concurrency job
failed. v0.8.0 installs libclang-20-dev with clang-20
(CoreTrace/coretrace-concurrency-analyzer#204). Over this branch's
sources, its arm64 image exits 0, with no unit skipped and no result.

The image is pinned rather than left to the action's default, which
moves with the v0 tag.
T v1 finds the function by its decoded names and the parameter by its
debug number, plans no scope check, and removes a suggestion on any
unexplained code difference. The design gives T explicit identities, a
plan validated before the rewrite, units made of a file and its
configuration, a closed list of compared facts, established
correspondences, and three verdicts: verified, refuted, indeterminate.

Refs #217
- The checked warning groups are enabled with -W<group>, then kept as
  warnings with -Wno-error=<group>, before and after the rewrite.
- The scope invariant covers the declarations written in the sources;
  instantiated signatures are left to the correspondences.
- Heuristics have no say in a verdict.
- D16: sizeof(__PRETTY_FUNCTION__) becomes indeterminate, and the limit
  this creates is documented.

Refs #217
Count the parameters of a variadic function without its trailing entry, read the declared parameters from the debug declarations, and leave an unestablished layout without a type. Share each unit's initial compilation across entities. Record D17 and the fixture that D13 adds.
The prototype binds every case from the declaration that spilledParameter finds, with no output change on the fixtures. The subroutine type stays a fallback for an untyped declaration, only where its entries are established; the position reconciliation is dropped.
Step 9 recognizes the hidden return argument by its sret attribute instead of leaving the ... entry out of the count, which misbound optimized variadic functions. The effects list the artificial parameters that are no longer reported; Step 14 holds the review's test cases.
…d rewrites

Cases from the final review of #239. A renamed dependency that the file
only declares must keep the suggestion not checked, while one defined in
its header stays checked. A free operator declared again in a file of the
compilation database must keep it not checked, while a file with another
operator only does not. A rewrite whose file cannot be read, or no
longer has the planned text, must name the file without an exception.

rewrite() is exposed with its behavior unchanged so that the unit test
compiles; these cases fail until the next commit.
… only declared

A rewrite can change the specialization a function calls, h<int *> to
h<const int *>, when h is only declared in the file. Comparing the two
declarations proves nothing about the code another file defines for h,
and none may define the new one: the link failed after a suggestion
reported as checked. A renamed symbol that the file only declares on
both sides, other than the entity's own variants and inheriting
constructors, now keeps the suggestion not checked and names it.
…atabase

The scan of the compilation database records identifiers, and an
operator's name is not one: a file that declared operator== again with
its old type was not judged, and the suggestion was reported as checked
in every file of the database. The scan now records `operator` for an
operator, and the lookup uses the same token.
rewrite() skipped a file it could not read and threw std::out_of_range
on an edit past the end of the file. It now names the file, and the
suggestion is kept as not checked.
A rewrite that does not compile, and adds no error that explains it, is
not shown to be wrong: it is now kept as not checked instead of removed.
… and the header

Both still said that a suggestion is removed when the code no longer
compiles or generates different code. A suggestion is removed when the
rewrite adds an error or a tracked warning, or changes what Clang
resolves, and kept as not checked when it cannot be shown valid.
… declarations

Cases from the targeted review of #239's fixes. A C function named by an
identifier that starts with operator, operator$x or operatoré, and
declared again with its current type in a file of the compilation
database must keep the suggestion not checked; until the next commit it
is reported checked. A declaration made by a header included with
-isystem must keep it not checked too, for an operator and for an
identifier. The check of a free operator declared elsewhere now asserts,
from the trace, which units are judged and their verdicts.
…kind

The lookup in the compilation database took a name for an operator's when
it started with operator and went on with neither a letter, a digit nor
an underscore: operator$x and operatoré were looked up as the keyword
operator, a file that declares them again was not judged, and the
suggestion was reported checked. The plan now records whether Clang's
kind of the name is an operator's, and the lookup uses that. For a free
operator, every unit that writes the keyword is still judged, system
headers included.
…rning

When the rewritten code does not compile and the rewrite adds no error,
the reason cited the first diagnostic, which can be a tracked warning the
code already had. It now cites the first error. No input is known to
reach this path (#239 review, M6).
… its cost

--help and ConstSuggestionCheck.hpp stated removal without the steps
that stop the check first: a fatal error, or a change beyond the
declarations it plans. The README now lists a renamed symbol that the
file only declares and a failed rewrite that adds no error, and says
that an operator's suggestion compiles most C++ files of the database,
and what that costs. The comments on the check, on rewrite() and on the
facts after an error claimed more than the code does.
SizzleUnrlsd added a commit that referenced this pull request Nov 1, 2026
GitHub retires the macos-14 image on November 2, 2026, and fails its jobs
during the scheduled brownouts before that (actions/runner-images#13518):
"Build on macos-14" was cancelled before any step on #239 and #241 on
October 30.

The job moves to macos-15 with Xcode 16.0, the oldest Xcode of that
image: its AppleClang 16.0.0 is the oldest Apple compiler GitHub's
runners still provide. The step fails if the image no longer has Xcode
16.0, rather than building with another one.

Two things are then no longer tested: AppleClang 15, the compiler the
job was added for (#228), since macos-14 is the only image with Xcode
15; and the macOS 14 C++ runtime, whose demangler predates C++23
explicit object members (#240). These are losses of coverage, not a
decision to drop compatibility with either.
…conditional

In C, a conditional between two functions whose types the rewrite of one
makes differ is a constraint violation, which Clang reports as
-Wpointer-type-mismatch, a warning by default. PCRE2's
callout_enumerate_function_8 reaches pcre2_callout_enumerate that way,
and its two suggestions are reported checked (#239). The test passes f1
or f2 through ?:, with the default warnings and with -Werror
-Wno-pointer-type-mismatch: the four suggestions must be removed for
that warning; until the next commit they are reported checked. A
conditional between const blk * and blk *, which C allows, must keep its
suggestions checked.
A rewrite that makes the two callbacks of a conditional differ adds this
warning, and the suggestion was kept as checked. The group is now among
the tracked warnings: the check enables it and, under -Werror, keeps it
the warning it is by default.
Preprocessing does not read a module that a file imports. A file that
gets f's declaration from one and never names f is not judged, and the
suggestion is reported checked. The test covers a Clang header module, a
C++20 header unit, a named module and a module implementation unit: each
must leave the suggestion not checked; until the next commit, each is
reported checked. A PCH, which preprocessing reads, stays a witness: the
file is judged and declares f (D17). A refutation by another file must
still win, with the importing file in the trace, and a file that imports
a module and includes the rewritten header must still be judged. A
static function stays checked in every case.
…dule

The scan now records whether a unit imports a module: the token of an
#include that becomes an import or of a header unit, an import
declaration, or the unit's own module declaration when it implements a
module. A unit that imports one, and does not see the change otherwise,
leaves the suggestion not checked, unless the rewrite changes only the
defining file and the function is not a free function with external
linkage: no other file can declare it then. A refutation still wins.
format-check failed on 8658cd0: clang-format 20 puts the tracked
warnings one per line now that the list has six, and breaks a lambda of
the import check before its brace. Whitespace only; the analyzer built
from it is byte for byte the same.
a.cpp, alone in the compilation database, defines f and imports the
Clang header module Pre, whose header includes a.h and initializes an
int (*)(int *) with f: the rewrite of f's parameter breaks it. a.cpp's
compilation loads Pre, which the check does not rebuild with the
rewritten a.h, so Pre's declaration of f keeps its type and the
suggestion stays not checked. Without -fmodules, a.cpp reads pre.h as
text, and the rewrite is refuted at pre.h:2. No code changes with this
test: it passes as the check stands, and fails when the verdict or its
reason changes.
Two specializations of one template get suggestions with the same rewrite: their
compilations after it have the same command, directory and texts, in the defining file
and in every other file that sees the rewrite. Each such compilation must run once, and
each suggestion must keep its own judgement in it: the address of first<int> is cast
explicitly (D15), which leaves it not checked where that happens, while first<double>
is checked.

Both checks fail on the current head, on the number of compilations only (2 instead of
1, and 4 instead of 2); their verdicts already hold.
… share it

Suggestions with the same rewrite, such as the specializations of one template, compiled
each unit after it once per suggestion, with the same inputs: the same command, directory
and texts. One compilation now serves them all (inspectEach): CodeGen, then one inspection
for each suggestion, with its own targets, plan and sites, as its own compilation would
give them. Every inspection gets the compilation's diagnostics, and they share its module,
prepared once since preparing changes it. Each suggestion is still judged alone and in the
same order: scope, facts, correspondences, code comparison, verdict, and the same trace.

The compilation made with an earlier suggestion waits for each later one until it reaches
that unit, and goes once that suggestion is judged.

A unit test compares, for each target, everything the judgement reads from a shared
compilation with its own compilation: facts, declarations, sites, diagnostics and code,
with an address cast in one specialization and a rewrite that does not compile.
…write

Suggestions with the same rewrite now share the defining file's compilation after it, and
only that one: another file that sees the rewrite is compiled for each suggestion and
released before the next, as without sharing. With a header rewrite that the defining
file and another file see, that is 3 compilations for 2 suggestions, each keeping its own
judgement in each file.

The check fails at af12b11 on the number of compilations only (2 instead of 3).
…s module

A compilation after a rewrite shared by several suggestions was kept, module included, for
each later suggestion until it reached that file. With a header rewrite that several files
see, every one of their modules stayed alive until the next suggestion, where they used
to go file by file.

Only the defining file's compilation is shared now, and a later suggestion keeps of it what
its judgement reads: the facts and the code prepared once from the module. The module goes
with the judgement of the suggestion that made the compilation, as without sharing.
Another file is compiled for each suggestion and released before the next, as before
af12b11. A kept compilation that has prepared code compiled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ParameterDebugBinding unit tests expect an empty parameter to take no IR argument, which fails on Linux arm64

2 participants