This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Tue, 2017-12-19 at 14:55 -0500, Jason Merrill wrote:
> 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.

In a similar vein, is the result of decl_constant_value (decl) required
to be the same type as that of the decl?

What's happening for this testcase (g++.dg/warn/Wformat-1.C) is that we
have a VAR_DECL with a DECL_INITIAL, but the types of the two don't
quite match up.

decl_constant_value returns an unshare_expr clone of the DECL_INITIAL,
and this is what's returned from fold_for_warn.

Am I right in thinking

(a) that the bug here is that a DECL_INITIAL ought to have the same
    type as its decl, and so there's a missing cast where that gets
    set up, or

(b) should decl_constant_value handle such cases, and introduce a cast
    if it uncovers them?

Thanks
Dave



> > +  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?

(I messed up; will be fixed in the next version)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]