[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

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University


2016-12-09  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* g++.dg/debug/dwarf2/typedef1.C: Adjust pattern for last change.

> 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