[PATCH] D137302: [clang-tidy] Add modernize-type-traits check

[Nathan James via Phabricator via cfe-commits]

njames93 marked an inline comment as done.
njames93 added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp:172
+void TypeTraitsCheck::registerMatchers(MatchFinder *Finder) {
+  static const ast_matchers::internal::VariadicDynCastAllOfMatcher<
+      Stmt,
----------------
carlosgalvezp wrote:
> `static` not needed here, remove. I don't think LLVM guidelines enforce strict const correctness either so the `const` could be removed as well if wanted.
const correctness isn't enforced, but it doesn't harm


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp:38
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use c++14 style type templates
+// CHECK-FIXES: std::enable_if_t<b>inTemplate();
+
----------------
carlosgalvezp wrote:
> njames93 wrote:
> > carlosgalvezp wrote:
> > > There should be a space in here, right? Same for the rest.
> > I'm assuming you mean between `<b>` and `inTemplate`.
> > If so, because we disable clang-format in the tests, the code isn't being reformatted and the replacement logic happens to get rid of the space too. As missing the space doesn't break the code and clang-format would reformat the code for us in typical use cases, its not worth worrying about.
> I disagree, just because the code "compiles" it doesn't mean it's good. The only time I've seen code like this is in [[ https://www.ioccc.org/ | obfuscated code competitions ]] :) 
> 
> We should not force the user to run clang-format to fix this - clang-tidy should preserve the existing space. This is not a matter of style either, it's a de-facto way of writting C++ (and many other languages): variable types and variable names in a declaration are separated by a space.
One of the original design decisions of clang-tidy was that check authors should not worry about formatting. This is partly because not everyone can agree on a style, but the main driving factor is that clang-tidy runs clang-format on the fixes that it generates unless you explicitly tell clang-tidy not to reformat your fixes.
Given that the set of real world projects that have a clang-tidy configuration file is nearly a perfect subset of the projects that have a clang-format configuration file, it's safe to assume that fixes would always be reformatted when running clang-tidy


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137302/new/

https://reviews.llvm.org/D137302