On Wed, 2017-11-15 at 12:11 +0100, Richard Biener wrote: > On Wed, Nov 15, 2017 at 7:17 AM, Trevor Saunders <tbsaunde@tbsaunde.o > rg> wrote: > > On Fri, Nov 10, 2017 at 04:45:17PM -0500, David Malcolm wrote: > > > This patch provides a mechanism in tree.c for adding a wrapper > > > node > > > for expressing a location_t, for those nodes for which > > > !CAN_HAVE_LOCATION_P, along with a new method of cp_expr. > > > > > > It's called in later patches in the kit via that new method. > > > > > > In this version of the patch, I use NON_LVALUE_EXPR for wrapping > > > constants, and VIEW_CONVERT_EXPR for other nodes. > > > > > > I also turned off wrapper nodes for EXCEPTIONAL_CLASS_P, for the > > > sake > > > of keeping the patch kit more minimal. > > > > > > The patch also adds a STRIP_ANY_LOCATION_WRAPPER macro for > > > stripping > > > such nodes, used later on in the patch kit. > > > > I happened to start reading this series near the end and was rather > > confused by this macro since it changes variables in a rather > > unhygienic > > way. Did you consider just defining a inline function to return > > the > > actual decl? It seems like its not used that often so the slight > > extra > > syntax should be that big a deal compared to the explicitness. > > Existing practice .... (STRIP_NOPS & friends). I'm fine either way, > the patch looks good. > > Eventually you can simplify things by doing less checking in > location_wrapper_p, like only checking > > +inline bool location_wrapper_p (const_tree exp) > +{ > + if ((TREE_CODE (exp) == NON_LVALUE_EXPR > + || (TREE_CODE (exp) == VIEW_CONVERT_EXPR > + && (TREE_TYPE (exp) > + == TREE_TYPE (TREE_OPERAND (exp, 0))) > + return true; > + return false; > +} > > and renaming to maybe_location_wrapper_p. After all you can't really > distinguish location wrappers from non-location wrappers? (and why > would you want to?)
That's the implementation I originally tried. As noted in an earlier thread about this, the problem I ran into was (in g++.dg/conversion/reinterpret1.C): // PR c++/15076 struct Y { Y(int &); }; int v; Y y1(reinterpret_cast<int>(v)); // { dg-error "" } where the "reinterpret_cast<int>" has the same type as the VAR_DECL v, and hence the argument to y1 is a NON_LVALUE_EXPR around a VAR_DECL, where both have the same type, and hence location_wrapper_p () on the cast would return true. Compare with: Y y1(v); where the argument "v" with a location wrapper is a VIEW_CONVERT_EXPR around a VAR_DECL. With the simpler conditions you suggested above, both are treated as location wrappers (leading to the dg-error in the test failing), whereas with the condition in the patch, only the latter is treated as a location wrapper, and an error is correctly emitted for the dg-error. Hope this sounds sane. Maybe the function needs a more detailed comment explaining this? Thanks Dave > Thanks, > Richard. > > > Other than that the series seems reasonable, and I look forward to > > having wrappers in more places. I seem to remember something I > > wanted > > to warn about they would make much easier. > > > > Thanks > > > > Trev > >