refs #10610 / fixed #13994 - compile `--rule-file` pattern only once / extracted regular expressions code to separate file #6211

firewave · 2024-03-30T14:25:48Z

No description provided.

firewave · 2024-03-30T14:28:22Z

This is mainly so the issues are reported on start-up instead on each file. It does not improve performance much.

The refactoring and improved testing will also make it easier to add support for PCRE2 since PCRE is EOL since quite a while (see https://trac.cppcheck.net/ticket/10610).

cli/cmdlineparser.cpp

+                        }
+
+                        rule.regex = std::make_shared<Regex>(rule.pattern);
+                        const std::string regex_err = rule.regex->compile();


lib/settings.h

@@ -293,6 +299,7 @@ class CPPCHECKLIB WARN_UNUSED Settings {
        std::string id = "rule"; // default id
        std::string summary;
        Severity severity = Severity::style; // default severity
+        std::shared_ptr<Regex> regex;


firewave · 2024-03-30T14:41:08Z

lib/regex.h

+    std::string mPattern;
+
+    class Data;
+    std::shared_ptr<Data> mData;


This needs to be a shared pointer because the it is used in Settings are they are being copied (the copies in production code will hopefully go away soon) and we have dynamic memory.

This means that multiple threads will use this but as the used data is const there should not be any issues. I did not see anything related to thread safety in the PCRE documentation. I will check again though. I hope any potential issues will be caught by the CI.

Thinking about that we a --rule test which actually has multiple input files. Wil add that.

firewave · 2024-03-30T18:11:01Z

So it seems the shared pointers do not seem to work with forked processes.

But there is something else:

[processfs_1.cpp :0]: (error) Internal error: Child process exited with 1

I wonder where the whitespace in the filename is coming from.

firewave · 2024-10-19T16:15:16Z

So it seems the shared pointers do not seem to work with forked processes.

The problem is that the memory behind the shared pointer is leaked in the forked process:

=================================================================
==8818==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1248 byte(s) in 13 object(s) allocated from:
    #0 0x560b0513391f in malloc (/home/runner/work/cppcheck/cppcheck/cmake.output/bin/testrunner+0x4a791f) (BuildId: 6af36c8d50c9763d4f05b83d43e79ba4537c85c0)
    #1 0x7f7c196469b9  (/lib/x86_64-linux-gnu/libpcre.so.3+0x559b9) (BuildId: 3982f316c887e3ad9598015fa5bae8557320476a)

Indirect leak of 1056 byte(s) in 4 object(s) allocated from:
    #0 0x560b0513391f in malloc (/home/runner/work/cppcheck/cppcheck/cmake.output/bin/testrunner+0x4a791f) (BuildId: 6af36c8d50c9763d4f05b83d43e79ba4537c85c0)
    #1 0x7f7c1961f2a3  (/lib/x86_64-linux-gnu/libpcre.so.3+0x2e2a3) (BuildId: 3982f316c887e3ad9598015fa5bae8557320476a)

SUMMARY: AddressSanitizer: 2304 byte(s) leaked in 17 allocation(s).

CMake 4.0 removed support for projects targeting versions prior to 3.5.

firewave · 2025-07-06T20:15:38Z

I think we should just allow the leak for now so we can get this refactoring merged and can proceed with this.

…ns code to separate file

danmar · 2025-07-08T12:30:35Z

I would really like to drop --rule-file completely. It's an inferior format to --addon. However it seems some users hates python completely and refuse to deal with it.
I would rather drop the PCRE dependency completely. Why not use std::regexp. I know that --rule-file will be slower then but this feature has been deprecated for 10-15 years. Also if you have 100 files in the project then the rule regexp is compiled 100 times or so, regexp compilation is not THAT slow.. I assume that cppcheck parser will take way longer time than the regexp..

firewave · 2025-07-09T09:35:58Z

So let's deprecate the rule (build) options without a target in the next version and switch the code to use std::regex (I have that prepared locally) so we can drop the PCRE dependency with the release after. I think it is fair for a deprecated feature to deteriorate in that way.

I would really like to drop --rule-file completely. It's an inferior format to --addon. However it seems some users hates python completely and refuse to deal with it.

We could re-write rules as addons internally and thus no longer need the actual code. But addons are magnitudes slower because of the process invocation and that would be even worse than using std::regex. Needs some performance data generated.

I would rather drop the PCRE dependency completely. Why not use std::regexp. I know that --rule-file will be slower then but this feature has been deprecated for 10-15 years. Also if you have 100 files in the project then the rule regexp is compiled 100 times or so, regexp compilation is not THAT slow.. I assume that cppcheck parser will take way longer time than the regexp..

It is only compiled once. It is the matching which is performed one the analysis.

firewave · 2025-07-09T09:51:51Z

Also if it is deprecated, this PR should be safe to be merged for the current release.

danmar · 2025-07-11T10:51:10Z

It is only compiled once.

That is what this PR is about isn't it? Otherwise the PR title is misleading. How much time does PR save if I have 100 files? Some rough test is fine.

firewave commented Mar 30, 2024

View reviewed changes

cli/cmdlineparser.cpp Outdated

}

rule.regex = std::make_shared<Regex>(rule.pattern);

const std::string regex_err = rule.regex->compile();

This comment was marked as outdated.

Sign in to view

firewave commented Mar 30, 2024

View reviewed changes

lib/settings.h

@@ -293,6 +299,7 @@ class CPPCHECKLIB WARN_UNUSED Settings {

std::string id = "rule"; // default id

std::string summary;

Severity severity = Severity::style; // default severity

std::shared_ptr<Regex> regex;

This comment was marked as outdated.

Sign in to view

firewave commented Mar 30, 2024

View reviewed changes

firewave marked this pull request as draft March 30, 2024 14:41

firewave force-pushed the regex branch from a3a0d2e to 5a7e7b2 Compare March 30, 2024 14:41

This comment was marked as outdated.

Sign in to view

firewave force-pushed the regex branch from 5a7e7b2 to 08ef4d3 Compare April 11, 2024 08:28

firewave changed the title ~~compile --rule-file pattern only once / extracted regular expressions code to separate file / improved errorhandling of --rule and --rule-file=~~ compile --rule-file pattern only once / extracted regular expressions code to separate file Apr 11, 2024

firewave mentioned this pull request Apr 22, 2024

refs #10692 - added command-line option --cpp-header-probe to probe headers and extension-less files for Emacs C++ marker #6324

Merged

firewave force-pushed the regex branch from 08ef4d3 to 2cb05b3 Compare May 29, 2024 08:46

firewave mentioned this pull request Jun 11, 2024

Added rule to simplify token matching #6472

Closed

firewave force-pushed the regex branch 3 times, most recently from 1fb855b to b300dc5 Compare July 2, 2024 07:44

firewave force-pushed the regex branch 3 times, most recently from e6c61a6 to 2b3ebf5 Compare July 16, 2024 09:01

firewave force-pushed the regex branch 2 times, most recently from 1b2c244 to 2ed6ba0 Compare October 19, 2024 12:21

firewave marked this pull request as ready for review October 19, 2024 12:22

firewave marked this pull request as draft October 19, 2024 12:23

firewave force-pushed the regex branch from 2ed6ba0 to dfb2ae7 Compare October 19, 2024 12:34

firewave marked this pull request as ready for review October 19, 2024 12:46

firewave marked this pull request as draft October 19, 2024 16:55

firewave force-pushed the regex branch from dfb2ae7 to 6bb5084 Compare February 27, 2025 11:40

firewave referenced this pull request Apr 2, 2025

fixed building outdated (pre-3.5) CMake projects in CI (#7419)

43bbfa2

CMake 4.0 removed support for projects targeting versions prior to 3.5.

firewave mentioned this pull request Jul 6, 2025

Fix #12821 (GUI: exclude folders in imported project) #7645

Open

firewave force-pushed the regex branch 3 times, most recently from 0818aa8 to 31fbac0 Compare July 6, 2025 20:15

firewave force-pushed the regex branch 2 times, most recently from 0937a56 to 085f2f5 Compare July 6, 2025 21:05

firewave marked this pull request as ready for review July 6, 2025 21:06

firewave force-pushed the regex branch 4 times, most recently from 1b28fd1 to e8d3bc1 Compare July 6, 2025 22:28

firewave changed the title ~~compile --rule-file pattern only once / extracted regular expressions code to separate file~~ refs #10610 - compile --rule-file pattern only once / extracted regular expressions code to separate file Jul 7, 2025

firewave force-pushed the regex branch from e8d3bc1 to 8a99cd4 Compare July 7, 2025 09:08

firewave added 2 commits July 7, 2025 13:37

compile --rule-file pattern only once / extracted regular expressio…

508e940

…ns code to separate file

Regex: hide implementation details

b099926

firewave force-pushed the regex branch from 8a99cd4 to 43f8071 Compare July 7, 2025 11:39

firewave changed the title ~~refs #10610 - compile --rule-file pattern only once / extracted regular expressions code to separate file~~ refs #10610 / fixed #13994 - compile --rule-file pattern only once / extracted regular expressions code to separate file Jul 7, 2025

ignore leak from PCRE for now

c7ce1d3

firewave force-pushed the regex branch from 43f8071 to c7ce1d3 Compare July 7, 2025 12:53

firewave added the merge-after-next-release Wait with merging this PR until after the next Release label Jul 7, 2025

firewave removed the merge-after-next-release Wait with merging this PR until after the next Release label Jul 9, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refs #10610 / fixed #13994 - compile `--rule-file` pattern only once / extracted regular expressions code to separate file #6211

refs #10610 / fixed #13994 - compile `--rule-file` pattern only once / extracted regular expressions code to separate file #6211

Uh oh!

firewave commented Mar 30, 2024

Uh oh!

firewave commented Mar 30, 2024

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

firewave Mar 30, 2024

Uh oh!

firewave commented Mar 30, 2024

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

firewave commented Oct 19, 2024

Uh oh!

firewave commented Jul 6, 2025

Uh oh!

danmar commented Jul 8, 2025

Uh oh!

firewave commented Jul 9, 2025

Uh oh!

firewave commented Jul 9, 2025

Uh oh!

danmar commented Jul 11, 2025

Uh oh!

Uh oh!

refs #10610 / fixed #13994 - compile --rule-file pattern only once / extracted regular expressions code to separate file #6211

Are you sure you want to change the base?

refs #10610 / fixed #13994 - compile --rule-file pattern only once / extracted regular expressions code to separate file #6211

Uh oh!

Conversation

firewave commented Mar 30, 2024

Uh oh!

firewave commented Mar 30, 2024

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

firewave Mar 30, 2024

Choose a reason for hiding this comment

Uh oh!

firewave commented Mar 30, 2024

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

firewave commented Oct 19, 2024

Uh oh!

firewave commented Jul 6, 2025

Uh oh!

danmar commented Jul 8, 2025

Uh oh!

firewave commented Jul 9, 2025

Uh oh!

firewave commented Jul 9, 2025

Uh oh!

danmar commented Jul 11, 2025

Uh oh!

Uh oh!

refs #10610 / fixed #13994 - compile `--rule-file` pattern only once / extracted regular expressions code to separate file #6211

refs #10610 / fixed #13994 - compile `--rule-file` pattern only once / extracted regular expressions code to separate file #6211