[clang-tidy] Add a checker for converting into C++17 variable template type traits

[royjacobson]

Summary

This adds a clang-tidy checker that can transform C++11-style type traits into C++17-style
variable templates. Variable templates can compile faster because they can be implemented
without instantiating classes.

I'm not very familiar with clang matchers; There's probably a better way to do what I
want than regex + hacky type checking, so suggestions are very welcome.

[njames93]

Why limit this to just the value type traits, makes sense to also support type type traits.

clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp

74

Won't this assert if a pointer is supplied but it isn't a ClassTemplateSpecializationDefl. Wouldn't dyn_cast be needed.
In any case a test where you access a static variable from a non template class needs adding to make sure no warning is emitter.

83

Make the diagnostic a bit clearer.

87

This fixit logic looks cumbersome, just replace the qualifier loc(::)to the declared with an _v. But only iff the qualified name loc is not a macro.

clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.h

35

We don't need an ordering here, so an llvm::DenseSet<StringRef> may be more suited here.

clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst

29

I'm not sure this option would ever be used in the wild as it would override all the default values in the stl.

[royjacobson]

In D135404#3842156, @njames93 wrote:

Why limit this to just the value type traits, makes sense to also support type type traits.

I'd love to, but I can't match against the ::type usages with DeclRefExpr. I think I'd have to match all types in the program or something like that, but I'm not sure if even that's enough because of unevaluated contexts etc... So I decided to focus on this first.

clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp

74

Definitely need a dyn_cast here, thanks!

87

I think to do that I have to get the location of the template-id of the qualifier from the ClassTemplateSpecializationDecl, but I haven't really managed to do that. Or is there maybe a way to get the lexer tokens list of a source range or something like that?

clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst

29

The intended use case is one-time running of this check over a code base to modernize it, and you want to add your own type traits that you define across your own libraries. I think that's useful, at least.

Maybe this should just _add_ traits, and not replace them? Is this something that clang-tidy does? I basically copied this from the modernize-use-emplace check.

[royjacobson]

Addrees CR comments

[royjacobson]

friendly ping @njames93
FWIW I ran this over the LLVM codebase and have seen no crashes or false positives.

[njames93]

I'd also suggest using the IgnoreUnlessSpelledInSource traversal kind here. Given that most of the time these will be instantiated we don't want to warn and emit a fix for each instantiation, Or you could just add unless(isInTemplateInstantiation()) to your matcher.

clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp

87

Take a look here https://godbolt.org/z/Wj5bjbqGh
You'd want to create an insertion of _v at the start of the <
getQualifierLoc().getTypeLoc().getAs<clang::TemplateSpecializationTypeLoc>().getLAngleLoc()
Then a removal from the start of the :: until the end of the whole expr
{getQualifierLoc().getEndLoc(), getEndLoc()}

clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst

34

Newline

[royjacobson]

Address comments.

[royjacobson]

In D135404#3899904, @njames93 wrote:

I'd also suggest using the IgnoreUnlessSpelledInSource traversal kind here. Given that most of the time these will be instantiated we don't want to warn and emit a fix for each instantiation, Or you could just add unless(isInTemplateInstantiation()) to your matcher.

I'm not sure what to do here... Until this is instantiated, I have only a DependentScopeDeclRefExpr that is not resolved yet. And it's important to have this check for dependent contexts because that's how it's usually used.

clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp

87

That's great debugging tool, thanks! :)

[royjacobson]

Closing in favor of the much more mature D137302.

clang-tools-extra/clang-tidy/modernize/CMakeLists.txt

add_clang_library(clangTidyModernizeModule
   PassByValueCheck.cpp
   RawStringLiteralCheck.cpp
   RedundantVoidArgCheck.cpp
   ReplaceAutoPtrCheck.cpp
   ReplaceDisallowCopyAndAssignMacroCheck.cpp
   ReplaceRandomShuffleCheck.cpp
   ReturnBracedInitListCheck.cpp
   ShrinkToFitCheck.cpp
+  TypeTraitsCheck.cpp
   UnaryStaticAssertCheck.cpp
   UseAutoCheck.cpp
   UseBoolLiteralsCheck.cpp
   UseDefaultMemberInitCheck.cpp
   UseEmplaceCheck.cpp
   UseEqualsDefaultCheck.cpp
   UseEqualsDeleteCheck.cpp
   UseNodiscardCheck.cpp

clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp

 #include "PassByValueCheck.h"
 #include "RawStringLiteralCheck.h"
 #include "RedundantVoidArgCheck.h"
 #include "ReplaceAutoPtrCheck.h"
 #include "ReplaceDisallowCopyAndAssignMacroCheck.h"
 #include "ReplaceRandomShuffleCheck.h"
 #include "ReturnBracedInitListCheck.h"
 #include "ShrinkToFitCheck.h"
+#include "TypeTraitsCheck.h"
 #include "UnaryStaticAssertCheck.h"
 #include "UseAutoCheck.h"
 #include "UseBoolLiteralsCheck.h"
 #include "UseDefaultMemberInitCheck.h"
 #include "UseEmplaceCheck.h"
 #include "UseEqualsDefaultCheck.h"
 #include "UseEqualsDeleteCheck.h"
 #include "UseNodiscardCheck.h"
CheckFactories.registerCheck<ReplaceAutoPtrCheck>(
         "modernize-replace-auto-ptr");
     CheckFactories.registerCheck<ReplaceDisallowCopyAndAssignMacroCheck>(
         "modernize-replace-disallow-copy-and-assign-macro");
     CheckFactories.registerCheck<ReplaceRandomShuffleCheck>(
         "modernize-replace-random-shuffle");
     CheckFactories.registerCheck<ReturnBracedInitListCheck>(
         "modernize-return-braced-init-list");
     CheckFactories.registerCheck<ShrinkToFitCheck>("modernize-shrink-to-fit");
+    CheckFactories.registerCheck<TypeTraitsCheck>("modernize-type-traits");
     CheckFactories.registerCheck<UnaryStaticAssertCheck>(
         "modernize-unary-static-assert");
     CheckFactories.registerCheck<UseAutoCheck>("modernize-use-auto");
     CheckFactories.registerCheck<UseBoolLiteralsCheck>(
         "modernize-use-bool-literals");
     CheckFactories.registerCheck<UseDefaultMemberInitCheck>(
         "modernize-use-default-member-init");
     CheckFactories.registerCheck<UseEmplaceCheck>("modernize-use-emplace");

clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.h

+//===--- TypeTraitsCheck.h - clang-tidy -------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_TYPETRAITSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_TYPETRAITSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Convert type traits from the form `trait<...>::value` into `trait_v<...>`
+/// The template variable traits from C++17 can improve compile times, as in
+/// modern library implementations they don't instantiate structs.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/type-traits.html
+class TypeTraitsCheck : public ClangTidyCheck {
+public:
+  TypeTraitsCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus17;
+  }
+
+private:
+  llvm::DenseSet<StringRef> ValueTypeTraits;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_TYPETRAITSCHECK_H

clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp

+//===--- TypeTraitsCheck.cpp - clang-tidy ---------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "TypeTraitsCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/FixIt.h"
+#include "llvm/Support/Regex.h"
+#include <iostream>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+const auto DefaultValueTypeTraits =
+    "std::alignment_of;std::conjunction;std::disjunction;std::extent;std::has_"
+    "unique_object_representations;std::has_virtual_destructor;std::is_"
+    "abstract;std::is_aggregate;std::is_arithmetic;std::is_array;std::is_"
+    "assignable;std::is_base_of;std::is_bounded_array;std::is_class;std::is_"
+    "compound;std::is_const;std::is_constructible;std::is_convertible;std::is_"
+    "copy_assignable;std::is_copy_constructible;std::is_default_constructible;"
+    "std::is_destructible;std::is_empty;std::is_enum;std::is_final;std::is_"
+    "floating_point;std::is_function;std::is_fundamental;std::is_integral;std::"
+    "is_invocable;std::is_invocable_r;std::is_layout_compatible;std::is_lvalue_"
+    "reference;std::is_member_function_pointer;std::is_member_object_pointer;"
+    "std::is_member_pointer;std::is_move_assignable;std::is_move_constructible;"
+    "std::is_nothrow_assignable;std::is_nothrow_constructible;std::is_nothrow_"
+    "convertible;std::is_nothrow_copy_assignable;std::is_nothrow_copy_"
+    "constructible;std::is_nothrow_default_constructible;std::is_nothrow_"
+    "destructible;std::is_nothrow_invocable;std::is_nothrow_invocable_r;std::"
+    "is_nothrow_move_assignable;std::is_nothrow_move_constructible;std::is_"
+    "nothrow_swappable;std::is_nothrow_swappable_with;std::is_null_pointer;std:"
+    ":is_object;std::is_pointer;std::is_pointer_interconvertible_base_of;std::"
+    "is_polymorphic;std::is_reference;std::is_rvalue_reference;std::is_same;"
+    "std::is_scalar;std::is_scoped_enum;std::is_signed;std::is_standard_layout;"
+    "std::is_swappable;std::is_swappable_with;std::is_trivial;std::is_"
+    "trivially_assignable;std::is_trivially_constructible;std::is_trivially_"
+    "copy_assignable;std::is_trivially_copy_constructible;std::is_trivially_"
+    "copyable;std::is_trivially_default_constructible;std::is_trivially_"
+    "destructible;std::is_trivially_move_assignable;std::is_trivially_move_"
+    "constructible;std::is_unbounded_array;std::is_union;std::is_unsigned;std::"
+    "is_void;std::is_volatile;std::negation;std::rank;std::reference_"
+    "constructs_from_temporary;std::reference_converts_from_temporary";
+
+TypeTraitsCheck::TypeTraitsCheck(StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context) {
+  auto VectorValueTypeTraits = utils::options::parseStringList(
+      Options.get("ValueTypeTraits", DefaultValueTypeTraits));
+  ValueTypeTraits = llvm::DenseSet<StringRef>(VectorValueTypeTraits.begin(),
+                                              VectorValueTypeTraits.end());
+}
+
+void TypeTraitsCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      declRefExpr(to(varDecl(hasName("value")))).bind("memberExpr"), this);
+}
+
+void TypeTraitsCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<DeclRefExpr>("memberExpr");
+  if (MatchedDecl) {
+    const auto *Qualifier = MatchedDecl->getQualifier();
+    if (!Qualifier)
+      return;
+    const auto *RecordQualifier =
+        dyn_cast_if_present<ClassTemplateSpecializationDecl>(
+            Qualifier->getAsRecordDecl());
+    if (!RecordQualifier)
+      return;
+    auto TraitName = RecordQualifier->getQualifiedNameAsString();
+    if (ValueTypeTraits.find(TraitName) == ValueTypeTraits.end())
+      return;
+
+    SourceLocation ValueLoc = MatchedDecl->getSourceRange().getEnd();
+    auto IssuedDiag = diag(ValueLoc, "use c++17 style variable templates");
+
+    if (ValueLoc.isMacroID())
+      return;
+
+    IssuedDiag << FixItHint::CreateInsertion(
+                      MatchedDecl->getQualifierLoc()
+                          .getTypeLoc()
+                          .getAs<clang::TemplateSpecializationTypeLoc>()
+                          .getLAngleLoc(),
+                      "_v")
+               << FixItHint::CreateRemoval(
+                      {MatchedDecl->getQualifierLoc().getEndLoc(),
+                       MatchedDecl->getNameInfo().getSourceRange().getEnd()});
+  }
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`cppcoreguidelines-avoid-const-or-ref-data-members
 
   Warns when a struct or class uses const or reference (lvalue or rvalue) data members.
 
 - New :doc:`cppcoreguidelines-avoid-do-while
   <clang-tidy/checks/cppcoreguidelines/avoid-do-while>` check.
 
   Warns when using ``do-while`` loops.
 
+- New :doc:`modernize-type-traits
+  <clang-tidy/checks/modernize/type-traits>` check.
+
+  Finds usages of C++11 style type traits like ``std::is_same<...>::value`` and replaces
+  them with the C++17 variable template ``std::is_same_v<...>``. This transformation can
+  help reduce compile times.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
 - New alias :doc:`cert-msc54-cpp
   <clang-tidy/checks/cert/msc54-cpp>` to
   :doc:`bugprone-signal-handler
   <clang-tidy/checks/bugprone/signal-handler>` was added.
 

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `modernize-pass-by-value <modernize/pass-by-value.html>`_, "Yes"
    `modernize-raw-string-literal <modernize/raw-string-literal.html>`_, "Yes"
    `modernize-redundant-void-arg <modernize/redundant-void-arg.html>`_, "Yes"
    `modernize-replace-auto-ptr <modernize/replace-auto-ptr.html>`_, "Yes"
    `modernize-replace-disallow-copy-and-assign-macro <modernize/replace-disallow-copy-and-assign-macro.html>`_, "Yes"
    `modernize-replace-random-shuffle <modernize/replace-random-shuffle.html>`_, "Yes"
    `modernize-return-braced-init-list <modernize/return-braced-init-list.html>`_, "Yes"
    `modernize-shrink-to-fit <modernize/shrink-to-fit.html>`_, "Yes"
+   `modernize-type-traits <modernize/type-traits.html>`_, "Yes"
    `modernize-unary-static-assert <modernize/unary-static-assert.html>`_, "Yes"
    `modernize-use-auto <modernize/use-auto.html>`_, "Yes"
    `modernize-use-bool-literals <modernize/use-bool-literals.html>`_, "Yes"
    `modernize-use-default-member-init <modernize/use-default-member-init.html>`_, "Yes"
    `modernize-use-emplace <modernize/use-emplace.html>`_, "Yes"
    `modernize-use-equals-default <modernize/use-equals-default.html>`_, "Yes"
    `modernize-use-equals-delete <modernize/use-equals-delete.html>`_, "Yes"
    `modernize-use-nodiscard <modernize/use-nodiscard.html>`_, "Yes"

clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst

+.. title:: clang-tidy - modernize-type-traits
+
+modernize-type-traits
+=====================
+
+
+Convert type traits from the form ``trait<...>::value`` into ``trait_v<...>``
+The template variable traits from C++17 can improve compile times, as in
+modern library implementations they don't instantiate structs.
+
+For example, this:
+
+.. code-block:: c++
+
+  std::is_same<int, float>::value
+  std::is_integral<float>::value
+
+is replaced by:
+
+.. code-block:: c++
+
+  std::is_same_v<int, float>
+  std::is_integral_v<float>
+
+Options
+-------
+
+.. option:: ValueTypeTraits
+
+    Specifies a list of type traits which are used with a static ``::value``
+    member for metaprogramming instead of the regular one of traits available
+    in the STL. This check assumes that a matching version exists with the
+    same name and a ``_v`` suffix.

clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp

+// RUN: %check_clang_tidy -std=c++17-or-later %s modernize-type-traits %t
+
+namespace std {
+
+template <class T, T x>
+struct integral_constant {
+    static constexpr inline T value = x;
+};
+
+template <class U, class V>
+struct is_same : integral_constant<bool, false> {};
+
+template <class T>
+struct is_trivially_default_constructible : integral_constant<bool, false> {};
+
+template <class T>
+struct alignment_of : integral_constant<int, 3> {};
+
+}
+
+
+template <class T>
+void f() {
+    auto x = std::is_same<int, T>::value;
+    auto y = std::is_trivially_default_constructible<T>::value;
+    auto z = std::alignment_of<T>::value;
+
+// CHECK-FIXES:      auto x = std::is_same_v<int, T>;
+// CHECK-FIXES-NEXT: auto y = std::is_trivially_default_constructible_v<T>;
+// CHECK-FIXES-NEXT: auto z = std::alignment_of_v<T>;
+// Test that we don't get this twice or something weird like that because
+// we're in a dependant context.
+// CHECK-FIXES-NOT:  auto x = std::is_same_v<int, T>;
+}
+
+void g() {
+    f<int>();
+    f<float>();
+}
+
+void h() {
+    auto x = std::is_same<int, float>::value;
+    auto y = std::is_trivially_default_constructible<float>::value;
+    auto z = std::alignment_of<float>::value;
+// CHECK-FIXES:      auto x = std::is_same_v<int, float>;
+// CHECK-FIXES-NEXT: auto y = std::is_trivially_default_constructible_v<float>;
+// CHECK-FIXES-NEXT: auto z = std::alignment_of_v<float>;
+}
+
+
+template <class T>
+struct NonDiagnosableTemplate : std::integral_constant<bool, false> {};
+
+struct NonDiagnosable1 : std::integral_constant<bool, false> {};
+struct NonDiagnosable2 {
+    using value = int;
+};
+
+#define IN_A_MACRO std::is_same<int, int>::value
+void macro_use() {
+    bool a = IN_A_MACRO;
+}
+// Don't fix-it if we use it inside a macro.
+// CHECK-FIXES: #define IN_A_MACRO std::is_same<int, int>::value
+
+void test_no_diagnostic() {
+    auto x = NonDiagnosableTemplate<int>::value;
+    auto y = NonDiagnosable1::value;
+    auto z = NonDiagnosable2::value(4);
+// CHECK-FIXES:      auto x = NonDiagnosableTemplate<int>::value;
+// CHECK-FIXES-NEXT: auto y = NonDiagnosable1::value;
+// CHECK-FIXES-NEXT: auto z = NonDiagnosable2::value(4);
+}