This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 02/14] Support for adding and stripping location_t wrapper nodes
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Trevor Saunders <tbsaunde at tbsaunde dot org>
- Cc: David Malcolm <dmalcolm at redhat dot com>, Jason Merrill <jason at redhat dot com>, Nathan Sidwell <nathan at acm dot org>, Jakub Jelinek <jakub at redhat dot com>, gcc-patches List <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 15 Nov 2017 12:11:27 +0100
- Subject: Re: [PATCH 02/14] Support for adding and stripping location_t wrapper nodes
- Authentication-results: sourceware.org; auth=none
- References: <CADzB+2kfB26fTGXk2AVWuN1mv641OrcEBze+Lq-KexXHeS=Y9A@mail.gmail.com> <1510350329-48956-1-git-send-email-dmalcolm@redhat.com> <1510350329-48956-3-git-send-email-dmalcolm@redhat.com> <20171115061704.q4jypxdhfiofk5o5@ball>
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
>