fixed #12465 - added (undocumented) command-line option --executor to specify the used executor implementation

[firewave]

No description provided.

[firewave]

I wanted to wait with this until I am done with executor stuff but I need this in an upcoming unit test as there is stuff I cannot easily fix in the process implementation so I need to limit some tests to the thread one.

[firewave]

This still needs adjustments for the way we use the ThreadSanitizer in Python tests...

[firewave]

This still needs adjustments for the way we use the ThreadSanitizer in Python tests...

This hits on something I am thinking about for a long time - forcing the invocation with a different default. This affects the following:

• --executor
• --clang
• -j

In a perfect world running Cppcheck with either of those options added should not lead to any difference in the output.

So I guess the injection should be specific to those arguments. So if the argument is not specified in the argument list it should not be added. That seems test which explicitly depend on the default need to specify that on the command-line. --clang should not matter but tests which e.g. should be run with a single job need to specify -j1 and so on. We could also just prepend them and they would be overwritten by later one since we are processing them left-to-right but I would prefer not to have duplicated arguments.

That would allow us to have a much bigger test coverage in the Python tests without introducing additional cases. We might also be able to remove some (like all the *_j ones I added in #6042 and other PRs).

[danmar]

Do you envision that users will use this option? What are the valid use cases?

If your target is to use it for testing purposes.. imho we could let it be an unofficial option

[danmar]

chose => choose/use

[firewave]

Do you envision that users will use this option? What are the valid use cases?

I guess it is unlikely the regular user will use this.

The thread mode has the following advantages:

• it allows for (easier) debugging of the analysis with multiple jobs
• it is (currently) faster if you have lots of small files (because of the way the ProcessExecutor is implemented - will be improved by the executor reworking)
• it might also be faster if you have lots of data generated (also because of the way ProcessExecutor is implemented and because we need to transfer the data back to the main process)
• it has less bugs (also related to data needed to be transferred back to the main process - e.g. unmatchedSuppression not reported properly - I do not have the ticket number handy). Will file missing tickets later on.

Spawning additional processes might also be a security violation. But as this has not yet been brought up yet I guess it is actually a valid use case.

If your target is to use it for testing purposes.. imho we could let it be an unofficial option

I am fine with not documenting it.

[firewave]

• it has less bugs (also related to data needed to be transferred back to the main process - e.g. unmatchedSuppression not reported properly - I do not have the ticket number handy). Will file missing tickets later on.

That's also one of the reasons why the executor stuff still isn't finished. I keep finding issues and gaps in the testings and before I shared the code I want to have at least one of the job implementations working as expected/intended (i.e. the single job one) so I know that the reworking didn't introduce any new bugs.

[firewave]

This is also a per-requisite for #5647.

[danmar]

I am fine with not documenting it.

Alright I suggest we make it undocumented for now. If we see a need to make it official later we can do it. Feel free to remove the help text and then merge it.

[firewave]

Alright I suggest we make it undocumented for now. If we see a need to make it official later we can do it. Feel free to remove the help text and then merge it.

I removed it from the documentation.