Re: [PATCH] Improved diagnostics for casts and enums

[Volker Reichelt <v dot reichelt at netcologne dot de>]

On 27 Apr, Martin Sebor wrote:
> On 04/27/2017 01:29 AM, Volker Reichelt wrote:
>> Hi,
>>
>> the following two patches aim at improving GCC's diagnostics to help
>> the user to get rid of old-style casts. While old-style pointer casts
>> are really bad and need to be weeded out quickly, old-style casts between
>> arithmetic types are IMHO much more tolerable. The patches allow to
>> easily distinguish between those situations.
> 
> FWIW, it can be most helpful to include this sort of detail (and
> similar) in diagnostics.  In the case of the C-style cast, besides
> mentioning the type of the result, it might be even more helpful
> to mention the type of the operand because unlike that of the
> result, its type is not apparent from the cast itself.

In this particular case the operand is evaluated after the warning
message. So I couldn't include it in the message without fiddling
around with the logic. I was also afraid that the warning message would
become too long.

>> The first patch for cp_parser_cast_expression in parser.c just adds
>> the target type of the cast to the diagnostic (like in
>> maybe_warn_about_useless_cast in typeck.c).
>>
>> The second patch for type_to_string in error.c tackles the problem
>> that the name of a type doesn't tell you if you have a class or just
>> a simple enum. Similar to adding "{aka 'int'}" to types that
>> are essentially integers, this patch adds "{enum}" to all
>> enumeration types (and adjusts two testcases accordingly).
> 
> In the C front end %qT prints 'enum E' for an argument of
> an enumerated type.  Is there some significance to having
> the C++ front end print 'E { enum }' or can C++ be made
> consistent?
>
> Martin

Ah, good point! I was so focused on the '{aka ...}' thing that I didn't
see the obvious: I should have used %q#T instead of just %qT to get
the enum prefix with the type. Unfortunately, I already committed the
patch after Nathan's OK before seeing your message.

I'll prepare another patch this weekend to revert my error.c changes and
use %q#T for the warnings about old-style and useless casts mentioned
above.

Another thought just crossed my mind: Other users might find an
enum/class prefix helpful for different warnings. Right now they
would have to modify the compile to use %q#T instead of %qT.
Would it make sense to add a compiler option that sets the verbose
flag for cp_printer unconditionally? Or am I just unaware of existing
functionality?

Regards,
Volker