murrayc updated this revision to Diff 62355.
murrayc marked an inline comment as done.
murrayc added a comment.

Same as previous patch, but with a tiny suggested whitespace corretion.


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
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-explicit-operator-bool.rst
  test/clang-tidy/modernize-explicit-operator-bool-void-pointer.cpp
  test/clang-tidy/modernize-explicit-operator-bool.cpp

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 -- -- -std=c++11
+
+// 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: test/clang-tidy/modernize-explicit-operator-bool-void-pointer.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/modernize-explicit-operator-bool-void-pointer.cpp
@@ -0,0 +1,46 @@
+// RUN: %check_clang_tidy %s modernize-explicit-operator-bool %t -- -- -std=c++11
+
+// This should trigger the check:
+class SomethingBad {
+  operator const void *() const {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: implicit operator const void* declaration should probably be explicit operator bool [modernize-explicit-operator-bool]
+    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: docs/clang-tidy/checks/modernize-explicit-operator-bool.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/modernize-explicit-operator-bool.rst
@@ -0,0 +1,28 @@
+.. 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``.
+
+This check also finds implicit ``operator const void*`` overloads. These were
+often used before C++11 to avoid implicit conversions to ``bool`` when providing
+an ``operator bool`` overload.
+
+To disable the check for ``operator const void*`` overloads, you may set
+The :option:`WarnOnOperatorVoidPointer` option to 1.
+
+.. 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
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -204,6 +204,13 @@
 
   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.
+  Also finds ``operator const void*`` overloads, which should often be
+  ``explicit operator bool`` overloads.
+
 - New `modernize-make-shared
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-make-shared.html>`_ check
 
Index: clang-tidy/modernize/ModernizeTidyModule.cpp
===================================================================
--- clang-tidy/modernize/ModernizeTidyModule.cpp
+++ clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -12,6 +12,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "AvoidBindCheck.h"
 #include "DeprecatedHeadersCheck.h"
+#include "ExplicitOperatorBoolCheck.h"
 #include "LoopConvertCheck.h"
 #include "MakeSharedCheck.h"
 #include "MakeUniqueCheck.h"
@@ -41,6 +42,8 @@
         "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");
Index: clang-tidy/modernize/ExplicitOperatorBoolCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/modernize/ExplicitOperatorBoolCheck.h
@@ -0,0 +1,41 @@
+//===--- 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.
+/// It also finds implicit operator const void* overloads, which should often be
+/// explicit operator bool overloads.
+///
+/// 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);
+  void storeOptions(ClangTidyOptions::OptionMap &Opt) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const bool WarnOnOperatorVoidPointer;
+};
+
+} // 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,83 @@
+//===--- 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 {
+
+ExplicitOperatorBoolCheck::ExplicitOperatorBoolCheck(StringRef Name,
+                                                     ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      WarnOnOperatorVoidPointer(Options.get("WarnOnOperatorVoidPointer", 1) !=
+                                0) {}
+
+void ExplicitOperatorBoolCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "WarnOnOperatorVoidPointer", WarnOnOperatorVoidPointer);
+}
+
+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);
+
+  if (WarnOnOperatorVoidPointer) {
+    Finder->addMatcher(cxxConversionDecl(returns(pointerType(pointee(
+                                             isConstQualified(), voidType()))),
+                                         unless(isExplicit()))
+                           .bind("operator-void-pointer"),
+                       this);
+  }
+}
+
+void ExplicitOperatorBoolCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *MatchedDecl =
+          Result.Nodes.getNodeAs<CXXConversionDecl>("operator-bool")) {
+    diag(MatchedDecl->getLocation(),
+         "operator bool declaration is not explicit")
+        << FixItHint::CreateInsertion(MatchedDecl->getLocation(), "explicit ");
+  } else if (const auto *MatchedDecl =
+                 Result.Nodes.getNodeAs<CXXConversionDecl>(
+                     "operator-void-pointer")) {
+    diag(MatchedDecl->getLocation(),
+         "implicit operator const 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/CMakeLists.txt
===================================================================
--- clang-tidy/modernize/CMakeLists.txt
+++ clang-tidy/modernize/CMakeLists.txt
@@ -3,6 +3,7 @@
 add_clang_library(clangTidyModernizeModule
   AvoidBindCheck.cpp
   DeprecatedHeadersCheck.cpp
+  ExplicitOperatorBoolCheck.cpp
   LoopConvertCheck.cpp
   LoopConvertUtils.cpp
   MakeSmartPtrCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to