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: [PATCH 02/14] Support for adding and stripping location_t wrapper nodes


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


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