| Message ID | 7dd11e5f-7a2d-7d2f-f34b-8b10f64029cc@gmail.com |
|---|---|
| State | New |
| Headers | show |
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. jeff
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. diff --git a/gcc/c-family/c-pretty-print.c b/gcc/c-family/c-pretty-print.c index 7ad5900..c32d0a0 100644 --- a/gcc/c-family/c-pretty-print.c +++ b/gcc/c-family/c-pretty-print.c @@ -904,15 +904,6 @@ pp_c_void_constant (c_pretty_printer *pp) static void pp_c_integer_constant (c_pretty_printer *pp, tree i) { - int idx; - - /* We are going to compare the type of I to other types using - pointer comparison so we need to use its canonical type. */ - tree type = - TYPE_CANONICAL (TREE_TYPE (i)) - ? TYPE_CANONICAL (TREE_TYPE (i)) - : TREE_TYPE (i); - if (tree_fits_shwi_p (i)) pp_wide_integer (pp, tree_to_shwi (i)); else if (tree_fits_uhwi_p (i)) @@ -929,24 +920,6 @@ pp_c_integer_constant (c_pretty_printer *pp, tree i) print_hex (wi, pp_buffer (pp)->digit_buffer); pp_string (pp, pp_buffer (pp)->digit_buffer); } - if (TYPE_UNSIGNED (type)) - pp_character (pp, 'u'); - if (type == long_integer_type_node || type == long_unsigned_type_node) - pp_character (pp, 'l'); - else if (type == long_long_integer_type_node - || type == long_long_unsigned_type_node) - pp_string (pp, "ll"); - else for (idx = 0; idx < NUM_INT_N_ENTS; idx ++) - if (int_n_enabled_p[idx]) - { - char buf[2+20]; - if (type == int_n_trees[idx].signed_type - || type == int_n_trees[idx].unsigned_type) - { - sprintf (buf, "I%d", int_n_data[idx].bitsize); - pp_string (pp, buf); - } - } } /* Print out a CHARACTER literal. */