From patchwork Fri Dec 9 10:16:46 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rainer Orth X-Patchwork-Id: 704390 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3tZp7b1MNYz9t1H for ; Fri, 9 Dec 2016 21:17:13 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="qBfZg0hJ"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:references:date:in-reply-to:message-id :mime-version:content-type; q=dns; s=default; b=RqJUo2DG6dKdjE4A u3QqsrWPWbCnth+ansdbhAJ20DcRLz8ZiR0YpNZ8NIpVloWXoygjzrJPA08Sqanh JOyjFmPl+6pmAgk/pDSjPfoLcfBSvWG5VCuasR6HXtHCS5Apwk5cbPYAdEox7LLa bQ/EN73CJ7x0B/noivzA1CNQidY= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:references:date:in-reply-to:message-id :mime-version:content-type; s=default; bh=1eLaemXzfY/hhgH/PzMtE5 yzMgk=; b=qBfZg0hJSiYzY9vRdBG5by6iW7gGfAIyvJuoXYBZYbCHxGFZSEwc/h 9dq5EG0kQqxaUH8a3pps9I3z0fdVqeeIqbDNgOJA+p1uywq6vya4iWIlUfd0MyOY CqkH9mA16e/NtQRQ0vue1mN1r/uDnXMSAlgY6sanGWXB0Cyfn/23s= Received: (qmail 17058 invoked by alias); 9 Dec 2016 10:17:04 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 17039 invoked by uid 89); 9 Dec 2016 10:17:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=gave, stopping, pound, practical X-HELO: smtp.CeBiTec.Uni-Bielefeld.DE Received: from smtp.CeBiTec.Uni-Bielefeld.DE (HELO smtp.CeBiTec.Uni-Bielefeld.DE) (129.70.160.84) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 09 Dec 2016 10:16:53 +0000 Received: from localhost (localhost.CeBiTec.Uni-Bielefeld.DE [127.0.0.1]) by smtp.CeBiTec.Uni-Bielefeld.DE (Postfix) with ESMTP id D2F67D6A; Fri, 9 Dec 2016 11:16:50 +0100 (CET) Received: from smtp.CeBiTec.Uni-Bielefeld.DE ([127.0.0.1]) by localhost (malfoy.CeBiTec.Uni-Bielefeld.DE [127.0.0.1]) (amavisd-new, port 10024) with LMTP id t7J48dvNK+YY; Fri, 9 Dec 2016 11:16:47 +0100 (CET) Received: from lokon.CeBiTec.Uni-Bielefeld.DE (lokon.CeBiTec.Uni-Bielefeld.DE [129.70.161.152]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by smtp.CeBiTec.Uni-Bielefeld.DE (Postfix) with ESMTPS id 2B827D69; Fri, 9 Dec 2016 11:16:47 +0100 (CET) Received: (from ro@localhost) by lokon.CeBiTec.Uni-Bielefeld.DE (8.15.2+Sun/8.15.2/Submit) id uB9AGkTa004162; Fri, 9 Dec 2016 11:16:46 +0100 (MET) From: Rainer Orth To: Jeff Law Cc: Martin Sebor , Joseph Myers , Gcc Patch List Subject: Re: [RFC PATCH] avoid printing type suffix with %E (PR c/78165) References: <79de46b9-9770-02dd-6d98-9c72b7e6c59d@gmail.com> <7dd11e5f-7a2d-7d2f-f34b-8b10f64029cc@gmail.com> <76f75104-3da5-3947-4cfc-d4c3cbc6d5c4@redhat.com> Date: Fri, 09 Dec 2016 11:16:46 +0100 In-Reply-To: <76f75104-3da5-3947-4cfc-d4c3cbc6d5c4@redhat.com> (Jeff Law's message of "Tue, 29 Nov 2016 20:57:02 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (usg-unix-v) MIME-Version: 1.0 X-IsSubscribed: yes Jeff Law writes: > On 11/19/2016 02:04 PM, Martin Sebor wrote: >> On 10/26/2016 02:46 PM, Joseph Myers wrote: >>> On Wed, 26 Oct 2016, Martin Sebor wrote: >>> >>>> The attached patch implements one such approach by having the pretty >>>> printer recognize the space format flag to suppress the type suffix, >>>> so "%E" still prints the suffix but "% E" does not. I did this to >>>> preserve the existing output but I think it would be nicer to avoid >>>> printing the suffix with %E and treat (for instance) the pound sign >>>> as a request to add the suffix. I have tested the attached patch >>>> but not the alternative. >>> >>> I think printing the suffixes is a relic of %E being used to print full >>> expressions. >>> >>> It's established by now that printing expressions reconstructed from >>> trees >>> is a bad idea; we can get better results by having precise location >>> ranges >>> and underlining the relevant part of the source. So if we could make >>> sure >>> nowhere is trying the use %E (or %qE, etc.) with expressions that might >>> not be constants, where the type might be relevant, then we'd have >>> confidence that stopping printing the suffix is safe. But given the low >>> quality of the reconstructed expressions, it's probably safe anyway. >>> >>> (Most %qE uses are for identifiers not expressions. If we give >>> identifiers a different static type from "tree" - and certainly there >>> isn't much reason for them to have the same type as expressions - then >>> we'll need to change the format for either identifiers or expressions.) >> >> Attached is a trivial patch to remove the suffix. I didn't see >> any failures in the test suite as a result. I didn't attempt to >> remove the type suffix from any tests (nor did my relatively >> superficial search find any) but it will help simplify the tests >> for my patches that are still in the review queue. >> >> I should add to the rationale for the change I gave in my reply >> to Jeff: >> >> https://gcc.gnu.org/ml/gcc-patches/2016-11/msg01692.html >> >> that the print_dec[su] function that's sometimes used to format >> integers in warning messages (e.g., by the -Walloca-larger-than >> pass) doesn't add the suffix because it doesn't have knowledge >> of the argument's type (it operates on wide_int). That further >> adds to the inconsistency in diagnostics. This patch makes all >> integers in diagnostics consistent regardless of their type. >> >> Thanks >> Martin >> >> gcc-78165.diff >> >> >> PR c/78165 - avoid printing type suffix for constants in %E output >> >> gcc/c-family/ChangeLog: >> >> PR c/78165 >> * c-pretty-print (pp_c_integer_constant): Avoid formatting type >> suffix. > I think you and Joseph have made a practical case for dropping the type > suffix. > > Ok for the trunk. The check-in lacked the gcc/testsuite ChangeLog. Besides, the patch caused a testsuite regression on Solaris with /bin/as (sparc and x86, 32 and 64-bit): +FAIL: g++.dg/debug/dwarf2/typedef1.C -std=gnu++11 scan-assembler-times DW_AT_name: "foo<1>"|"foo<1u>.."[^\\n]*DW_AT_name 1 +FAIL: g++.dg/debug/dwarf2/typedef1.C -std=gnu++14 scan-assembler-times DW_AT_name: "foo<1>"|"foo<1u>.."[^\\n]*DW_AT_name 1 +FAIL: g++.dg/debug/dwarf2/typedef1.C -std=gnu++98 scan-assembler-times DW_AT_name: "foo<1>"|"foo<1u>.."[^\\n]*DW_AT_name 1 Turns out an incomplete adjustment to the pattern, fixed as follows. Will commit as obvious once testing on more configurations (gas, Linux) has completed. Rainer # HG changeset patch # Parent 9cf8fdbb2f5fbebf7cf273c95a1d1e72567aa76c Fix g++.dg/debug/dwarf2/typedef1.C diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/typedef1.C b/gcc/testsuite/g++.dg/debug/dwarf2/typedef1.C --- a/gcc/testsuite/g++.dg/debug/dwarf2/typedef1.C +++ b/gcc/testsuite/g++.dg/debug/dwarf2/typedef1.C @@ -3,7 +3,7 @@ // { dg-options "-gdwarf-2 -dA -fno-debug-types-section" } // { dg-do compile } // { dg-final { scan-assembler-times "DW_TAG_structure_type" 2 } } -// { dg-final { scan-assembler-times "DW_AT_name: \"foo<1>\"|\"foo<1u>..\"\[^\n\]*DW_AT_name" 1 } } +// { dg-final { scan-assembler-times "DW_AT_name: \"foo<1>\"|\"foo<1>..\"\[^\n\]*DW_AT_name" 1 } } // { dg-final { scan-assembler-times "DW_TAG_enumeration_type" 2 } } // { dg-final { scan-assembler-times "DW_AT_name: \"typedef foo<1>::type type\"|\"typedef foo<1>::type type..\"\[^\n\]*DW_AT_name" 1 } } // { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_enumeration_type" 1 } }