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 E17BA3858012 for ; Tue, 4 May 2021 19:00:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E17BA3858012 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-584-JUKuaP46OqeMPLSbizNYxA-1; Tue, 04 May 2021 15:00:53 -0400 X-MC-Unique: JUKuaP46OqeMPLSbizNYxA-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A33D0A40C4; Tue, 4 May 2021 19:00:52 +0000 (UTC) Received: from t14s.localdomain (ovpn-112-65.phx2.redhat.com [10.3.112.65]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0F04219C44; Tue, 4 May 2021 19:00:51 +0000 (UTC) Message-ID: Subject: Re: [PATCH] Add gnu::diagnose_as attribute From: David Malcolm To: Matthias Kretz , gcc-patches@gcc.gnu.org Cc: libstdc++@gcc.gnu.org Date: Tue, 04 May 2021 15:00:51 -0400 In-Reply-To: <9837144.KqQxQWANqO@excalibur> References: <14205410.xuKvIAzr1H@excalibur> <91863212.B8guWdUDZo@excalibur> <72d4620b42f3bdbb3eab94a46ccc0434ed160fe0.camel@redhat.com> <9837144.KqQxQWANqO@excalibur> User-Agent: Evolution 3.38.4 (3.38.4-1.fc33) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 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=-7.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, 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: Tue, 04 May 2021 19:00:57 -0000 On Tue, 2021-05-04 at 16:23 +0200, Matthias Kretz wrote: > On Tuesday, 4 May 2021 15:34:13 CEST David Malcolm wrote: > > On Tue, 2021-05-04 at 13:13 +0200, Matthias Kretz wrote: > > > This attribute overrides the diagnostics output string for the > > > entity > > > it > > > appertains to. The motivation is to improve QoI for library TS > > > implementations, where diagnostics have a very bad signal-to- > > > noise > > > ratio > > > due to the long namespaces involved. > > > [...] > > > > Thanks for the patch, it looks very promising. > > Thanks. I'm new to modifying the compiler like this, so please be > extra > careful with my patch. I believe I understand most of what I did, but > I might > have misunderstood. :) That's tends to be how I feel when working on the C++ FE :) > > > The patch has no testcases; it should probably add test coverage > > for: > > - the various places and ways in which diagnose_as can affect the > > output, > > - disabling it with the option > > - the various ways in which the user can get diagnose_as wrong > > - etc > > Right. If you know of an existing similar testcase, that'd help me a > lot to > get started. FWIW I implemented the %H and %I codes in f012c8ef4b35dcee9b5a3807868d050812d5b3b9 and that commit adds various DejaGnu testcases that exercise C++ diagnostics with and without various options, verifying the precise wording of errors. So that might be a good place to look. > > > Does the patch affect the output of labels when underlining ranges of > > source code in diagnostics? > > AFAIU (and tested), it doesn't affect source code output. So, no? Sorry, I was unclear. I was referring to this kind of thing: $ cat t.C #include std::string test (std::string s) { return &s; } $ g++ t.C t.C: In function ‘std::string test(std::string)’: t.C:5:12: error: could not convert ‘& s’ from ‘std::string*’ {aka ‘std::__cxx11::basic_string*’} to ‘std::string’ {aka ‘std::__cxx11::basic_string’} 5 | return &s; | ^~ | | | std::string* {aka std::__cxx11::basic_string*} i.e. the final line in the output above, where the underlined range of source code in line 5 is labelled, showing the type of the expression. Hopefully it ought to automatically just drop out of the work you've already done. FWIW an example of implementation this can be seen in a14feb3c783fba6af8d66b8138214a3a313be5c5 (which added labels for type errors in C++ binary operators). > Does the patch interact correctly with the %H and %I codes that try to > show the differences between two template types? I don't know. I'll try to find out. If you have a good idea (or pointer) for a testcase, let me know. See above. Dave