This is an archive of the discontinued LLVM Phabricator instance.

Disable tidy checks with too many hits
ClosedPublic

Authored by ilya-biryukov on Feb 1 2019, 2:24 AM.

Details

Summary

Some tidy checks have too many hits in the codebase, making it hard to spot
other results from clang-tidy, therefore rendering the tool less useful.

Two checks were disabled:

  • misc-non-private-member-variable-in-classes in the whole LLVM monorepo, it is very common to have those in LLVM and the style guide does not forbid them.
  • readability-identifier-naming in the clang subtree. There are thousands of violations in 'Sema.h' alone.

Before the change, 'Sema.h' had >1000 tidy warnings, after the change the number
dropped to 3 warnings (unterminated namespace comments).

Diff Detail

Repository
rC Clang

Event Timeline

ilya-biryukov created this revision.Feb 1 2019, 2:24 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFeb 1 2019, 2:24 AM
  • Also disable 'misc-non-private-member-variables-in-classes' in llvm/.clang-tidy
hokein accepted this revision.Feb 1 2019, 2:57 AM

I'm +1 on disabling these checks as they add too much noise in editors (even though the naming check is correct, we can't do a global cleanup on the codebase). Let's commit it, we could revert it if someone has other concerns.

clang/.clang-tidy
2 ↗(On Diff #184704)

"naming checks" => readability-identifier-naming check. Maybe also mention these violations add too much noise in editors?

This revision is now accepted and ready to land.Feb 1 2019, 2:57 AM
ilya-biryukov marked an inline comment as done.
  • Address comments
This revision was automatically updated to reflect the committed changes.
alexfh added a comment.Feb 2 2019, 3:26 PM

I wonder whether a list of specific checks (without wildcards) would make more sense for llvm?