Skip to content
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

enabled and fixed performance-enum-size clang-tidy warnings #6221

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

firewave
Copy link
Collaborator

@firewave firewave commented Apr 2, 2024

No description provided.

@firewave
Copy link
Collaborator Author

firewave commented Apr 2, 2024

This totally throws off several premium-only checks...

@firewave
Copy link
Collaborator Author

firewave commented Apr 2, 2024

CC @danmar

@firewave
Copy link
Collaborator Author

firewave commented Apr 2, 2024

Nevermind - it only occurs in the regular version.

@firewave firewave marked this pull request as draft April 2, 2024 20:36
@firewave
Copy link
Collaborator Author

firewave commented Apr 2, 2024

I am not able to reproduce these issues locally. I assume it is related to class : std::uint8_t which leads to it no longer being treated as an enum but as a class with inheritance.

@chrchr-github
Copy link
Collaborator

Here's a reproducer:

struct S {
    enum class E : std::uint8_t {
        E0
    };
    void f(S::E e) {
        if (e == S::E::E0) {}
    }
    char a[20];
};

@firewave
Copy link
Collaborator Author

firewave commented Apr 2, 2024

Here's a reproducer:

Thanks. That's very much the example I tried. I can reproduce it now on a different system. Maybe there is a Windows/Linux difference. Will check tomorrow.

I filed https://trac.cppcheck.net/ticket/12564 for it.

@firewave
Copy link
Collaborator Author

firewave commented Apr 3, 2024

In the regular selfcheck we only have passedByValue false positives. The additional ones are just with the premium version.

But there is also the following:

cli/processexecutor.cpp:72:1: debug: Scope::checkVariable found variable 'namespace' with varid 0. [varid0]
namespace {
^
lib/mathlib.h:39:5: debug: SymbolDatabase couldn't resolve all user defined types. [debug]
    class value {
    ^

And with the unusedFunction selfcheck:

cli/processexecutor.cpp:77:0: style: The function 'PipeWriter' is never used. [unusedFunction]
        explicit PipeWriter(int pipe) : mWpipe(pipe) {}
^

@firewave
Copy link
Collaborator Author

firewave commented Apr 4, 2024

@danmar After this passes the selfcheck we need an updated premium version which includes all the related fixes so it can pass the CI.

@chrchr-github
Copy link
Collaborator

chrchr-github commented Apr 4, 2024

Reduced FP: https://trac.cppcheck.net/ticket/12588

@firewave
Copy link
Collaborator Author

firewave commented Apr 4, 2024

All fixed, thanks. Now we just need an updated premium version.

@firewave
Copy link
Collaborator Author

firewave commented Apr 5, 2024

All fixed, thanks. Now we just need an updated premium version.

The related fixes are:
2b88ca8
3170c17
4503fbf

@firewave
Copy link
Collaborator Author

firewave commented May 23, 2024

@danmar We need a 2.14-based premium in the CI for this to land.

@firewave
Copy link
Collaborator Author

The premium CI job is currently disabled because it started to interfere with other changes so this can be merged.

@firewave firewave marked this pull request as ready for review June 14, 2024 15:29
@chrchr-github chrchr-github merged commit 1be5af7 into danmar:main Jun 14, 2024
63 checks passed
@firewave firewave deleted the tidy-enum-size branch June 14, 2024 20:28
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