Skip to content

enabled information messages in self-check #3090

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

firewave
Copy link
Collaborator

@firewave firewave commented Jan 28, 2021

The unmatchedSuppression messages might be a bug where the regular expression appears to be incorrectly validated. I saw this with local runs but didn't look into it yet. There was also different behavior between using no threads and -j. I will file tickets as soon as I have the time to look at it.

@chrchr-github
Copy link
Collaborator

So we have inline suppressions, but don't check if they are (still) matched? Ugh.

@firewave firewave changed the title enabled information messages in self-check enabled information messages in self-check Oct 11, 2023
@firewave firewave force-pushed the self-check-info branch 3 times, most recently from fcb246a to 68bfc72 Compare October 12, 2023 08:57
@firewave
Copy link
Collaborator Author

The "issue" we are having is that we are using a single suppression file for multiple different Cppcheck analysis invocations. This causes unmatchedSuppression warnings for unmatched global suppressions and suppressions for files which are not part of the analysis.

The first could be solved by using more granular suppressions. That might not be what you want. We possible need some way to filter out error IDs without suppressing them indicating that you just not care about them. Suppression should also be considered temporary hence the unmatchedSuppression warning.

The latter cannot be solved easily. We adjust the warning to only report them as unmatched if neither the regular expression nor the ID matched. That would require some re-working of the suppression code (which might happen as part of the still in progress executor rework) but might result in lots of outdated entries in the suppression file.

Another issue is that you also cannot control unmatchedSuppressionproperly. It is part of information - which it shouldn't IMO - see https://trac.cppcheck.net/ticket/10697. And if you enable that and you also suppress that you might still end up with an unmatchedSuppression. So maybe that should have a separate type in --enable/--disable.

@firewave
Copy link
Collaborator Author

As most (all) of the issues are with having a suppression file and those do not apply to inline suppression at all maybe we should introduce unmatchedInlineSuppression?

@chrchr-github
Copy link
Collaborator

As most (all) of the issues are with having a suppression file and those do not apply to inline suppression at all maybe we should introduce unmatchedInlineSuppression?

Sounds reasonable. Unmatched inline suppressions are more problematic anyway.

@firewave
Copy link
Collaborator Author

firewave commented Feb 24, 2025

*/moc_*.cpp:-1:0: information: Unmatched suppression: symbolDatabaseWarning [unmatchedSuppression]
^

I filed https://trac.cppcheck.net/ticket/13659 about the invalid location.

I also filed https://trac.cppcheck.net/ticket/13660 about such warnings being shown even if no files matching the wildcard were encountered.

@firewave
Copy link
Collaborator Author

firewave commented Feb 24, 2025

lib/filesettings.h:98:0: information: Unmatched suppression: unusedFunction [unmatchedSuppression]
    const std::string& sfilename() const
^
lib/utils.h:297:0: information: Unmatched suppression: unusedFunction [unmatchedSuppression]
std::size_t getArrayLength(const T (& /*unused*/)[size])
^
lib/errorlogger.h:198:0: information: Unmatched suppression: unusedFunction [unmatchedSuppression]
    const std::string &verboseMessage() const {
^

These are a bit a of problem.

The warnings are correct but we have a dedicated job in a different workflow which takes care of this and the check is not enabled in this case. So I have no idea how we can avoid these.

This also highlights that the unusedFunction check is not run in the selfchecks with the sanitizers. So we should enable it only in those and not fail the build in case any error is encountered in those. But in that case we would not see potential changes in behavior with the sanitizers enabled. As that is not an actual production use case it should be fine though.

Update: We could just hack these out by not reporting them if unusedFunction is not enabled. I think that logic already exists but might not be applied for inline suppressions.

@firewave
Copy link
Collaborator Author

firewave commented Feb 24, 2025

lib/token.h:1457:0: information: Unmatched suppression: premium-misra-cpp-2023-12.2.1 [unmatchedSuppression]
    bool mIsC : 1;
^

This will only trigger with the premium selfcheck so we should ignore these suppressions if it is not premium - which would be a horrible hack IMO.

Update: Well - I think we already have that "hack" in place for unusedFunction.

@firewave
Copy link
Collaborator Author

nofile:0:0: information: Unmatched suppression: autoNoType [unmatchedSuppression]

That does not occur in the external sources so we need to make that suppression conditional.

@firewave

This comment was marked as resolved.

@firewave
Copy link
Collaborator Author

I filed https://trac.cppcheck.net/ticket/13663 about not reporting unmatched premium-* suppressions in the regular version.

@firewave
Copy link
Collaborator Author

lib/tokenize.h:136:0: information: Unmatched suppression: functionConst [unmatchedSuppression]
    void arraySizeAfterValueFlow(); // cppcheck-suppress functionConst
^
lib/tokenize.h:168:0: information: Unmatched suppression: functionConst [unmatchedSuppression]
    void simplifyVarDecl(Token * tokBegin, const Token * tokEnd, bool only_k_r_fpar); // cppcheck-suppress functionConst // has side effects
^

This occurs when we can code which only includes that file. The warning should be reported on the implementation instead. I filed https://trac.cppcheck.net/ticket/13667 about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants