From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 92749 invoked by alias); 13 Dec 2018 18:59:43 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 92736 invoked by uid 89); 13 Dec 2018 18:59:42 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=as, so, So, oversight 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 ESMTP; Thu, 13 Dec 2018 18:59:40 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9E4F2C049587; Thu, 13 Dec 2018 18:59:39 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-24.rdu2.redhat.com [10.10.112.24]) by smtp.corp.redhat.com (Postfix) with ESMTP id C96D466063; Thu, 13 Dec 2018 18:59:38 +0000 (UTC) Subject: Re: [PATCH] handle expressions in __builtin_has_attribute (PR 88383) To: Martin Sebor , Gcc Patch List References: From: Jeff Law Openpgp: preference=signencrypt Message-ID: <6b1f15b6-1446-21a2-54aa-b92ec6b5541f@redhat.com> Date: Thu, 13 Dec 2018 18:59:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg00965.txt.bz2 On 12/5/18 8:55 PM, Martin Sebor wrote: > The __builtin_has_attribute function fails with an ICE when > its first argument is an expression such as INDIRECT_REF, or > many others.  The code simply assumes it's either a type or > a decl.  The attached patch corrects this oversight. > > While testing the fix I also noticed the C++ front end expects > the first operand to be a unary expression, which causes most > other kinds of expressions to be rejected.  The patch fixes > that as well. > > Finally, while testing the fix even more I realized that > the built-in considers the underlying array itself in ARRAY_REF > expressions rather than its type, which leads to inconsistent > results for overaligned arrays (it's the array itself that's > overaligned, not its elements).  So I fixed that too and > adjusted the one test designed to verify this. > > Tested on x86_64-linux. > > Martin > > gcc-88383.diff > > PR c/88383 - ICE calling __builtin_has_attribute on a reference > > gcc/c-family/ChangeLog: > > PR c/88383 > * c-attribs.c (validate_attribute): Handle expressions. > (has_attribute): Handle types referenced by expressions. > Avoid considering array attributes in ARRAY_REF expressions . > > gcc/cp/ChangeLog: > > PR c/88383 > * parser.c (cp_parser_has_attribute_expression): Handle assignment > expressions. > > gcc/testsuite/ChangeLog: > > PR c/88383 > * c-c++-common/builtin-has-attribute-4.c: Adjust expectations. > * c-c++-common/builtin-has-attribute-6.c: New test. Well, the high level question here is do we want to support this builtin on expressions at all. Generally attributes apply to types and decls, not expressions. Clearly we shouldn't fault, but my first inclination is that the attribute applies to types or decls, not expressions. In that case we should just be issuing an error. I could be convinced otherwise, so if you think we should support passing expressions to this builtin, make your case. jeff