[cfe-commits] [PATCH] Warn on function-to-bool conversion.

[Douglas Gregor]

On Dec 1, 2011, at 7:22 PM, Lang Hames wrote:

> Oops - left my test case out of the previous patch.

A few comments:

+def warn_impcast_function_to_bool : Warning<
+    "in implicit conversion to bool, "
+    "the address of %q0 will always evaluate to 'true'">,
+    InGroup<BoolConversions>;

It's a little wordy. How about "address of function %q0 will always evaluate 'true'"?

+      if (D && !D->isWeakImported()) {
+        FunctionDecl* F = cast<FunctionDecl>(D);
+        S.Diag(E->getExprLoc(), diag::warn_impcast_function_to_bool)
+          << F << E->getSourceRange() << SourceRange(CC);
+        return;
+      }

There are other "weak" cases here. Check out the IsWeakDecl predicate in ExprConstant.cpp; I suggest making that available so you can re-use it here.

Otherwise, it looks good. I actually don't want us to diagnose

	if (&f)

because I expected that it's intentional more often than not (because, for example, "f" might be weak on some systems but not on others).

	- Doug

> - Lang.
> 
> On Thu, Dec 1, 2011 at 7:16 PM, Lang Hames <lhames at gmail.com> wrote:
> Hi Richard,
> 
> I've added source ranges to the diagnostic, however I'm wary of catching cases like
> 
> if (&foo) {}
> 
> That seems too deliberate. For now I'd prefer to stick to the simple case of missing parentheses.
> 
> What do you think?
> 
> Cheers,
> Lang.
> 
> On Mon, Nov 28, 2011 at 3:12 PM, Richard Trieu <rtrieu at google.com> wrote:
> On Mon, Nov 28, 2011 at 1:38 PM, Lang Hames <lhames at gmail.com> wrote:
> This patch adds a warning for implicit conversion from functions to booleans.
> 
> Could someone take a look and let me know if anything needs fixing or improving?
> 
> Cheers,
> Lang. 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 
> 
> Can this be changed to catch cases like this:
> 
> void foo();
> if (&foo) {}
> 
> Also, have you thought about formatting the diagnostic to use the DiagnoseImpCast() helper functions like the other implicit cast warnings?  If you don't use it, it would be nice to add E->getSourceRange() to underline the function name.
> 
> 
> 
> 
> <func-as-bool-warning-3.patch>_______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111203/20e4c510/attachment.html>