Investigation attempt for #10489: unmatchedSuppression missingInclude…

[gerboengels]

Cross post from the forum (https://sourceforge.net/p/cppcheck/discussion/general/thread/847b8527de/)

I'm looking a bit into https://trac.cppcheck.net/ticket/10489 and I think I found the issue and a very hacky solution, but I'm looking for input for the correct solution.

Problem: with this code

#include <string>

and calling it with

cppcheck.exe --suppress=missingIncludeSystem -j 2 --enable=information test.cpp

yields

Checking test.cpp ...
nofile:0:0: information: Unmatched suppression: missingIncludeSystem [unmatchedSuppression]

This is due to the -j 2 flag. If I compare it with a suppressed regular error, I notice that those go through ThreadExecutor::SyncLogForwarder, which somehow synchonizes messages betwee threads (?). missingInclude (and missingSystemInclude) do not go through SyncLogForwarder, and therefore its suppression-match-status gets lost when the thread ends.

I put together kind of a fix, but I think it is just a hack, and I have no idea if i really makes sense. Diff is added (based upon current main, 4ce76d0b). Basically, I added a virtual function to ErrorLogger so that in Preprocessor::missingInclude I can call that function so the suppression-state get propagated through SyncLogForwarder. Is this the right direction? What should be the implementations? Should all calls to mSettings.nomsg.isSuppressed(errorMessage) actually be calls to the new function?

[danmar]

Is this the right direction? What should be the implementations? Should all calls to mSettings.nomsg.isSuppressed(errorMessage) actually be calls to the new function?

I don't feel I remember all the suppressions details. There have been problems there. It will feel better with a test.

maybe. it seems reasonable to use the thread executor settings if there are threads. I don't understand why all other messages work but missingIncludeSystem doesn't.

[firewave]

There's tickets about it and I also have WIP tests/changes for this locally. Please give me a few days to give some insight on this.

[gerboengels]

There's tickets about it and I also have WIP tests/changes for this locally. Please give me a few days to give some insight on this.

Great! Then I'll wait for that. Again, my hacky fix works for my problem, but I feel it is not the right solution. And CI tells me there's a couple more projects that aren't in the .sln, but which are affected. No idea what to do with that.

[firewave]

Sorry for not attending to this yet.

There's another issue with missingInclude I want to look into first and need to write some tests for. That should probably get me a bit back on track.

[firewave]

I will hopefully take a look at this next week.

[firewave]

I have some cleanups and refactoring towards understanding/fixing this but at the core it boils down to the Settings being mutable all the time and reporting all the relevant changes back to the main process.

I think the settings need to be split into actual settings and "runtime data". That will be a of work though.

To fix the suppression issue only we probably "just" need to align the process and thread code in the way error reporting is done. After my cleanups that should hopefully be quite straight forward.

But I think there's still other underlying issues with the suppressions that need to be resolved afterwards.

[firewave]

I think I have figured it out - we "just" need to move some logic from the executor(s) to the overlying logger.

Some other PRs have to land first before I can get the cleanups in because they clash with each other. Afterwards I can do the fix.