fixed #6366 - some unique errors were omitted

[firewave]

The code which checks for unique error messages did not apply the template configuration which lead to suppression of unique messages:

(information) Cppcheck cannot find all the include files (use --check-config for details)
nofile:0:0: information: Cppcheck cannot find all the include files (use --check-config for details) [missingInclude]

(information) Cppcheck cannot find all the include files (use --check-config for details)
nofile:0:0: information: Cppcheck cannot find all the include files (use --check-config for details) [missingIncludeSystem]

The latter was omitted since the messages are identical.

There is possibly a different approach which would be including the ID if no template is specified. Or use the same format as when no template is specified and added the ID.

[danmar]

it seems redundant to say "Cppcheck cannot find all the include files" twice.. so does this fix that?

[firewave]

it seems redundant to say "Cppcheck cannot find all the include files" twice.. so does this fix that?

No it does enable that. But it is two different IDs. I wanted to change the message to say include and system include so they are actually unique so they can be distinguished when not using IDs at all.

It's also just an intermediate fix since these warnings will go away completely with #3229.

[firewave]

Still this would cause different errors with identical messages though not be to be printed. Or it might lead actual duplicated messages being shown depending on your template - e.g. you could just output the file, line and column and would get multiple ones.

As people can write their own addons and all the message are passed through here it might even be intentional to use the same message with varying IDs though.

I wonder if the filtering should be configurable since suppression non-unique messages seems like a workaround for some kind of bug.

I am aware though that the main reason for this is to prevent duplicated messages from headers.

[firewave]

A different example highlighting the issue:

#include <vector>

void func(std::vector<char>&& v)
{
    (void)&v[0];
}

void f1()
{
    func(std::vector<char>());
}
void f2()
{
    func(std::vector<char>());
}

scratch_3.cpp:5:13: error: Out of bounds access in expression 'v[0]' because 'v' is empty. [containerOutOfBounds]
    (void)&v[0];
            ^
scratch_3.cpp:14:27: note: Calling function 'func', 1st argument 'std::vector<char>()' value is size=0
    func(std::vector<char>());
                          ^
scratch_3.cpp:5:13: note: Access out of bounds
    (void)&v[0];
            ^

IMO this should be showing two warnings because the callstack is different. The second one won't be visible until the first one is fixed. And if the fix needs to be done on the caller side and the code is big this might become increasing annoying especially when you rely on something like a CI run to provide these findings.

In this case it might be misleading though if the output does not include the callstack.

[firewave]

See https://trac.cppcheck.net/ticket/6366 for a related issue.

[firewave]

These changes make sure that messages that are unique when they will be shown to the user are not being omitted. But it should not be dependent on the settings at all. Something to address later on.

The internal default format which is not used in production cannot be dropped easily because everything in the unit tests relies on it.

[firewave]

This is obviously a bug.

[firewave]

I filed https://trac.cppcheck.net/ticket/13671 about it. It was also the remaining duplicated message already visible in our unit tests.

[firewave]

This test was relying on the bugged behavior. Needs to be resolved differently.

[firewave]

These changes make sure that messages that are unique when they will be shown to the user are not being omitted. But it should not be dependent on the settings at all. Something to address later on.

The template format is only used when we print the results as text. So it does not make sense at all to use it to determine which messages are unique and we need an proper unique identifier. But that is something for later.

[firewave]

Another case where the same error might on the same line but in a different column might occur when using macros or compact/uglified code. Probably should add such test cases as well.

[danmar]

I understand