From patchwork Tue Nov 18 16:30:12 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew MacLeod X-Patchwork-Id: 412086 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 6695714012A for ; Wed, 19 Nov 2014 03:31:29 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=KL7iGc4MZ4N8mCZCS S9tLICrUSOD6G/kmj9bfDiWaTYS0bYE4eVZLWi+dyYFF0fm9UnIExwGXmYNApb4V C8XHBqDQ/BaoN4iyDSOTOSuePULLfyXtSGXjOvc0n1jzsKVmEsRhtP3lxFJj1gcG 1XEbZsV5CmJV2Ft00GLn34oAVE= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=etT9j/QAiqakCnqZckRkd/w +jn0=; b=LTJJa9wki0eW60aPobVD6pYAd7P5bafUYFx2nhuCO64KWoAZX9Rvf7d XaDtgP5Its1y1ixc+57iLnyJDhU/EOlbRC+X7m+06CbyKc9SAeRi+SnL161G/Y7N +khTgF6FqXjfZnZLxivu7JNL0/kwsCwRX8H7D3CDs5PddJ6VEZGw= Received: (qmail 31037 invoked by alias); 18 Nov 2014 16:30:17 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 31006 invoked by uid 89); 18 Nov 2014 16:30:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 18 Nov 2014 16:30:15 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sAIGUEuQ028151 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 18 Nov 2014 11:30:14 -0500 Received: from [10.10.60.178] (vpn-60-178.rdu2.redhat.com [10.10.60.178]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sAIGUCYk012032; Tue, 18 Nov 2014 11:30:13 -0500 Message-ID: <546B7414.3070500@redhat.com> Date: Tue, 18 Nov 2014 11:30:12 -0500 From: Andrew MacLeod User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Jason Merrill , GCC , gcc-patches CC: Richard Biener Subject: Re: Query about the TREE_TYPE field References: <546B572F.7070103@redhat.com> <546B5A64.3080807@redhat.com> <546B5D2B.2000208@redhat.com> In-Reply-To: <546B5D2B.2000208@redhat.com> X-IsSubscribed: yes 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