From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 59461 invoked by alias); 9 Aug 2017 19:26:20 -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 56938 invoked by uid 89); 9 Aug 2017 19:26:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.3 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=ham version=3.3.2 spammy=H*RU:209.85.216.195, retain, Hx-spam-relays-external:209.85.216.195 X-HELO: mail-qt0-f195.google.com Received: from mail-qt0-f195.google.com (HELO mail-qt0-f195.google.com) (209.85.216.195) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 09 Aug 2017 19:26:17 +0000 Received: by mail-qt0-f195.google.com with SMTP id p3so6720108qtg.5 for ; Wed, 09 Aug 2017 12:26:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=Gj6zVw4ebJ0oWb4AztO6C+4dOBKKKWY11EJobdH4NhI=; b=J+9ejlf47M0Hj3BLd4sinLqRs/HowDqMHjoJgw93Lad3qaPmkXpwikCZKRdPQTL47H Y6jgaaJD4Eo4DL+hnmT2+WDMSLp+zsBeIYeBKPPJYtD7JWIc1oW91nZ3epKA1UJAUMIp nVL1JAithfUshIbH6YXb5gAsuPgpYulzZavS3arF8PoEHLhhtsS82HGvZM61apn/YuNT oU0rHDc48PLR7tqk29Nw24t4icq0W8e3OA94t1g7cMSnAHMKTBKOGep/TRfeYlqDyFdQ 0sebTXbP2fXXs+43gZvXXxUNkW+KVpxSlyUL2DdEcim1nFY7917sqI1u5HRe/SafKzGk j82Q== X-Gm-Message-State: AHYfb5geMT3ZJTSQZGA49G4XPMpt2Rc0EoPISYPa6G5E0l2aX6dv/S6U kYCiTIrFx8PstADr X-Received: by 10.237.61.200 with SMTP id j8mr11797843qtf.111.1502306775467; Wed, 09 Aug 2017 12:26:15 -0700 (PDT) Received: from localhost.localdomain (174-16-105-12.hlrn.qwest.net. [174.16.105.12]) by smtp.gmail.com with ESMTPSA id e66sm2306776qkb.69.2017.08.09.12.26.14 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 09 Aug 2017 12:26:14 -0700 (PDT) Subject: Re: [PATCH 1/3] improve detection of attribute conflicts (PR 81544) To: Joseph Myers References: <20170809115341.GM17069@redhat.com> <52dbf0e4-09b3-f313-d6b1-92f0e5ac6c16@gmail.com> Cc: Marek Polacek , Gcc Patch List , Jason Merrill From: Martin Sebor Message-ID: Date: Wed, 09 Aug 2017 19:26:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2017-08/txt/msg00698.txt.bz2 On 08/09/2017 11:13 AM, Joseph Myers wrote: > On Wed, 9 Aug 2017, Martin Sebor wrote: > >>>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c >>>> b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c >>>> index 146b76c..58a4742 100644 >>>> --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c >>>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c >>>> @@ -113,7 +113,7 @@ int test9 (int *intarr) >>>> >>>> int test99 (int *intarr) >>>> { >>>> - extern int foo9 (int) __attribute__ ((pure)); >>>> + extern int foo9 (int) __attribute__ ((const)); >>>> int h, v; >>>> g9 = 9; >>>> h = foo9 (g9); >>>> >>> >>> And why this? I'd avoid modifying existing tests like that unless >>> it's directly related to the new diagnostic. >> >> The same function is declared earlier on in the file with attribute >> const and the redeclaration with attribute pure triggers a warning >> due to the two being considered mutually exclusive. > > const + pure is a *redundant* combination, but I think it should always > have the meaning of const regardless of the order in which they appear or > whether they appear on separate declarations. That's different from > combinations that are actually nonsensical. To a large degree I agree with this characterization. I think of one as a subset of the other. FWIW, my initial approach was to introduce the concept of attribute subsets (as in pure being a subset of the restrictions of const). But in discussing it with Marek we felt it was too complicated and that mutual exclusivity was good enough here. I'd be fine with reintroducing the subset/superset distinction, or any other that makes sense and helps find potential bugs that might result from redeclaring the same function with the other of this pair attributes. I'd also be okay with not diagnosing this combination if I could convince myself that it's safe (or can be made safe) and treated consistently. > It's not clear to me that > const + pure should be diagnosed by default any more than declaring the > same function twice, with the const attribute both time, should be > diagnosed. The problem whose subset the diagnostic detects is declaring the function const and calling it before the pure declaration that corresponds to its definition is seen. I.e., the inverse of what the ssa-ccp-2.c test does. I think a mismatch between the attributes is as suspect as a mismatch in the function's signature. In any event, it sounds to me that conceptually, it might be useful to be able to specify which of a pair of mutually exclusive (or redundant) attributes to retain and/or accept: the first or the second, and not just whether to accept or drop the most recent one. That will make it possible to make the most appropriate decision about each specific pair of attributes without impacting any of the others. If this sounds right to you (and others) let me make that change and post an updated patch. But If I misunderstood and this solution wouldn't help please let me know. Martin PS I've also wondered if -Wattributes is the right option to use for these sorts of conflicts/redundancies. It is the only option used now but the manual makes it sound that -Wignored-attributes should be expected. AFAICS, -Wignored-attributes is only used by the C++ front end for templates. Should it be extended to these cases as well?