murrayc updated this revision to Diff 59367.
murrayc added a comment.

With suggested changes. Ran clang-format (LLVM style). Used voidType() matcher.


http://reviews.llvm.org/D20857

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ExplicitOperatorBoolCheck.cpp
  clang-tidy/modernize/ExplicitOperatorBoolCheck.h
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/OperatorVoidPointerCheck.cpp
  clang-tidy/modernize/OperatorVoidPointerCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-explicit-operator-bool.rst
  docs/clang-tidy/checks/modernize-operator-void-pointer.rst
  test/clang-tidy/modernize-explicit-operator-bool.cpp
  test/clang-tidy/modernize-operator-void-pointer.cpp

Index: test/clang-tidy/modernize-operator-void-pointer.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-operator-void-pointer.cpp
@@ -0,0 +1,46 @@
+// RUN: %check_clang_tidy %s modernize-operator-void-pointer %t
+
+// This should trigger the check:
+class SomethingBad {
+  operator const void *() const {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: implicit operator void* declaration should probably be explicit operator bool [modernize-operator-void-pointer]
+    return reinterpret_cast<void *>(something != 0);
+  }
+
+  int something = 0;
+};
+
+class SomethingGood {
+  //Note: Use modernize-explicit-operator-bool to check for implicit operator bool.
+  explicit operator bool() const {
+    return something != 0;
+  }
+
+  int something = 0;
+};
+
+class SomethingGoodExplicitConstVoidPtr {
+  explicit operator const void *() const {
+    return &something;
+  }
+
+  const int something = 0;
+};
+
+class SomethingGoodExplicitNonConstVoidPtr {
+  explicit operator void *() {
+    return &something;
+  }
+
+  int something = 0;
+};
+
+class SomethingGoodNonConstVoidPtr {
+  // A non-const void* is unlikely to to be meant as operator bool before C++11
+  // let us use explicit.
+  operator void *() {
+    return &something;
+  }
+
+  int something = 0;
+};
Index: test/clang-tidy/modernize-explicit-operator-bool.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-explicit-operator-bool.cpp
@@ -0,0 +1,19 @@
+// RUN: %check_clang_tidy %s modernize-explicit-operator-bool %t
+
+// This should trigger the check:
+class SomethingBad {
+  operator bool() const {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator bool declaration is not explicit [modernize-explicit-operator-bool]
+    return something != 0;
+  }
+
+  int something = 0;
+};
+
+class SomethingGood {
+  explicit operator bool() const {
+    return something != 0;
+  }
+
+  int something = 0;
+};
Index: docs/clang-tidy/checks/modernize-operator-void-pointer.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/modernize-operator-void-pointer.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - modernize-operator-void-pointer
+
+modernize-operator-void-pointer
+================================
+
+This check finds implicit ``operator void*`` overloads and replaces them with
+``explicit operator bool`` overloads, available since C++11.
+
+Implicit ``operator void*`` overloads were often used before C++11 to avoid
+implicit conversions to ``bool`` when providing an ``operator bool`` overload,
+but C++11 provides the ``explicit`` keyword.
+
+See also the modernize-implicit-operator-bool check.
+
+.. code-block:: c++
+
+  operator void* () const;
+
+  // becomes
+
+  explicit operator bool () const;
Index: docs/clang-tidy/checks/modernize-explicit-operator-bool.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/modernize-explicit-operator-bool.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - modernize-explicit-operator-bool
+
+modernize-explicit-operator-bool
+================================
+
+This check finds implicit ``operator bool`` overloads and inserts the
+``explicit`` keyword, which is available since C++11.
+
+Without the ``explicit`` keyword, the implicit ``bool`` overload can allow
+objects to be compared accidentally. For instance, even when objects a and b
+have no ``operator ==`` overloads, an implicit ``operator bool`` would allow
+``a == b`` to compile because both a and b can be implictly converted to
+``bool``.
+
+.. code-block:: c++
+
+  operator bool () const;
+
+  // becomes
+
+  explicit operator bool () const;
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -93,6 +93,7 @@
    misc-virtual-near-miss
    modernize-avoid-bind
    modernize-deprecated-headers
+   modernize-explicit-operator-bool
    modernize-loop-convert
    modernize-make-shared
    modernize-make-unique
@@ -106,6 +107,7 @@
    modernize-use-default
    modernize-use-nullptr
    modernize-use-override
+   modernize-operator-void-pointer
    performance-faster-string-find
    performance-for-range-copy
    performance-implicit-cast-in-loop
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -190,11 +190,22 @@
 
   Replaces C standard library headers with their C++ alternatives.
 
+- New `modernize-explicit-operator-bool
+  <http://clang.llvm.org/extra/clang-tidy/checks/modernize-explicit-operator-bool.html>`_ check
+
+  Adds the ``explicit`` keyword to ``operator bool`` overloads.
+
 - New `modernize-make-shared
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-make-shared.html>`_ check
 
   Replaces creation of ``std::shared_ptr`` from new expression with call to ``std::make_shared``.
 
+- New `modernize-operator-void-pointer
+  <http://clang.llvm.org/extra/clang-tidy/checks/modernize-operator-void-pointer.html>`_ check
+
+  Finds ``operator const void*`` overloads. These should often be
+  ``explicit operator bool`` overloads.
+
 - New `modernize-raw-string-literal
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-raw-string-literal.html>`_ check
 
Index: clang-tidy/modernize/OperatorVoidPointerCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/modernize/OperatorVoidPointerCheck.h
@@ -0,0 +1,36 @@
+//===--- OperatorVoidPointerCheck.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_OPERATOR_VOID_POINTER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_OPERATOR_VOID_POINTER_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// This check finds implicit operator void* overloads and replaces them with
+/// explicit operator bool overloads, available since C++11.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-operator-void-pointer.html
+class OperatorVoidPointerCheck : public ClangTidyCheck {
+public:
+  OperatorVoidPointerCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_OPERATOR_VOID_POINTER_H
Index: clang-tidy/modernize/OperatorVoidPointerCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/modernize/OperatorVoidPointerCheck.cpp
@@ -0,0 +1,57 @@
+//===--- OperatorVoidPointerCheck.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 "OperatorVoidPointerCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+void OperatorVoidPointerCheck::registerMatchers(MatchFinder *Finder) {
+  // Use of the explicit keyword with operator overloads requires C++11 or
+  // later.
+  if (!getLangOpts().CPlusPlus11)
+    return;
+
+  Finder->addMatcher(cxxConversionDecl(returns(pointerType(pointee(
+                                           isConstQualified(), voidType()))),
+                                       unless(isExplicit()))
+                         .bind("operator-void-pointer"),
+                     this);
+}
+
+void OperatorVoidPointerCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl =
+      Result.Nodes.getNodeAs<CXXConversionDecl>("operator-void-pointer");
+
+  diag(MatchedDecl->getLocation(), "implicit operator void* declaration should "
+                                   "probably be explicit operator bool");
+
+  // FIXME: This tries to change the type and add explicit, but
+  // MatchedDecl->getTypeSpecStartLoc() gets the start of void, not the start
+  // of const, in const void*. We would need something like
+  // getConversionTypeStartLoc().
+  // const auto Type = MatchedDecl->getConversionType().
+  // const auto OldRange = CharSourceRange::getTokenRange(
+  //  MatchedDecl->getTypeSpecStartLoc(),
+  //  MatchedDecl->getTypeSpecStartLoc().getLocWithOffset(Type.getAsString().size()));
+  // diag(MatchedDecl->getLocation(),
+  //  "implicit operator void* declaration should probably be explicit operator
+  //  bool")
+  //  << FixItHint::CreateReplacement(OldRange, "bool")
+  //  << FixItHint::CreateInsertion(MatchedDecl->getLocStart(), "explicit ");
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/modernize/ModernizeTidyModule.cpp
===================================================================
--- clang-tidy/modernize/ModernizeTidyModule.cpp
+++ clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -12,9 +12,11 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "AvoidBindCheck.h"
 #include "DeprecatedHeadersCheck.h"
+#include "ExplicitOperatorBoolCheck.h"
 #include "LoopConvertCheck.h"
 #include "MakeSharedCheck.h"
 #include "MakeUniqueCheck.h"
+#include "OperatorVoidPointerCheck.h"
 #include "PassByValueCheck.h"
 #include "RawStringLiteralCheck.h"
 #include "RedundantVoidArgCheck.h"
@@ -39,9 +41,13 @@
         "modernize-avoid-bind");
     CheckFactories.registerCheck<DeprecatedHeadersCheck>(
         "modernize-deprecated-headers");
+    CheckFactories.registerCheck<ExplicitOperatorBoolCheck>(
+        "modernize-explicit-operator-bool");
     CheckFactories.registerCheck<LoopConvertCheck>("modernize-loop-convert");
     CheckFactories.registerCheck<MakeSharedCheck>("modernize-make-shared");
     CheckFactories.registerCheck<MakeUniqueCheck>("modernize-make-unique");
+    CheckFactories.registerCheck<OperatorVoidPointerCheck>(
+        "modernize-operator-void-pointer");
     CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value");
     CheckFactories.registerCheck<RawStringLiteralCheck>(
         "modernize-raw-string-literal");
Index: clang-tidy/modernize/ExplicitOperatorBoolCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/modernize/ExplicitOperatorBoolCheck.h
@@ -0,0 +1,36 @@
+//===--- ExplicitOperatorBoolCheck.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_EXPLICIT_OPERATOR_BOOL_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_EXPLICIT_OPERATOR_BOOL_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// This check finds implicit operator bool overloads and inserts the explicit
+/// keyword, which is available since C++11.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-explicit-operator-bool.html
+class ExplicitOperatorBoolCheck : public ClangTidyCheck {
+public:
+  ExplicitOperatorBoolCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_EXPLICIT_OPERATOR_BOOL_H
Index: clang-tidy/modernize/ExplicitOperatorBoolCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/modernize/ExplicitOperatorBoolCheck.cpp
@@ -0,0 +1,41 @@
+//===--- ExplicitOperatorBoolCheck.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 "ExplicitOperatorBoolCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+void ExplicitOperatorBoolCheck::registerMatchers(MatchFinder *Finder) {
+  // Use of the explicit keyword with operator overloads requires C++11 or
+  // later.
+  if (!getLangOpts().CPlusPlus11)
+    return;
+
+  Finder->addMatcher(
+      cxxConversionDecl(returns(booleanType()), unless(isExplicit()))
+          .bind("operator-bool"),
+      this);
+}
+
+void ExplicitOperatorBoolCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl =
+      Result.Nodes.getNodeAs<CXXConversionDecl>("operator-bool");
+  diag(MatchedDecl->getLocation(), "operator bool declaration is not explicit")
+      << FixItHint::CreateInsertion(MatchedDecl->getLocation(), "explicit ");
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/modernize/CMakeLists.txt
===================================================================
--- clang-tidy/modernize/CMakeLists.txt
+++ clang-tidy/modernize/CMakeLists.txt
@@ -3,12 +3,14 @@
 add_clang_library(clangTidyModernizeModule
   AvoidBindCheck.cpp
   DeprecatedHeadersCheck.cpp
+  ExplicitOperatorBoolCheck.cpp
   LoopConvertCheck.cpp
   LoopConvertUtils.cpp
   MakeSmartPtrCheck.cpp
   MakeSharedCheck.cpp
   MakeUniqueCheck.cpp
   ModernizeTidyModule.cpp
+  OperatorVoidPointerCheck.cpp
   PassByValueCheck.cpp
   RawStringLiteralCheck.cpp
   RedundantVoidArgCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to