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

[Lang Hames]

Thanks for the feedback Doug and Richard. Committed (with feedback
incorporated) in r145845 and r145849.

- Lang.

On Sat, Dec 3, 2011 at 12:40 PM, Douglas Gregor <dgregor at apple.com> wrote:

>
> 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/20111205/d77aa41d/attachment.html>