On Wed, Nov 15, 2017 at 7:17 AM, Trevor Saunders <tbsau...@tbsaunde.org> 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?) 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 >