From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id C0F963847805 for ; Fri, 2 Jul 2021 20:52:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C0F963847805 Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-500-4KntZXSpOsyxgcn6HZVOBQ-1; Fri, 02 Jul 2021 16:52:02 -0400 X-MC-Unique: 4KntZXSpOsyxgcn6HZVOBQ-1 Received: by mail-qt1-f200.google.com with SMTP id r9-20020ac86d290000b0290251ccc34170so2835842qtu.1 for ; Fri, 02 Jul 2021 13:52:02 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=YVZwsKNG6ORq9+rAAmH415YnffkpdrFTJqXUCQZtT8Q=; b=XPkMHIJ6aKvKhuv18NmbwEtqLsUbskgYDeDsFxFRrHOinyLYTQvZLb8ReiOhRx31f3 0Gk7+CEedPa2qTTORCkN1AVIYzD49pYNYfUNWZSbo/PHn1eIPM0c9cRXduEyo+It/lSG Lfe3qrntTRFDixikMghfmTqT9JOO914/TzeV54W/2C7F2OzL9LnT0xtcnQKxHWpr05h0 OUVuIhrYWfmXZYVVJVlG5m9nzO7olhFRnwlxcZMl94uiRK48ugSbIw/3SJhaW8YHehai Rsi+36WWJQzVr1RyR2cBYVu/OZICMlCDW7/ARIso6uJh6B9V+rGM373EHrACL8Y1gA0G xQYA== X-Gm-Message-State: AOAM533imO2wWP7l4+R4PMQrIBoivyfL0ePRMcSWXp7O+Gt4nSyrCp6L ygnpnwqQB1S5L772QvEDolnL7Hd/Vm64rKc0LWjoJ2LI5KyNks1gt62CHCZlWfwcN50oUToq2LZ EPchoRQKINd2Z+Nwmeg== X-Received: by 2002:a37:43c9:: with SMTP id q192mr1736487qka.470.1625259122224; Fri, 02 Jul 2021 13:52:02 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxRSLPcK0GBkVHREXNij9SqrQ7cfWqc+5nZLCOlVaY9kr0Jl9k1FjDqOgY5Dd5QkXMrAH26sQ== X-Received: by 2002:a37:43c9:: with SMTP id q192mr1736467qka.470.1625259121983; Fri, 02 Jul 2021 13:52:01 -0700 (PDT) Received: from t14s.localdomain (c-73-69-212-193.hsd1.ma.comcast.net. [73.69.212.193]) by smtp.gmail.com with ESMTPSA id i21sm272240qtq.91.2021.07.02.13.52.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 02 Jul 2021 13:52:01 -0700 (PDT) Message-ID: Subject: Re: [PING][PATCH 2/4] remove %G and %K from calls in front end and middle end (PR 98512) From: David Malcolm To: Martin Sebor , gcc-patches Cc: Jeff Law , Aldy Hernandez Date: Fri, 02 Jul 2021 16:52:00 -0400 In-Reply-To: <8632a55d-dacc-7173-4576-7c9ae96e63d8@gmail.com> References: <2e49b6c6-a403-a207-c41e-58f78df96b84@gmail.com> <945093c7-de5e-0350-6030-e4e79ea41161@gmail.com> <5aff247f-dfcb-cecf-e07e-b5fca877f911@gmail.com> <36e75bb4-2c3c-5b3a-9fe1-9cdb046666a3@gmail.com> <12ed261b-d35e-4c07-949a-67e085cec750@gmail.com> <7b58bb26-fab6-fc65-32e5-e09139474ba5@gmail.com> <9fd191802dfa8724ec0e09bac796c8c375ebf678.camel@redhat.com> <8632a55d-dacc-7173-4576-7c9ae96e63d8@gmail.com> User-Agent: Evolution 3.38.4 (3.38.4-1.fc33) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.4 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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, 02 Jul 2021 20:52:06 -0000 On Thu, 2021-07-01 at 14:14 -0600, Martin Sebor wrote: > On 6/30/21 5:35 PM, David Malcolm wrote: > > On Wed, 2021-06-30 at 13:45 -0600, Martin Sebor wrote: > > > On 6/30/21 9:39 AM, Martin Sebor wrote: > > > > Ping.  Attached is the same patch rebased on top the latest > > > > trunk. > > > > > > Please see the attached patch instead.  The previous one had typo > > > in it. > > > > > > > > > > > As discussed in the review of Aldy's recent changes to the > > > > backwards > > > > threader, he has run into the same bug the patch fixes.  > > > > Getting this > > > > patch set reviewed and approved would be helpful in keeping him > > > > from > > > > having to work around the bug. > > > > > > > > https://gcc.gnu.org/pipermail/gcc/2021-June/236608.html > > > > > > > > On 6/10/21 5:27 PM, Martin Sebor wrote: > > > > > This diff removes the uses of %G and %K from all warning_at() > > > > > calls > > > > > throughout GCC front end and middle end.  The inlining > > > > > context is > > > > > included in diagnostic output whenever it's present. > > > > > > > > > > > Thanks for writing the patch. > > > > I went through the full patch, though my eyes may have glazed over > > in > > places at all of the %G and %K removals.  I *think* you got them > > mostly > > correct, apart from the following possible issues and nits... > > > > > diff --git a/gcc/expr.c b/gcc/expr.c > > > index 025033c9ecf..b9fe1cf91d7 100644 > > > --- a/gcc/expr.c > > > +++ b/gcc/expr.c > > > > [...] > > > > > @@ -11425,10 +11425,10 @@ expand_expr_real_1 (tree exp, rtx > > > target, machine_mode tmode, > > >                                          DECL_ATTRIBUTES > > > (fndecl))) != NULL) > > >           { > > >             const char *ident = lang_hooks.decl_printable_name > > > (fndecl, 1); > > > -           warning_at (tree_nonartificial_location (exp), > > > +           warning_at (EXPR_LOCATION (exp), > > > > Are we preserving the existing behavior for > > __attribute__((__artificial__)) here? > > Is this behavior handled somewhere else in the patch kit? > > Yes.  The warning infrastructure (set_inlining_locations) uses > the location of the site into which the statement has been inlined > regardless of whether the inlined function is artificial. Do we have test coverage for this, though? [...] > > > > @@ -90,8 +90,8 @@ NOIPA void warn_g2 (struct A *p) > > >     g2 (p); > > >   } > > >   > > > -// { dg-message "inlined from 'g2'" "" { target *-*-* } 0 } > > > -// { dg-message "inlined from 'warn_g2'" "" { target *-*-* } 0 } > > > +// { dg-message "inlined from 'g2'" "note on line 93" { target > > > *-*-* } 0 } > > > +// { dg-message "inlined from 'warn_g2'" "note on line 94" { > > > target *-*-* } 0 } > > > > You've added descriptions to disambiguate all of the various > > directives > > on line 0, which is good, but I don't like the use of line numbers > > in > > the descriptions, since it will get very confusing if the numbering > > changes. > > > > Would it work to use the message text as the description e.g. > > > >    // { dg-message "inlined from 'warn_g2'" "inlined from > > 'warn_g2'" { target *-*-* } 0 } > > > > or somesuch? > > It would certainly work, they're just informational labels printed > by DejaGnu when the assertions fail.  I added them to help me see > what they went with while working with the test.  I'm not concerned > about the line numbers changing.  If they do and someone notices, > they can update them, the same way they might want to if they > rename the functions they're inlined into. [...] > > > > After reading through this and trying to grok it, I see that this > > file > > logically can be split into several parts: the "warn*" functions, > > then > > the "nowarn*_ignore0" functions, then the "nowarn*_ignore_1" > > functions > > etc. > > > > Please add some kind of separator comment between each of these > > parts > > to make it easy for the reader to see this structure. > > Sure, I've added a comment. > Thanks. > Attached is the revised patch for reference.  Since it just removes > the uses of the %K and %G directives made redundant by the first > patch in the series I'll go ahead and commit it as obvious in a day > or so after patch 1 unless someone has further questions or requests > for changes. Please can you look into the "__artificial__" test coverage, and address the line number thing above. Dave