This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Query about the TREE_TYPE field
- From: Andrew MacLeod <amacleod at redhat dot com>
- To: Jason Merrill <jason at redhat dot com>, GCC <gcc at gcc dot gnu dot org>, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Cc: Richard Biener <richard dot guenther at gmail dot com>
- Date: Tue, 18 Nov 2014 11:30:12 -0500
- Subject: Re: Query about the TREE_TYPE field
- Authentication-results: sourceware.org; auth=none
- References: <546B572F dot 7070103 at redhat dot com> <546B5A64 dot 3080807 at redhat dot com> <546B5D2B dot 2000208 at redhat dot com>
On 11/18/2014 09:52 AM, Andrew MacLeod wrote:
On 11/18/2014 09:40 AM, Jason Merrill wrote:
On 11/18/2014 09:26 AM, Andrew MacLeod wrote:
I was poking around attribs.c while trial running my tree-type-safety
stuff, and it triggered something in decl_attributes() that seems fishy
to me. It looks like it was part of the fix for
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=35315
decl_attributes() can be passed a tree node which is either decl or a
type, but we get to this little snippet:
if (spec->type_required && DECL_P (*anode))
{
anode = &TREE_TYPE (*anode);
/* Allow ATTR_FLAG_TYPE_IN_PLACE for the type's naming
decl. */
if (!(TREE_CODE (*anode) == TYPE_DECL
&& *anode == TYPE_NAME (TYPE_MAIN_VARIANT
(TREE_TYPE (*anode)))))
flags &= ~(int) ATTR_FLAG_TYPE_IN_PLACE;
}
anode is changed to point to the TREE_TYPE of the original decl, and
*then* checks if it is a TYPE_DECL... That doesnt seem right to me..
we can't have a TYPE_DECL as a TREE_TYPE can we?
No.
is that code suppose to be checking is the original DECL is a TYPE_DECL
rather than the TREE_TYPE?
I think so.
Maybe the assignment to anode should be after the if instead of in
front
of it?
Probably.
Strange that the 35315 patch fixed the testcase with that change
being a placebo...
Jason
Indeed. The condition is negated, so effectively it becomes an
always true condition and the ATTR_FLAG_TYPE_IN_PLACE is always turned
off when a DECL is passed and a type is required..
maybe that works most/all of the time? huh, that also means the code
is the same as it was before the patch :-P
maybe one of the follow up patches in bugzilla supercede it?
Andrew
I tried doing the if before chaning to TREE_TYPE... absolutely no
effect on the testsuite or anything else :-)
What do you think, should I check this in? What is there is clearly
incorrect. we could also revert the original patch since that is what
the code base is effectively is doing at the moment...
Andrew
* attribs.c (decl_attributes): Check for TYPE_DECL before switching to
using the TREE_TYPE.
Index: attribs.c
===================================================================
*** attribs.c (revision 217234)
--- attribs.c (working copy)
*************** decl_attributes (tree *node, tree attrib
*** 501,512 ****
the decl's type in place here. */
if (spec->type_required && DECL_P (*anode))
{
- anode = &TREE_TYPE (*anode);
/* Allow ATTR_FLAG_TYPE_IN_PLACE for the type's naming decl. */
if (!(TREE_CODE (*anode) == TYPE_DECL
&& *anode == TYPE_NAME (TYPE_MAIN_VARIANT
(TREE_TYPE (*anode)))))
flags &= ~(int) ATTR_FLAG_TYPE_IN_PLACE;
}
if (spec->function_type_required && TREE_CODE (*anode) != FUNCTION_TYPE
--- 501,512 ----
the decl's type in place here. */
if (spec->type_required && DECL_P (*anode))
{
/* Allow ATTR_FLAG_TYPE_IN_PLACE for the type's naming decl. */
if (!(TREE_CODE (*anode) == TYPE_DECL
&& *anode == TYPE_NAME (TYPE_MAIN_VARIANT
(TREE_TYPE (*anode)))))
flags &= ~(int) ATTR_FLAG_TYPE_IN_PLACE;
+ anode = &TREE_TYPE (*anode);
}
if (spec->function_type_required && TREE_CODE (*anode) != FUNCTION_TYPE