On Wed, Nov 15, 2017 at 7:17 AM, Trevor Saunders <[email protected]> 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
>
