xazax.hun updated the summary for this revision.
xazax.hun removed rL LLVM as the repository for this revision.
xazax.hun updated this revision to Diff 75530.
xazax.hun marked 3 inline comments as done.
xazax.hun added a comment.

- Fixed the performance problems.
- Altered the diagnostic text.
- Documentation improvements.
- Added an option to silence some warnings.
- Updated to latest trunk.


https://reviews.llvm.org/D24894

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseTransparentFunctorsCheck.cpp
  clang-tidy/modernize/UseTransparentFunctorsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-transparent-functors.rst
  test/clang-tidy/modernize-use-transparent-functors.cpp

Index: test/clang-tidy/modernize-use-transparent-functors.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-use-transparent-functors.cpp
@@ -0,0 +1,107 @@
+// RUN: %check_clang_tidy %s modernize-use-transparent-functors %t -- -- -std=c++14
+
+namespace std {
+template<class T>
+struct remove_reference;
+
+template <class T>
+constexpr T &&forward(typename std::remove_reference<T>::type &t);
+
+template <class T>
+constexpr T &&forward(typename std::remove_reference<T>::type &&t);
+
+template <typename T = void>
+struct plus {
+  constexpr T operator()(const T &Lhs, const T &Rhs) const;
+};
+
+template <>
+struct plus<void> {
+  template <typename T, typename U>
+  constexpr auto operator()(T &&Lhs, U &&Rhs) const -> 
+    decltype(forward<T>(Lhs) + forward<U>(Rhs));
+};
+
+template <typename T = void>
+struct less {
+  constexpr bool operator()(const T &Lhs, const T &Rhs) const;
+};
+
+template <>
+struct less<void> {
+  template <typename T, typename U>
+  constexpr bool operator()(T &&Lhs, U &&Rhs) const;
+};
+
+template <typename T = void>
+struct logical_not {
+  constexpr bool operator()(const T &Arg) const;
+};
+
+template <>
+struct logical_not<void> {
+  template <typename T>
+  constexpr bool operator()(T &&Arg) const;
+};
+
+template <typename T>
+class allocator;
+
+template <
+    class Key,
+    class Compare = std::less<>,
+    class Allocator = std::allocator<Key>>
+class set {};
+
+template <
+    class Key,
+    class Compare = std::less<Key>,
+    class Allocator = std::allocator<Key>>
+class set2 {};
+
+template <class InputIt, class UnaryPredicate>
+InputIt find_if(InputIt first, InputIt last,
+                UnaryPredicate p);
+
+template <class RandomIt, class Compare>
+void sort(RandomIt first, RandomIt last, Compare comp);
+
+class iterator {};
+class string {};
+}
+
+int main() {
+  using std::set;
+  using std::less;
+  std::set<int, std::less<int>> s;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: prefer transparent functors (aka diamond operators) [modernize-use-transparent-functors]
+  // CHECK-FIXES: {{^}}  std::set<int, std::less<>> s;{{$}}
+  set<int, std::less<int>> s2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: prefer transparent functors
+  // CHECK-FIXES: {{^}}  set<int, std::less<>> s2;{{$}}
+  set<int, less<int>> s3;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: prefer transparent functors
+  // CHECK-FIXES: {{^}}  set<int, less<>> s3;{{$}}
+  std::set<int, std::less<>> s4;
+  std::set<char *, std::less<std::string>> s5;
+  std::set<set<int, less<int>>, std::less<>> s6;
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: prefer transparent functors
+  // CHECK-FIXES: {{^}}  std::set<set<int, less<>>, std::less<>> s6;{{$}}
+  std::iterator begin, end;
+  sort(begin, end, std::less<int>());
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: prefer transparent functors
+  std::sort(begin, end, std::less<>());
+  find_if(begin, end, std::logical_not<bool>());
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: prefer transparent functors
+  std::find_if(begin, end, std::logical_not<>());
+  using my_set = std::set<int, std::less<int>>;
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: prefer transparent functors
+  // CHECK-FIXES: {{^}}  using my_set = std::set<int, std::less<>>;{{$}}
+  using my_set2 = std::set<char*, std::less<std::string>>;
+  using my_less = std::less<std::string>;
+  find_if(begin, end, my_less());
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: prefer transparent functors
+  std::set2<int> control;
+}
+
+
Index: docs/clang-tidy/checks/modernize-use-transparent-functors.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-transparent-functors.rst
@@ -0,0 +1,33 @@
+.. title:: clang-tidy - modernize-use-transparent-functors
+
+modernize-use-transparent-functors
+==================================
+
+Prefer transparent functors to non-transparent ones. When using transparent
+functors, the type does not need to be repeated. The code is easier to read,
+maintain and less prone to errors. It not possible to introduce unwanted
+conversions.
+
+  .. code-block:: c++
+
+    // Non-transparent functor  
+    std::map<int, std::string, std::greater<int>> s;
+
+    // Transparent functor.
+    std::map<int, std::string, std::greater<>> s;
+
+It is not always a safe transformation though. The following case will be 
+untouched to preserve the semantics.
+
+  .. code-block:: c++
+
+    // Non-transparent functor  
+    std::map<const char *, std::string, std::greater<std::string>> s;
+
+Options
+-------
+
+.. option:: SafeMode
+
+   If the option is set to non-zero (default is `0`), the check will not
+   warn on those cases where automatic FIXIT is not safe to apply.
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -111,6 +111,7 @@
    modernize-use-emplace
    modernize-use-nullptr
    modernize-use-override
+   modernize-use-transparent-functors
    modernize-use-using
    mpi-buffer-deref
    mpi-type-mismatch
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -81,6 +81,11 @@
   Warns if an object is used after it has been moved, without an intervening
   reinitialization.
 
+- New `modernize-use-transparent-functors
+  <http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-transparent-functors.html>`_ check
+
+  Replaces uses of non-transparent functors with transparent ones where applicable.
+
 - New `mpi-buffer-deref
   <http://clang.llvm.org/extra/clang-tidy/checks/mpi-buffer-deref.html>`_ check
 
Index: clang-tidy/modernize/UseTransparentFunctorsCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/modernize/UseTransparentFunctorsCheck.h
@@ -0,0 +1,37 @@
+//===--- UseTransparentFunctorsCheck.h - clang-tidy--------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_TRANSPARENT_FUNCTORS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_TRANSPARENT_FUNCTORS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// Prefer using transparent functors to non-transparent ones.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-transparent-functors.html
+class UseTransparentFunctorsCheck : public ClangTidyCheck {
+public:
+  UseTransparentFunctorsCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+private:
+  const bool SafeMode;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_TRANSPARENT_FUNCTORS_H
Index: clang-tidy/modernize/UseTransparentFunctorsCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/modernize/UseTransparentFunctorsCheck.cpp
@@ -0,0 +1,120 @@
+//===--- UseTransparentFunctorsCheck.cpp - clang-tidy----------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseTransparentFunctorsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+UseTransparentFunctorsCheck::UseTransparentFunctorsCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context), SafeMode(Options.get("SafeMode", 0)) {}
+
+void UseTransparentFunctorsCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SafeMode", SafeMode ? 1 : 0);
+}
+
+void UseTransparentFunctorsCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus14)
+    return;
+
+  const auto TransparentFunctors = classTemplateSpecializationDecl(
+      unless(hasAnyTemplateArgument(refersToType(voidType()))),
+      hasAnyName("::std::plus", "::std::minus", "::std::multiplies",
+                 "::std::divides", "::std::modulus", "::std::negate",
+                 "::std::equal_to", "::std::not_equal_to", "::std::greater",
+                 "::std::less", "::std::greater_equal", "::std::less_equal",
+                 "::std::logical_and", "::std::logical_or",
+                 "::std::logical_not", "::std::bit_and", "::std::bit_or",
+                 "::std::bit_xor", "::std::bit_not"));
+
+  // Non-transparent functor mentioned as a template parameter. FIXIT.
+  Finder->addMatcher(
+      loc(qualType(
+              unless(elaboratedType()),
+              hasDeclaration(classTemplateSpecializationDecl(
+                  unless(hasAnyTemplateArgument(templateArgument(refersToType(
+                      qualType(pointsTo(qualType(isAnyCharacter()))))))),
+                  hasAnyTemplateArgument(
+                      templateArgument(refersToType(qualType(hasDeclaration(
+                                           TransparentFunctors))))
+                          .bind("Functor"))))))
+          .bind("FunctorParentLoc"),
+      this);
+
+  if (SafeMode)
+    return;
+
+  // Non-transparent functor constructed. No FIXIT. There is no easy way
+  // to rule out the problematic char* vs string case.
+  Finder->addMatcher(cxxConstructExpr(hasDeclaration(cxxMethodDecl(
+                                          ofClass(TransparentFunctors))),
+                                      unless(isInTemplateInstantiation()))
+                         .bind("FuncInst"),
+                     this);
+}
+
+static const StringRef Message =
+    "prefer transparent functors (aka diamond operators)";
+
+void UseTransparentFunctorsCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  if (const auto *FuncInst =
+          Result.Nodes.getNodeAs<CXXConstructExpr>("FuncInst")) {
+    diag(FuncInst->getLocStart(), Message);
+    return;
+  }
+
+  const auto *Functor = Result.Nodes.getNodeAs<TemplateArgument>("Functor");
+  const auto FunctorParentLoc =
+      Result.Nodes.getNodeAs<TypeLoc>("FunctorParentLoc")
+          ->getAs<TemplateSpecializationTypeLoc>();
+
+  if (!FunctorParentLoc)
+    return;
+
+  unsigned ArgNum = 0;
+  for (; ArgNum < FunctorParentLoc.getNumArgs(); ++ArgNum) {
+    const TemplateArgument &Arg =
+        FunctorParentLoc.getArgLoc(ArgNum).getArgument();
+    if (Arg.getKind() != TemplateArgument::Type)
+      continue;
+    QualType ParentArgType = Arg.getAsType();
+    if (ParentArgType->isRecordType() &&
+        ParentArgType->getAsCXXRecordDecl() ==
+            Functor->getAsType()->getAsCXXRecordDecl())
+      break;
+  }
+  // Functor is a default template argument.
+  if (ArgNum == FunctorParentLoc.getNumArgs())
+    return;
+  TemplateArgumentLoc FunctorLoc = FunctorParentLoc.getArgLoc(ArgNum);
+  TemplateSpecializationTypeLoc FunctorTypeLoc;
+  TypeLoc TypeLocStep = FunctorLoc.getTypeSourceInfo()->getTypeLoc();
+  while (FunctorTypeLoc.isNull() && !TypeLocStep.isNull()) {
+    FunctorTypeLoc = TypeLocStep.getAs<TemplateSpecializationTypeLoc>();
+    TypeLocStep = TypeLocStep.getNextTypeLoc();
+  }
+  if (FunctorTypeLoc.isNull())
+    return;
+
+  SourceLocation ReportLoc = FunctorLoc.getLocation();
+  diag(ReportLoc, Message) << FixItHint::CreateRemoval(
+      FunctorTypeLoc.getArgLoc(0).getSourceRange());
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/modernize/ModernizeTidyModule.cpp
===================================================================
--- clang-tidy/modernize/ModernizeTidyModule.cpp
+++ clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -26,6 +26,7 @@
 #include "UseEmplaceCheck.h"
 #include "UseNullptrCheck.h"
 #include "UseOverrideCheck.h"
+#include "UseTransparentFunctorsCheck.h"
 #include "UseUsingCheck.h"
 
 using namespace clang::ast_matchers;
@@ -59,6 +60,8 @@
     CheckFactories.registerCheck<UseEmplaceCheck>("modernize-use-emplace");
     CheckFactories.registerCheck<UseNullptrCheck>("modernize-use-nullptr");
     CheckFactories.registerCheck<UseOverrideCheck>("modernize-use-override");
+    CheckFactories.registerCheck<UseTransparentFunctorsCheck>(
+        "modernize-use-transparent-functors");
     CheckFactories.registerCheck<UseUsingCheck>("modernize-use-using");
   }
 
Index: clang-tidy/modernize/CMakeLists.txt
===================================================================
--- clang-tidy/modernize/CMakeLists.txt
+++ clang-tidy/modernize/CMakeLists.txt
@@ -20,6 +20,7 @@
   UseEmplaceCheck.cpp
   UseNullptrCheck.cpp
   UseOverrideCheck.cpp
+  UseTransparentFunctorsCheck.cpp
   UseUsingCheck.cpp
 
   LINK_LIBS
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to