On Tue, 2017-10-24 at 09:53 -0400, Jason Merrill wrote:
> On Fri, Oct 20, 2017 at 5:53 PM, David Malcolm <dmalc...@redhat.com>
> wrote:
> > Design questions:
> > 
> > * The patch introduces a new kind of tree node, currently called
> >   DECL_WRAPPER_EXPR (although it's used for wrapping constants as
> > well
> >   as decls).  Should wrappers be a new kind of tree node, or should
> > they
> >   reuse an existing TREE_CODE? (e.g. NOP_EXPR, CONVERT_EXPR, etc).
> >     * NOP_EXPR: seems to be for use as an rvalue
> >     * CONVERT_EXPR: for type conversions
> >     * NON_LVALUE_EXPR: "Value is same as argument, but guaranteed
> > not an
> >       lvalue"
> >       * but we *do* want to support lvalues here
> 
> I think using NON_LVALUE_EXPR for constants would be appropriate.
> 
> >     * VIEW_CONVERT_EXPR: viewing one thing as of a different type
> >       * can it support lvalues?
> 
> Yes, the purpose of VIEW_CONVERT_EXPR is to support lvalues, it seems
> like the right choice.
> 
> Jason

Thanks.  I've been working on a new version of the patch using those
tree codes, but have run into an issue.

In g++.dg/conversion/reinterpret1.C:

  // PR c++/15076

  struct Y { Y(int &); };

  int v;
  Y y1(reinterpret_cast<int>(v));  // { dg-error "" }

With trunk, this successfully generates an error:

  reinterpret1.C:6:6: error: cannot bind non-const lvalue reference of type 
‘int&’ to an rvalue of type ‘int’
   Y y1(reinterpret_cast<int>(v));  // { dg-error "" }
        ^~~~~~~~~~~~~~~~~~~~~~~~
  reinterpret1.C:3:12: note:   initializing argument 1 of ‘Y::Y(int&)’
   struct Y { Y(int &); };
              ^

where internally there's a NON_LVALUE_EXPR around a VAR_DECL, where
both have the same type:

(gdb) call debug_tree (expr)
 <non_lvalue_expr 0x7ffff145f6e0
    type <integer_type 0x7ffff132e5e8 int public type_6 SI
        size <integer_cst 0x7ffff1331120 constant 32>
        unit-size <integer_cst 0x7ffff1331138 constant 4>
        align:32 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 
0x7ffff132e5e8 precision:32 min <integer_cst 0x7ffff13310d8 -2147483648> max 
<integer_cst 0x7ffff13310f0 2147483647>
        pointer_to_this <pointer_type 0x7ffff1336a80> reference_to_this 
<reference_type 0x7ffff144ca80>>
   
    arg:0 <var_decl 0x7ffff7ffbd80 v type <integer_type 0x7ffff132e5e8 int>
        used public static tree_1 read SI 
/home/david/coding-3/gcc-git-expr-vs-decl/src/gcc/testsuite/g++.dg/conversion/reinterpret1.C:5:5
 size <integer_cst 0x7ffff1331120 32> unit-size <integer_cst 0x7ffff1331138 4>
        align:32 warn_if_not_align:0 context <translation_unit_decl 
0x7ffff131e168 
/home/david/coding-3/gcc-git-expr-vs-decl/src/gcc/testsuite/g++.dg/conversion/reinterpret1.C>
        chain <type_decl 0x7ffff141a720 Y type <record_type 0x7ffff144c150 Y>
            public decl_2 VOID 
/home/david/coding-3/gcc-git-expr-vs-decl/src/gcc/testsuite/g++.dg/conversion/reinterpret1.C:3:8
            align:8 warn_if_not_align:0 context <translation_unit_decl 
0x7ffff131e168 
/home/david/coding-3/gcc-git-expr-vs-decl/src/gcc/testsuite/g++.dg/conversion/reinterpret1.C>
 chain <function_decl 0x7ffff144f800 __cxa_call_unexpected>>>
    
/home/david/coding-3/gcc-git-expr-vs-decl/src/gcc/testsuite/g++.dg/conversion/reinterpret1.C:6:6
 start: 
/home/david/coding-3/gcc-git-expr-vs-decl/src/gcc/testsuite/g++.dg/conversion/reinterpret1.C:6:6
 finish: 
/home/david/coding-3/gcc-git-expr-vs-decl/src/gcc/testsuite/g++.dg/conversion/reinterpret1.C:6:29>

The problem is that this reinterpret cast "looks" just like one of my
location wrappers.

I see a similar issue with constants, where with:

  struct Y { Y(int &); };
  Y y1(reinterpret_cast<int>(42));

trunk generates an error like the above, but my code handles the
  NON_LVALUE_EXPR<int>(INTEGER_CST<int>(42))
as if it were a location wrapper around the INTEGER_CST, and thus
doesn't emit the error.

I can envisage a few hackish solutions, like assuming that there will
be a location wrapper around such a constant, and thus to expect:
  NON_LVALUE_EXPR<int>(NON_LVALUE_EXPR<int>(INTEGER_CST<int>(42)))
but this seems clunky and fragile; maybe we do need a new tree code for
this (e.g. LOCATION_WRAPPER or somesuch?)

Thoughts?

Thanks
Dave

Reply via email to