[pfultz2]

cppcheckdata.match() re-parsed its pattern string on every call: it split the pattern, split off the @ bindings, and match_atom() ran through a chain of in/startswith checks for each word before doing the actual comparison. On a failed match it also rebuilt the list of binding keys and allocated a new MatchResult. Since an addon typically matches the same handful of pattern strings against every token, almost all of this work was repeated.

This PR adds a CompiledPattern class that compiles a pattern string once into a list of per-word matcher functions, plus a precomputed MatchResult for the failed case. match(token, pattern) keeps its signature and looks the compiled pattern up in a cache keyed by the pattern string. The internal match_atom() helper is removed; its logic is now CompiledPattern.compile_atom().

Two behavior changes, both for degenerate patterns:

• A whitespace-only pattern used to report a successful match (with end = None). It now fails, the same as an empty pattern.
• |= is now a literal word. Previously the | in it was also read as an alternation of '' and =, so the pattern |= matched a = token as well. (The C++ Token::Match still has that quirk; the Python side no longer shares it.)

Performance

Measured on the first configuration of a lib/token.cpp dump (49,349 tokens), Python 3.

12 patterns (if (*) {, %name% (*) ;, %var%@lhs = %var%@op1 | %var%@op2 ;, struct|class %name% {, %name% !!(, for (*) {*}, %comp%, ::, …) matched against every token, ~592k calls:

	before	after	speedup
total time	0.78 s	0.21 s	~3.7×
per call	~1.3 µs	~0.36 µs

200,000 calls of match(tok, '%name% (*) ;') on one token (per-call overhead, no matching work):

	before	after	speedup
total time	0.23 s	0.04 s	~5.5×

One-time compile cost per distinct pattern (not cached per word; happens once per pattern string):

pattern	compile
if (*) {	1.6 µs
%var%@lhs = %var%@op1 | %var%@op2 ;	2.3 µs
struct|class %name% {	2.0 µs
**{	1.3 µs

[dmcppcheck]

AI review

I found two new problems; neither causes a wrong match result.

1. One failed result object is reused for every call with the same pattern. CompiledPattern.failed is a single MatchResult, and match() hands that same object back on every failed call with that pattern. MatchResult is an ordinary mutable object. If an addon sets or changes an attribute on a failed result, every later failed match with that pattern returns the changed value. Before this PR each failed call got a new object, so this is a new way for state to leak between calls.
2. The |= fix only covers |= as a whole word. |= is literal only when it is the entire word. Inside !!|=, **|= or an alternation such as a|=|b, the '|' in p split runs first, as it did before. So !!|= still means "!! or =" and matches any token, including a |= token. Behaviour is unchanged here, but the PR description says the quirk is fixed.

Apart from the two documented behaviour changes, a differential fuzz of old vs new match() found no difference in results.

Testing

• I built cppcheck in pr/ and ran python3 -m pytest -q test/addon --cppcheck-binary=./cppcheck: 114 passed, 3 xfailed.
• Differential fuzz (scratch/fz/fuzz.py): this loads base/addons/cppcheckdata.py and pr/addons/cppcheckdata.py side by side. It uses mock linked tokens for f ( a , b ) { a |= b ; a = a | b ; if ( a || ! b ) { ... } ... }. It generated 3×200k random patterns from a vocabulary of literals, %…% classes, !!x, **x, alternations, bindings (@x), empty words and |=, and compared truthiness, end and the bindings for every token and for None.
  • Every difference involved one of the two documented changes: a whole-word |=, or a whitespace-only pattern.
  • After filtering those out, there were 0 differences.
  • No exceptions or hangs.
• Reusing the failed result (mock tokens from the fuzz script):

  r = m.match(None, 'a@x'); r.x = 'stale'
  print(m.match(toks[0], 'a@x').x)

  Output:

  old fresh failed .x = None
  new fresh failed .x = stale
  

• |= inside compound words: match(tok_or_assign, '!!|=') returns True with both old and new. match(tok_or_assign, 'a|=|b') returns False with both.

---

Automated review of ec8567f by Claude. The pull request was built and tested locally.

[dmcppcheck]

AI review

match() returns this one MatchResult from every failed call with this pattern, and MatchResult is mutable. If an addon assigns to a failed result (r = match(tok, 'a@x'); r.x = ...), every later failed match with the same pattern returns that value: I reproduced .x == 'stale' on a later, unrelated failed match. Before this PR each failed call got a new object. A simple fix is to store only keys and build MatchResult(False, keys=self._failed_keys) on failure, which is still much cheaper than before. Another option is to make MatchResult reject attribute assignment.

[pfultz2]

I dont think this is an issue. The MatchResult shouldn't be modified.

[dmcppcheck]

AI review

|= is literal only when it is the whole word. In !!|=, **|= or an alternation such as a|=|b, this '|' in p split still runs before the !!/** handling. So !!|= still means !! OR = and matches every token, including |= itself; I verified this against old and new. Also, |= can't be one of the options in an alternation. The PR description says the |= quirk is fixed. Either handle the !!/** prefixes before splitting on |, as the C++ Token::Match does for !!, or narrow the claim in the description.

[pfultz2]

Fixed.

[dmcppcheck]

AI review

I found one new problem, a small regression: match(tok, None) now raises AttributeError. Before this PR it returned a failed result. The old code started with if not pattern: return MatchResult(False), and that guard is gone, so CompiledPattern(None) calls None.split(). No in-tree addon calls match(), but out-of-tree addons that pass an optional pattern would now crash.

The description is also missing some behaviour changes. The latest commit changed more than the two cases it lists: !!||, !!|, **|| and **| used to split on |, so !!|| and !!| matched every token. Now they mean "not ||", "not |" and so on. These changes are improvements, but the description should list them.

The other problems I saw (the shared failed result, |= inside alternations) are already in comments.md.

Testing

• I built cppcheck in pr/, then ran python3 -m pytest -q test/addon --cppcheck-binary=./cppcheck: 117 passed, 3 xfailed. All of the new tests pass: test_literal_or_assign, test_whitespace_only_pattern, test_negation_* and test_find_forward_alternatives.
• I compared old and new behaviour by loading base/addons/cppcheckdata.py and pr/addons/cppcheckdata.py side by side, using a mock token (scratch/t.py):

  class T: pass
  t = T(); t.str = 'a'; t.next = None; t.link = None
  m.match(t, None)
  t2 = T(); t2.str = '||'; t2.next = None; t2.link = None
  for pat in ['!!||', '!!|', '**|', '!!|=']: m.match(t2, pat)

  python3 -I t.py gives:

  base None False
  pr None EXC AttributeError 'NoneType' object has no attribute 'split'
  base !!|| True      pr !!|| False
  base !!|  True      pr !!|  True
  base **|  False     pr **|  False
  base !!|= True      pr !!|= True
  

  • With None as the pattern, match() now raises an exception. Before, it returned a failed match.
  • !!|| changed meaning: before, it matched every token, including ||; now it is "not ||". The PR description doesn't mention this.

---

Automated review of c59f7f1 by Claude. The pull request was built and tested locally.

[dmcppcheck]

AI review

The old match() started with if not pattern: return MatchResult(False). Without that guard, match(tok, None) now raises AttributeError: 'NoneType' object has no attribute 'split' in CompiledPattern.__init__; before, it returned a failed result. I reproduced this against base and PR. A short guard here keeps the old behaviour:

if not pattern:
    return MatchResult(False)

That guard also keeps falsy patterns out of _pattern_cache.

[dmcppcheck]

AI review

I found two new problems, both caused by the new pattern cache. Neither produces a wrong result for a fixed pattern string used by an addon that leaves patterns alone.

1. The cache has no size limit, and patterns built at run time get slower. _pattern_cache stores every distinct pattern string forever. An addon that builds patterns at run time, such as match(tok, '%s ( ' % name), now pays the compile cost on every new string and keeps every compiled pattern alive. With 200k distinct patterns, match() is about 5× slower than before and the cache ends up with 200k entries.
2. Compiled patterns copy entries of the module-level patterns dict. compile_atom() copies patterns[p] when the pattern is compiled. If an addon adds a %foo% entry to cppcheckdata.patterns after a pattern using %foo% was compiled, the cached pattern still treats %foo% as a literal. Before this PR the dict was looked up on every call, so the new entry was used.

The guard that C7 asked for is back. !!|= and **|= now work, but |= still can't be one of the options in an alternation (a|=|b).

Earlier comments

#	Comment	By	Where	Status
1	Shared mutable failed MatchResult is returned from every failed call with the same pattern	AI review	general	💬 Answered: Author: MatchResult shouldn't be modified
2	The |= fix only works when |= is the whole word, not in !!|=, **|= or alternations	AI review	general	✅ Fixed
3	match(tok, None) raises AttributeError	AI review	general	✅ Fixed
4	Description doesn't list the changed meaning of !!||, !!|, **|| and **|	AI review	general	⚠️ Open

@pfultz2, please fix, resolve or reply to the open comments: #4.

Testing

• Built cppcheck in pr/.
• Ran python3 -m pytest -q test/addon --cppcheck-binary=./cppcheck: 117 passed, 3 xfailed.
• To compare old and new behaviour, I loaded base/addons/cppcheckdata.py and pr/addons/cppcheckdata.py side by side with mock tokens for x = y ;. Run with python3 -I scratch/t.py:

for m in (b, p):
    print(m.__name__, bool(m.match(toks[0], '%foo% =')), end=' ')
    m.patterns['%foo%'] = lambda t: t if t.isName else None
    print(bool(m.match(toks[0], '%foo% =')))
for m in (b, p):
    t0 = time.time()
    for i in range(200000):
        m.match(toks[0], 'name%d =' % i)
    print(m.__name__, 'dyn', round(time.time()-t0, 2), len(getattr(m, '_pattern_cache', {})))

Output:

b False True
p False False      <- new %foo% entry ignored, the cached pattern is stale
b dyn 0.21 0
p dyn 1.03 200001  <- 5x slower, 200k cache entries kept

---

Automated review of 955b562 by Claude. The pull request was built and tested locally.

[dmcppcheck]

AI review

This cache has no size limit. An addon that builds pattern strings at run time, such as match(tok, name + ' ('), pays the compile cost for every new string and keeps every compiled pattern for the rest of the run. I measured 200k distinct patterns: 1.03 s against 0.21 s before this PR, with 200,001 cache entries left at the end. functools.lru_cache(maxsize=...) on a compile_pattern(pattern) helper would put a limit on the memory. Building patterns at run time would still be slower than before, so the docstring should tell addon authors to use constant pattern strings.

[dmcppcheck]

AI review

This copies the entry of the module-level patterns dict when the pattern is compiled, and the result is then cached. If an addon adds an entry such as cppcheckdata.patterns['%foo%'] = ... after a pattern using %foo% was compiled, the cached pattern still treats %foo% as a literal. Before this PR the dict was looked up on every call. In my test match(tok, '%foo% =') stays False in the PR after %foo% is registered, while base returns True. Two possible fixes: clear _pattern_cache when patterns changes, or look up patterns[p] lazily inside the matcher.