[PATCH] D20857: [clang-tidy] Add modernize-explicit-operator-bool check.

[Alexander Kornienko via cfe-commits]

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

Sorry for the delay. Feel free to ping me earlier.


================
Comment at: clang-tidy/modernize/ExplicitOperatorBoolCheck.cpp:38
@@ +37,3 @@
+  Finder->addMatcher(
+      cxxConversionDecl(returns(booleanType()), unless(isExplicit()))
+          .bind("operator-bool"),
----------------
Please merge these two matchers to avoid repeated work. Something along the lines of:

  cxxConversionDecl(unless(isExplicit()),
    anyOf(cxxConversionDecl(returns(booleanType())).bind("operator-bool"),
      cxxConversionDecl(returns(pointerType(pointee(isConstQualified(), voidType()))).bind("operator-void-pointer"))));

================
Comment at: clang-tidy/modernize/ExplicitOperatorBoolCheck.cpp:55
@@ +54,3 @@
+    diag(MatchedDecl->getLocation(),
+         "operator bool declaration is not explicit")
+        << FixItHint::CreateInsertion(MatchedDecl->getLocation(), "explicit ");
----------------
The message should say a bit more on why it is bad, e.g.: "implicit operator bool allows erroneous comparisons; consider marking operator bool 'explicit'". Or in a more prescriptive way: "operator bool should be marked 'explicit' to avoid erroneous comparisons". Feel free to suggest a better option.

================
Comment at: clang-tidy/modernize/ExplicitOperatorBoolCheck.cpp:64
@@ +63,3 @@
+
+    // FIXME: This tries to change the type and add explicit, but
+    // MatchedDecl->getTypeSpecStartLoc() gets the start of void, not the start
----------------
Looks like you'll need to re-lex the range of the declaration and find the location of tokens `const`, `void`, `*` between `operator` and `(`. See clang-tidy/modernize/UseOverrideCheck.cpp for an example.


http://reviews.llvm.org/D20857