[v2 of PATCH 13/14] c-format.c: handle location wrappers

Jason Merrill jason@redhat.com
Tue Dec 19 19:55:00 GMT 2017


On 12/17/2017 11:29 AM, David Malcolm wrote:
> On Mon, 2017-12-11 at 18:45 -0500, Jason Merrill wrote:
>> On 11/10/2017 04:45 PM, David Malcolm wrote:
>>> +  STRIP_ANY_LOCATION_WRAPPER (format_tree);
>>> +
>>>       if (VAR_P (format_tree))
>>>         {
>>>           /* Pull out a constant value if the front end didn't.  */
>>
>> It seems like we want fold_for_warn here instead of the special variable
>> handling.  That probably makes sense for the other places you change in
>> this patch, too.
> 
> Here's an updated version of the patch which uses fold_for_warn,
> rather than STRIP_ANY_LOCATION_WRAPPER.
> 
> In one place it was necessary to add a STRIP_NOPS, since the
> fold_for_warn can add a cast around a:
>    ADDR_EXPR <POINTER_TYPE(ARRAY_TYPE(char))> (
>      STRING_CST)
> turning it into a:
>    NOP_EXPR<POINTER_TYPE(char))> (
>      ADDR_EXPR <POINTER_TYPE(ARRAY_TYPE(char))> (
>        STRING_CST))

Hmm, that seems like a bug.  fold_for_warn shouldn't change the type of 
the result.

> +  format_tree = fold_for_warn (format_tree);
> +
>     if (VAR_P (format_tree))
>       {
>         /* Pull out a constant value if the front end didn't.  */

I was suggesting dropping the if (VAR_P... block, since pulling out the 
constant value should now be covered by fold_for_warn.

> -    format_tree = TREE_OPERAND (format_tree, 0);
> +    {
> +      format_tree = fold_for_warn (TREE_OPERAND (format_tree, 0));
> +    }

Why the added braces?

Jason



More information about the Gcc-patches mailing list