[RFC] avoid printing type suffix with %E (PR c/78165)

Jeff Law <law@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

> 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.

Thanks!  I tested x86_64 so it's possible that similar adjustments
will be needed in other target specific tests.  I'll be on the lookout
for more fallout.

Martin