Jeff Law <l...@redhat.com> 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
--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
2016-12-09 Rainer Orth <r...@cebitec.uni-bielefeld.de>
* g++.dg/debug/dwarf2/typedef1.C: Adjust pattern for last change.
# 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 } }
