From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by sourceware.org (Postfix) with ESMTP id 1E3B9386F451 for ; Fri, 29 May 2020 16:30:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1E3B9386F451 Received: from mail-qv1-f72.google.com (mail-qv1-f72.google.com [209.85.219.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-272-DjO3eGjuPE2ROs26V5nHRw-1; Fri, 29 May 2020 12:30:18 -0400 X-MC-Unique: DjO3eGjuPE2ROs26V5nHRw-1 Received: by mail-qv1-f72.google.com with SMTP id w12so1529446qvh.13 for ; Fri, 29 May 2020 09:30:18 -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:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=RT270LMoj4L8dsgdJCfbA5iGJj8dbseQx4TooOQNW9c=; b=idfbjo3JWt1s2mLu9HDCOoG5T/bgC+EupylALS7Y1lxTUAU5HBNW0PswFtVkWdmm+q bpfhxLd/xjFOhNLOctPcEtPM9FvwvRxYJ+lW2b7LsLKMcNSZXc1eiWm3M+6GJys2pnp1 6NuXa7f4Y7btNfbIa+G7t6SU8F6mRvsCvqq+QWtOhKA4JSuy2prhLMxcX1b2n4+/rqyi brs5wjhOOW92PFv2diD1ePpVBwEZH8BmXrZoDKVLCGergWl+0sKrHP8v27Ldw0WG5X/a v2eaFYHQKVFa3s56RREAw6OUbu7lcUfzVcJc5dBtu4zAWxW4Jk8MJ3O9HyuOYUdwdcwH Ra+w== X-Gm-Message-State: AOAM5328lKacMGSXTywhgAo82NKAI29Fxl+4N5bk0XAf5eUWjs7c603K +lzJYPUH49QodCDQl+M5rntpSg5n85qTOG07b4XL0FnzyFNC0yK/OdrIsJ3blY95XtI4QtQBEHz PZUE8h3EvtMIjPa16Xw== X-Received: by 2002:a05:620a:158d:: with SMTP id d13mr8595943qkk.327.1590769817300; Fri, 29 May 2020 09:30:17 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx3tApIo7dSnOkdw8waLI+7JyjaC4eAbFvfwlUbspP0Czmiw/vbXPm0S9s2N9I0hAGriRBGKw== X-Received: by 2002:a05:620a:158d:: with SMTP id d13mr8595903qkk.327.1590769816881; Fri, 29 May 2020 09:30:16 -0700 (PDT) Received: from [192.168.1.148] (209-6-216-142.s141.c3-0.smr-cbr1.sbo-smr.ma.cable.rcncustomer.com. [209.6.216.142]) by smtp.gmail.com with ESMTPSA id 130sm7601974qko.113.2020.05.29.09.30.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 29 May 2020 09:30:16 -0700 (PDT) Subject: Re: [PATCH v2] c++: Fix bogus -Wparentheses warning [PR95344] To: Marek Polacek Cc: GCC Patches References: <20200527002541.546573-1-polacek@redhat.com> <20200528231121.GL39571@redhat.com> From: Jason Merrill Message-ID: <392dd7c0-8bfa-7022-9e97-1281637507ef@redhat.com> Date: Fri, 29 May 2020 12:30:15 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <20200528231121.GL39571@redhat.com> Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-18.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 May 2020 16:30:21 -0000 On 5/28/20 7:11 PM, Marek Polacek wrote: > On Thu, May 28, 2020 at 05:01:51PM -0400, Jason Merrill wrote: >> On 5/26/20 8:25 PM, Marek Polacek wrote: >>> Since r267272, which added location wrappers, cp_fold loses >>> TREE_NO_WARNING on a MODIFY_EXPR that finish_parenthesized_expr set, and >>> that results in a bogus -Wparentheses warning. >>> >>> I.e., previously we had "b = 1" but now we have "VIEW_CONVERT_EXPR(b) = 1" >>> and cp_fold_maybe_rvalue folds away the location wrapper and so we do >>> 2718 x = fold_build2_loc (loc, code, TREE_TYPE (x), op0, op1); >>> in cp_fold and the flag is lost. >>> >>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10/9? >>> >>> PR c++/95344 >>> * cp-gimplify.c (cp_fold) : Set TREE_NO_WARNING. >>> >>> * c-c++-common/Wparentheses-2.c: New test. >>> --- >>> gcc/cp/cp-gimplify.c | 5 ++++- >>> gcc/testsuite/c-c++-common/Wparentheses-2.c | 18 ++++++++++++++++++ >>> 2 files changed, 22 insertions(+), 1 deletion(-) >>> create mode 100644 gcc/testsuite/c-c++-common/Wparentheses-2.c >>> >>> diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c >>> index 53d715dcd89..8b505dd878c 100644 >>> --- a/gcc/cp/cp-gimplify.c >>> +++ b/gcc/cp/cp-gimplify.c >>> @@ -2745,7 +2745,10 @@ cp_fold (tree x) >>> x = org_x; >>> } >>> if (code == MODIFY_EXPR && TREE_CODE (x) == MODIFY_EXPR) >>> - TREE_THIS_VOLATILE (x) = TREE_THIS_VOLATILE (org_x); >>> + { >>> + TREE_THIS_VOLATILE (x) = TREE_THIS_VOLATILE (org_x); >>> + TREE_NO_WARNING (x) = TREE_NO_WARNING (org_x); >>> + } >> >> I wonder if we want to copy these flags lower down for any EXPR_P (x) where >> TREE_CODE (x) == code? > > Sounds good; I don't think we want to lose those flags when folding in general, > not just for MODIFY_EXPR. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? OK. > -- >8 -- > Since r267272, which added location wrappers, cp_fold loses > TREE_NO_WARNING on a MODIFY_EXPR that finish_parenthesized_expr set, and > that results in a bogus -Wparentheses warning. > > I.e., previously we had "b = 1" but now we have "VIEW_CONVERT_EXPR(b) = 1" > and cp_fold_maybe_rvalue folds away the location wrapper and so we do > 2718 x = fold_build2_loc (loc, code, TREE_TYPE (x), op0, op1); > in cp_fold and the flag is lost. > > PR c++/95344 > * cp-gimplify.c (cp_fold) : Don't set > TREE_THIS_VOLATILE here. > (cp_fold): Set it here along with TREE_NO_WARNING. > > * c-c++-common/Wparentheses-2.c: New test. > --- > gcc/cp/cp-gimplify.c | 8 ++++++-- > gcc/testsuite/c-c++-common/Wparentheses-2.c | 18 ++++++++++++++++++ > 2 files changed, 24 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/c-c++-common/Wparentheses-2.c > > diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c > index 53d715dcd89..d6723e44ec4 100644 > --- a/gcc/cp/cp-gimplify.c > +++ b/gcc/cp/cp-gimplify.c > @@ -2744,8 +2744,6 @@ cp_fold (tree x) > else > x = org_x; > } > - if (code == MODIFY_EXPR && TREE_CODE (x) == MODIFY_EXPR) > - TREE_THIS_VOLATILE (x) = TREE_THIS_VOLATILE (org_x); > > break; > > @@ -2994,6 +2992,12 @@ cp_fold (tree x) > return org_x; > } > > + if (EXPR_P (x) && TREE_CODE (x) == code) > + { > + TREE_THIS_VOLATILE (x) = TREE_THIS_VOLATILE (org_x); > + TREE_NO_WARNING (x) = TREE_NO_WARNING (org_x); > + } > + > if (!c.evaluation_restricted_p ()) > { > fold_cache->put (org_x, x); > diff --git a/gcc/testsuite/c-c++-common/Wparentheses-2.c b/gcc/testsuite/c-c++-common/Wparentheses-2.c > new file mode 100644 > index 00000000000..1aa5d314ae7 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/Wparentheses-2.c > @@ -0,0 +1,18 @@ > +// PR c++/95344 - bogus -Wparentheses warning. > +// { dg-do compile } > +// { dg-options "-Wparentheses" } > + > +#ifndef __cplusplus > +# define bool _Bool > +# define true 1 > +# define false 0 > +#endif > + > +void > +f (int i) > +{ > + bool b = false; > + if (i == 99 ? (b = true) : false) // { dg-bogus "suggest parentheses" } > + { > + } > +} > > base-commit: 3d8d5ddb539a5254c7ef83414377f4c74c7701d4 >