GeezerVison

Thanks for the contribution!

• Please create a issue. https://github.com/cppcheck-opensource/simplecpp/issues
• Please add a regression test

No need to repeat the whole issue description and what the fix does in the PR message as well.

I know AI loves to do that but it often gets out-of-sync if the code in the PR or the issue description changes (it did in this case) and might end up misleading (redundancies are never great).

Sorry for the delay, I just took into account your review and pushed force the changes.

I asked AI to review this. My experience is that AI reviews are almost always wrong, so take it with a mountain of salt. But please look at it and write your response..

What it claims to fix: when ## concatenates two tokens into a function-like macro name, and the ( for that macro's call is separated from it by
a comma (e.g. PAR(PREFIX_ ## kind, (VA_ARGS))), simplecpp aborts expansion instead of finding the (.

The motivating bug is real, and the target behavior is correct. I verified against gcc and clang:

#define PAR(a, ...) a VA_ARGS
#define PREFIX_SCALAR(T, N) T N
#define DISPATCH(kind, ...) PAR(PREFIX_ ## kind, (VA_ARGS))
DISPATCH(SCALAR, int, x)

• gcc/clang: int x
• master (built from HEAD): PREFIX_SCALAR ( int , x ) — under-expanded
• PR branch: int x — matches

This is legitimate standard-mandated behavior: PREFIX_ ## kind concatenates during argument substitution of DISPATCH, producing
PAR(PREFIX_SCALAR, (int, x)); rescanning that expands PAR to PREFIX_SCALAR (int, x); rescanning that finally invokes PREFIX_SCALAR. It's
ordinary multi-level rescanning (per 6.10.3.4), not something specific to ##.

However, the fix itself is not a correct implementation of that rescanning — it's a narrow pattern match that silently corrupts output on a
nearby, equally valid variant. The new code, when it doesn't find ( immediately after the concatenated token, walks forward on the same line
skipping , and expanding named tokens, looking for any (. It grabs whatever parenthesized thing it finds and treats it as the call's argument
list, without regard for which macro invocation that ( structurally belongs to.

Counter-example — just change PAR's body to put a comma between its parameters (equally valid C):

#define PAR2(a, ...) a, VA_ARGS
#define PREFIX_SCALAR(T, N) T N
#define DISPATCH2(kind, ...) PAR2(PREFIX_ ## kind, (VA_ARGS))
DISPATCH2(SCALAR, int, x)

• gcc/clang: PREFIX_SCALAR, (int, x) (correct — no adjacency, so no call)
• master: PREFIX_SCALAR , ( int , x ) — matches gcc/clang
• PR branch: int x , — wrong, and silently corrupted (no error at all)

Root cause: the forward scan in DISPATCH2's own replacement list sees PREFIX_ ## kind (→PREFIX_SCALAR), skips the comma, and grabs
(VA_ARGS) — but that (...) is actually PAR2's second argument, not call-parens for PREFIX_SCALAR. It swallows that argument into a
synthetic PREFIX_SCALAR(int, x) call, which fully expands to int x, leaving PAR2 invoked with only one argument and an empty variadic tail —
hence the mangled int x ,.

So the heuristic happens to work for the PR's own test cases only because PAR's body (a VA_ARGS) puts the substituted pieces directly
adjacent with no comma — it isn't simulating standard rescanning, it's guessing based on the concatenation site's local token layout, which has
no reliable relationship to where the eventual ( ends up after the enclosing macro's own substitution.

I confirmed the existing test suite (testrunner) still passes on the PR branch, and it compiles clean under -Wall -Wextra -pedantic -Werror —
but the PR's own regression test doesn't cover this case, so the corruption goes undetected.

Conclusion: the bug being fixed is real and the desired output is standards-correct, but this specific fix is unsound — it introduces a
silent-corruption regression for macros structurally very close to the motivating example. I'd recommend not merging as-is. A more robust
approach would extend the existing generic rescan machinery (rescanMacro/recursiveExpandToken from #682) so the
concatenated-name-followed-later-by-( case is handled through genuine iterative rescanning rather than an ad hoc local comma-skipping scan that
can't tell which enclosing call a ( belongs to.

Thanks for the thorough review — I've reworked the fix accordingly but before I push it, I want to know your opinion.

You were right that the forward scan was unsound. It was walking DISPATCH2's own replacement list and grabbing (VA_ARGS), which really belongs to PAR2's second argument, so it happily produced int x ,. I reproduced your counter-example and confirmed the old branch was silently corrupting it.

With AI help, I've dropped the forward scan entirely. expandHashHash now only keeps the case where ( is literally the next token after the pasted name (that one's still needed for things like X##ID(0)). Everything else is left to the rescanner, which is where it belonged.

The actual handling now hangs off the existing #682 machinery. After a parameter is substituted, if the output so far ends in a function-like macro name and the substituted parameter starts with (, it builds the call and expands it through the normal Macro::expand path. The important part is where it runs: only on the parameter-substitution branch. That's what makes it match the standard — a ( that came from parameter substitution is genuinely adjacent and binds, but a ( that only appears later because some object-like macro expanded during rescanning does not. Concretely:

• PAR(...) → int x
• PAR2(...) → PREFIX_SCALAR, (int, x)
• #define A B x / #define x (2) → B (2) (not 2)

all matching gcc/clang now.

I checked it against a spread of variants beyond the motivating one — variadic tails, nested indirection, chained rescans, self-referential macros, and pasted names that must stay literal — and they all line up with gcc. Your PAR2 case is now in the regression tests, along with a separate test that pins down the param-supplied-vs-macro-supplied ( boundary, since that's the subtle bit that's easy to regress. Full suite passes, clean under -Wall -Wextra -pedantic, and no leaks under ASan.

One small addition to your analysis: this isn't really about ## at all. The same bug shows up with no concatenation — e.g. outer(PREFIX_##k, (VA_ARGS)) behaves the same as a plain two-parameter indirection where a name comes from one arg and the ( from another. That's part of why I moved the logic into the rescan path rather than special-casing the paste site.

Let me know if you are agree with this new implementation and I'll push it.

Let me know if you are agree with this new implementation and I'll push it.

hmm I guess it sounds good. however I fear that if AI starts doing fixes in simplecpp this library will not work well very quickly.
I want that every PR author (a human) have good understanding of what his/her PR does.

personally I feel the simplecpp code is very difficult and there are details that I don't remember

I fear that I didn't understand the standard behavior exactly when I implemented simplecpp so I wrote it according to my rules in my head and then we have tried to tweak it afterwards to handle various special cases. It might be a good idea to take a look at the structure and see how it mismatches the standard..

This pull request has merge conflicts that must be resolved.