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 <[email protected]> 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 <[email protected]> wrote:
> On Mon, Nov 28, 2011 at 1:38 PM, Lang Hames <[email protected]> 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
> [email protected]
> 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
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
