murrayc added inline comments.
Comment at: clang-tidy/modernize/ExplicitOperatorBoolCheck.cpp:38
@@ +37,3 @@
+ Finder->addMatcher(
+ cxxConversionDecl(returns(booleanType()), unless(isExplicit()))
+ .bind("operator-bool"),
alexfh wrote:
> Please mer
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(
+ cxxCon
murrayc added a comment.
I'd still be perfectly happy if just the simple check for implicit operator
bool was accepted.
http://reviews.llvm.org/D20857
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/lis
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/ExplicitOper
murrayc added a comment.
In http://reviews.llvm.org/D20857#449080, @alexfh wrote:
> Looks like a useful check to have. I'm not sure though, that it has anything
> to do with "modernize". I'd suggest adding a new "bugprone" module (should be
> added by http://reviews.llvm.org/D18821, hopefully s
alexfh added a comment.
Looks like a useful check to have. I'm not sure though, that it has anything to
do with "modernize". I'd suggest adding a new "bugprone" module (should be
added by http://reviews.llvm.org/D18821, hopefully soon) and moving the check
there.
Comment at:
murrayc updated this revision to Diff 59537.
murrayc added a comment.
Combined into one check. Also specifies C++11 for the test.
http://reviews.llvm.org/D20857
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ExplicitOperatorBoolCheck.cpp
clang-tidy/modernize/ExplicitOpera
Eugene.Zelenko added a comment.
I think single check will be better from user's point of view.
Comment at: docs/clang-tidy/checks/modernize-explicit-operator-bool.rst:12
@@ +11,3 @@
+have no ``operator ==`` overloads, an implicit ``operator bool`` would allow
+``a == b`` to comp
etienneb added inline comments.
Comment at: test/clang-tidy/modernize-operator-void-pointer.cpp:39
@@ +38,3 @@
+class SomethingGoodNonConstVoidPtr {
+ // A non-const void* is unlikely to to be meant as operator bool before C++11
+ // let us use explicit.
You kno
aaron.ballman added a comment.
In http://reviews.llvm.org/D20857#446784, @murrayc wrote:
> In http://reviews.llvm.org/D20857#446732, @etienneb wrote:
>
> > Enabling/disabling can be done with options (see SizeofExpressionCheck).
>
>
> Thanks. Am I being asked to combine the checks?
I would pref
murrayc added inline comments.
Comment at: clang-tidy/modernize/OperatorVoidPointerCheck.cpp:27
@@ +26,3 @@
+ Finder->addMatcher(cxxConversionDecl(returns(pointerType(pointee(
+ isConstQualified(), voidType(,
+
etienneb added a comment.
In http://reviews.llvm.org/D20857#446784, @murrayc wrote:
> In http://reviews.llvm.org/D20857#446732, @etienneb wrote:
>
> > Enabling/disabling can be done with options (see SizeofExpressionCheck).
>
>
> Thanks. Am I being asked to combine the checks?
I'll let alexfh@
murrayc added a comment.
In http://reviews.llvm.org/D20857#446732, @etienneb wrote:
> Enabling/disabling can be done with options (see SizeofExpressionCheck).
Thanks. Am I being asked to combine the checks?
http://reviews.llvm.org/D20857
___
cfe-
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/mod
etienneb added a comment.
In http://reviews.llvm.org/D20857#446654, @murrayc wrote:
> In http://reviews.llvm.org/D20857#446101, @etienneb wrote:
>
> > I wonder if these two checks should not be merge in one checker.
>
>
> Personally I find it cleaner to keep them separate, but I would be happy to
murrayc added a comment.
In http://reviews.llvm.org/D20857#446101, @etienneb wrote:
> I wonder if these two checks should not be merge in one checker.
Personally I find it cleaner to keep them separate, but I would be happy to
combine them if that's wanted. I guessed that it would be easier to
murrayc updated this revision to Diff 59346.
murrayc added a comment.
These are the commits amended with the suggested changes.
Many thanks for the suggestions.
(I'm not quite sure what arcanist will do, so hopefully this will not create a
mess.)
http://reviews.llvm.org/D20857
Files:
clang-
etienneb added a comment.
I wonder if these two checks should not be merge in one checker.
modernize-explicit-conversion-operator
Comment at: clang-tidy/modernize/ExplicitOperatorBoolCheck.cpp:21
@@ +20,3 @@
+void ExplicitOperatorBoolCheck::registerMatchers(MatchFinder *Finde
18 matches
Mail list logo
