From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31771 invoked by alias); 15 Jan 2019 16:31:47 -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 31753 invoked by uid 89); 15 Jan 2019 16:31:45 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: =?ISO-8859-1?Q?No, score=-1.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=of, of=c2, be, Ping?= X-HELO: mail-qt1-f193.google.com Received: from mail-qt1-f193.google.com (HELO mail-qt1-f193.google.com) (209.85.160.193) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 15 Jan 2019 16:31:42 +0000 Received: by mail-qt1-f193.google.com with SMTP id p17so3622128qtl.5 for ; Tue, 15 Jan 2019 08:31:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:from:to:references:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=VKstrgLvilcbyT8oNeKvV7FfUqYY4mXz6i7hBsq5wmI=; b=pPvGBSoZVYYmki7rsAFaYCz9W0SYkpnf9fbTVggtpLO5LwunNbi9IL+5iGh6JpmB8y OUMcGxPFRZU6yI/y4hQZTcBsteF3PnEfrMZNdP1U98uAAs4ILHdMIPi6RWAn0lD3llFo WL2HkZRcBPzi0tM5OQEkg02XlZfbimpUwDam9JkcjEB84LJLuWTztRngxGKM1Z68jrYR 9QyIM8CM1FI7kKbhBC8W8KsEzNLtQgql4w4HJzHj1mOvzPletcFdyr4dG9dnDOATKmVu 31gN+fAWDonmwraO4+9V3NX/NzIlUzjJfbGw2vS9dNnv4TGVlVqXJ108aK2+peQrVPjX wLFg== Return-Path: Received: from [192.168.0.106] (174-16-101-177.hlrn.qwest.net. [174.16.101.177]) by smtp.gmail.com with ESMTPSA id b51sm59208927qtb.32.2019.01.15.08.31.39 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 15 Jan 2019 08:31:39 -0800 (PST) Subject: Re: PING #2 [PATCH] handle expressions in __builtin_has_attribute (PR 88383) From: Martin Sebor To: Jeff Law , Gcc Patch List References: <6b1f15b6-1446-21a2-54aa-b92ec6b5541f@redhat.com> <5599edd4-61ad-24ce-d19e-376dd9398183@gmail.com> <688c62e2-966a-c950-ca63-b58003610c62@gmail.com> <96b8cb0a-bdfd-b122-ee31-4f156ae09922@gmail.com> <474b59cc-4322-cf8d-e316-9eebefcddcab@gmail.com> <00fda3b4-0cb2-f85e-b4d1-7676834d8303@gmail.com> Message-ID: <2ffcff76-844d-87b7-2f5c-2bbc35cf63b5@gmail.com> Date: Tue, 15 Jan 2019 16:31: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: <00fda3b4-0cb2-f85e-b4d1-7676834d8303@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2019-01/txt/msg00855.txt.bz2 Ping: https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00337.html Jeff, do you have any more questions/concerns or is this patch good to commit? Martin On 1/7/19 5:32 PM, Martin Sebor wrote: > Ping: https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00337.html > > On 1/3/19 3:22 PM, Martin Sebor wrote: >> Ping: https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00337.html >> >> On 12/20/18 7:51 PM, Martin Sebor wrote: >>> Jeff, did this and the rest of the discussion answer your question >>> and if so, is the patch okay to commit? >>> >>>    https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00337.html >>> >>> Martin >>> >>> On 12/13/18 12:48 PM, Martin Sebor wrote: >>>> On 12/13/18 12:20 PM, Martin Sebor wrote: >>>>> On 12/13/18 11:59 AM, Jeff Law wrote: >>>>>> 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. >>>>> >>>>> The support is necessary in order to determine the attributes >>>>> in expressions such as: >>>>> >>>>>    struct S { __attribute__ ((packed)) int a[32]; }; >>>>> >>>>>    extern struct S s; >>>>> >>>>>    _Static_assert (__builtin_has_attribute (s.a, packed)); >>>> >>>> An example involving types might be a better one: >>>> >>>>    typedef __attribute__ ((may_alias)) int* BadInt; >>>> >>>>    void f (BadInt *p) >>>>    { >>>>      _Static_assert (__builtin_has_attribute (*p, may_alias)); >>>>    } >>>> >>>> Martin >>> >> >