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

Turn off two ClangTidy checkers #675

Merged
merged 1 commit into from
Jun 27, 2017
Merged

Conversation

bruntib
Copy link
Contributor

@bruntib bruntib commented Jun 26, 2017

These two checkers give too many false-positives

These two checkers give too many false-positives
@bruntib bruntib requested review from Xazax-hun and gyorb June 26, 2017 14:00
@Xazax-hun
Copy link
Contributor

I do not see why should those give too many false positives. Could you report the bugs to the clang tidy devs? I'm ok with turning them off by default in the meantime.

@bruntib
Copy link
Contributor Author

bruntib commented Jun 26, 2017

Actually I ran these on an opensource project and most of the findings were:

  • According to the checker the thrown object should be constructed in place as a temporary object instead of constructing a named one and throw that. Most of the findings threw an enum constant.
  • The other checker reported problems related to the known signed/unsigned - int/long int conversions. Of course these can be problems but in practice the findings were not bugs since the values were not close to INT_MAX where an error may occur.

@gyorb
Copy link
Contributor

gyorb commented Jun 26, 2017

I think the in the newest clang release (v4.0) there were some updates to these checker which caused to report more results.
We do not want flood the users with too much results. Even if we remove them from the default enabled list the users can re-enable them in the command line.

I think these type of changes should go into the release notes so the users are informed.

@gyorb gyorb merged commit 0434131 into Ericsson:master Jun 27, 2017
@bruntib bruntib deleted the turn_out_checkers branch June 27, 2017 15:41
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.

3 participants