Quuxplusone added inline comments.

================
Comment at: lib/Sema/SemaChecking.cpp:2623
+                QualType QT = Pointer->getType()->getPointeeType();
+                if (!QT.isNull() && QT->isBooleanType())
+                  // Warn on bool* to bool conversion.
----------------
Quuxplusone wrote:
> efriedma wrote:
> > Have you considered skipping the isBooleanType() check?  Passing any 
> > pointer to a function which expects a boolean parameter seems fundamentally 
> > suspicious.
> We already know that LLVM itself passes pointers to functions in many places, 
> and they're not bugs. I mean, this isn't the first time someone's proposed 
> that pointer->bool conversions are evil. :)
> 
> I know this just came up on a mailing list somewhere recently (rsmith was 
> involved in the rebuttal, as was I), but all I can find on Google is:
> https://groups.google.com/a/isocpp.org/d/msg/std-proposals/a3g9qXFVGCs/Wj40qj48H3wJ
> Anyway, the punch line I'm thinking of is something like
> ```
>     FooBar *FB = ...;
>     LLVMAssert(FB, "expected fb to be set");
> ```
> but with whatever the real names are. Found a bunch of those when I wrote the 
> diagnostic. And they're fairly solidly in the "not bugs" category.
> The thing about the bool*-only version is that bool pointers are rare in C++, 
> so I'm not sure we're gaining much. But if we can't do something more 
> general, there's still some benefit.
> I see your point about false positives for the more general version. I was 
> sort of considering an attribute to allow implicit conversions to bool for a 
> specific function (like an assertion), and then we could ban implicit 
> conversions to bool for parameters to other functions. But that's probably 
> overkill.

Based on @sberg's feedback that this warning found no true positives, and 
@Eugene.Zelenko's feedback that this warning already exists in clang-tidy 
today, personally I favor the resolution of "leave this warning in clang-tidy 
and abandon the attempt to move it into clang".


https://reviews.llvm.org/D45601



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to