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

[clang-tidy][cfg] Remove an alias from the extreme profile #3618

Merged
merged 1 commit into from
Mar 7, 2022

Conversation

steakhal
Copy link
Contributor

@steakhal steakhal commented Mar 4, 2022

It seems like that bugprone-narrowing-conversions check is the same as
the cppcoreguidelines-narrowing-conversions check.

Check this:
https://github.com/llvm/llvm-project/blob/c63522e6ba7782c335043893ae7cbd37eca24fe5/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp#L136-L137

It seems like both of these checks were included in the extreme
profile, thus each of these reports were duplicated basically.

Hereby I'm proposing to keep only one of them in the profile to
workaround this issue.

According to Compiler Explorer, clang-tidy reports these issues only
once, with the [check,alias,alias...] schema, thus it does the right
thing. However, PR #3238 introduced a logic that splits these reports
into individual reports.

I'm not sure if that is the right way of dealing with check aliases, but
we definitely need something more robust than including/excluding checks
from profiles.

@steakhal steakhal requested a review from csordasmarton as a code owner March 4, 2022 16:04
@steakhal
Copy link
Contributor Author

steakhal commented Mar 4, 2022

@dkrupp

Copy link
Contributor

@csordasmarton csordasmarton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

It seems like that `bugprone-narrowing-conversions` check is the same as
the `cppcoreguidelines-narrowing-conversions` check.

Check this:
https://github.com/llvm/llvm-project/blob/c63522e6ba7782c335043893ae7cbd37eca24fe5/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp#L136-L137

It seems like both of these checks were included in the `extreme`
profile, thus each of these reports were duplicated basically.

Hereby I'm proposing to keep only one of them in the profile to
workaround this issue.

According to Compiler Explorer https://godbolt.org/z/hvMnqrfEx,
clang-tidy reports these issues only once, with the
`[check,alias,alias...]` schema, thus it does the right thing.
However, PR Ericsson#3238 introduced a logic that splits these reports
into individual reports.

I'm not sure if that is the right way of dealing with check aliases, but
we definitely need something more robust than including/excluding checks
from profiles.
@dkrupp dkrupp merged commit b030dac into Ericsson:master Mar 7, 2022
@steakhal steakhal deleted the fix-check-alias branch March 7, 2022 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants