Repository navigation
Conversation
| // find variables | ||
| scope.getVariableList(); | ||
|
|
||
| if (mTokenizer.isC() && scope.type == ScopeType::eGlobal) { |
There was a problem hiding this comment.
This is an AI review take it with a grain of salt. Please feel free to reject it by clicking on Resolve button
I checked this for new false positives, built against its merge-base:
- C corpus: ~370 open-source C files (mbedtls, wpa_supplicant crypto/ap, STM32 HAL) plus
test/cfg/*.candsamples, with--enable=style,warning,performance,portability --inconclusive. The output was identical: 3715 warnings in both. - MISRA addon on
addons/test/misra/misra-test.c, the CTU tests and a probe file: the only difference is 4 fewermisra-c2012-8.9warnings, which were false positives (e.g.static int s; void w(void){ s = 5; } static int s = 0; void r(void){ ... s ... }). Base counted each declaration as an object used in only one function. - Targeted probes: write before a late initializer, address escape via a function pointer,
extern int a[10]; int a[] = {1,2,3};, tentative definition after an initialized one, globals shadowed by parameters, static locals and block-scopeextern. The PR removes 3 false positives that the merge-base has (twoknownConditionTrueFalse, onearrayIndexOutOfBoundsona[9]forextern int a[10]; int a[] = {1,2,3};) and adds none.
So from the false-positive point of view this looks safe to me. One unrelated existing issue I noticed: int g; void f(void) { extern int g; g = 4; } gives shadowVariable on both builds. Block-scope extern is out of scope for this PR, as the description says.
|
can you please clarify if you used AI for some of these changes? I fear we can't start accepting AI generated fixes in tokenizer/symboldatabase/valueflow. |
|
This pull request has merge conflicts that must be resolved. |
|
Test results for commit f775f03 (tools/test-my-pr.py, main compared to this PR): Test: http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8890/ Posted automatically by the cppcheck PR test runner. +N: warnings only with this PR, -N: warnings only with main. The AI review is written by Claude and can be wrong. |
C file-scope declarations such as
int x; int x;andextern int x; int x;currently receive different variable IDs and separateVariableobjects. This fixes both parts of Trac #6418, including the symbol-database issue noted in comment 2.Reuse the ID in the actual file-scope variable map, then combine C global declarations before assigning symbol pointers. Prefer the initialized/non-extern declaration while preserving linkage flags and previously complete array dimensions. Function parameters, local shadows, record members and C++ declaration identities remain separate.
Two supporting changes prevent regressions exposed by the shared identity: a late static initializer must not restore a constant value after an earlier function write or address escape, and a later unsized array initializer must not shrink a bound provided by an earlier declaration. The resulting analysis finds out-of-bounds accesses both before and after a complete array declaration, while accepting in-bounds accesses with partial initializers.
Preserve each merged declaration's original tokens and storage/initialization flags, and expose them as additive
<declaration>children in the XML dump. The Python parser retains one canonical variable and accepts older dumps. MISRA declaration checks and cross-file summaries use the occurrence metadata, so earlier extern declarations remain visible and declarations are not counted as ordinary uses. Internal and definition summaries retain one object identity.Validation on Windows with Clang 22.1.8 and CMake/Ninja (Debug, PCH disabled, serial build):
-Werror --strict-markers, including four new parser/declaration/CTU cases. Original suppression expectations are unchanged. The CTU reader now closes its input files deterministically.git diff --checkpass. The GCC range-loop warning in the new value-flow test is fixed.f775f03(the helper commit adds only its verification workflow): Ubuntu 22.04/GCC 11.4, standalone-Werrorand-Werror=range-loop-construct, 5,364 native tests with 360 existing TODOs and zero failures; 29 addon tests; 112 dump-parser tests with 3 expected failures; original MISRA CTU and C89/C11 verification. Compiler commands and logs are attached to that run. This is independent validation; the new upstream workflows still require maintainer approval. GUI was not tested in this follow-up.This covers compatible C file-scope redeclarations; it does not implement block-scope extern merging or full declaration compatibility checking.
Please assign this work to KiritoYG if needed and consider it under the published $10 bounty schedule, subject to acceptance and the required ticket closure. Please confirm eligibility and the supported settlement route; GitHub Sponsors is available if accepted. No award or payment is being claimed.