[PING #2] [PATCH] Improved diagnostics for casts and enums
Ping #2 of Volker's patch:
https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01576.html
The patch reverts the format used in the output of enumerated
types from the 'E {enum}' notation introduced in r247347 to just
'E' or to the standard 'enum E' with %#qT.
Nathan, unless you have concerns with or objections to this change,
since you approved r247347, can you please approve this?
Btw., to be consistent, GCC should make use of %#qT for other
kinds of types besides enums so that something like the following:
struct S { };
void g (S s) { s = (S)s; }
issues
warning: useless cast to type ‘struct S’ [-Wuseless-cast]
rather than just
warning: useless cast to type ‘S’ [-Wuseless-cast]
as it still does now. More broadly, I wonder if types should
always (or by default) include the class-key or enum keyword
in C++? In C?
Martin
On 06/20/2017 10:09 AM, Martin Sebor wrote:
So here's the patch that reverts the special enum handling in
type_to_string and uses %q#T instead of %qT for two casting-related
diagnostics.
Bootstrapped and regtested on x86_64-pc-linux-gnu.
OK for trunk?
The "E {enum}'" notation is still on trunk so it seems that this
patch has never been committed and I can't find approval of it
in the archive. To make sure it doesn't get forgotten, please
consider this a ping on Volker's behalf:
https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01576.html
Thanks
Martin
As one can see from the testsuite changes, there are several
casting- and conversion-related messages like "invalid conversion from",
"cannot convert", "invalid cast" that still use the simple %qT
form. I'll give it a try to use %q#T there as well and prepare a
separate patch if this really improves the diagnostics.
Regards,
Volker
2017-04-30 Volker Reichelt
* parser.c (cp_parser_cast_expression): Use %q#T instead of %qT
in old-style cast diagnostic.
* typeck.c (maybe_warn_about_useless_cast): Use %q#T instead of %qT
in useless cast diagnostic.
* error.c (type_to_string): Remove enum special handling.
Index: gcc/cp/parser.c
===
--- gcc/cp/parser.c(revision 247394)
+++ gcc/cp/parser.c(working copy)
@@ -8764,7 +8764,7 @@
&& !VOID_TYPE_P (type)
&& current_lang_name != lang_name_c)
warning (OPT_Wold_style_cast,
- "use of old-style cast to %qT", type);
+ "use of old-style cast to %q#T", type);
/* Only type conversions to integral or enumeration types
can be used in constant-expressions. */
Index: gcc/cp/typeck.c
===
--- gcc/cp/typeck.c(revision 247394)
+++ gcc/cp/typeck.c(working copy)
@@ -6631,7 +6631,7 @@
? xvalue_p (expr) : lvalue_p (expr))
&& same_type_p (TREE_TYPE (expr), TREE_TYPE (type)))
|| same_type_p (TREE_TYPE (expr), type))
-warning (OPT_Wuseless_cast, "useless cast to type %qT", type);
+warning (OPT_Wuseless_cast, "useless cast to type %q#T", type);
}
}
Index: gcc/cp/error.c
===
--- gcc/cp/error.c(revision 247394)
+++ gcc/cp/error.c(working copy)
@@ -3134,10 +3134,6 @@
if (len == aka_len && memcmp (p, p+aka_start, len) == 0)
p[len] = '\0';
}
-
- if (typ && TYPE_P (typ) && TREE_CODE (typ) == ENUMERAL_TYPE)
-pp_string (cxx_pp, M_(" {enum}"));
-
return pp_ggc_formatted_text (cxx_pp);
}
===
2017-04-30 Volker Reichelt
* g++.dg/cpp1z/direct-enum-init1.C: Rever special enum handling.
* g++.dg/warn/pr12242.C: Likewise.
Index: gcc/testsuite/g++.dg/cpp1z/direct-enum-init1.C
===
--- gcc/testsuite/g++.dg/cpp1z/direct-enum-init1.C(revision 247394)
+++ gcc/testsuite/g++.dg/cpp1z/direct-enum-init1.C(working copy)
@@ -17,67 +17,67 @@
void
foo ()
{
- A a1 { 5 };// { dg-error "invalid conversion from 'int' to
'A {enum}'" }
- B b1 { 7 };// { dg-error "invalid conversion from 'int' to
'B {enum}'" "" { target c++14_down } }
+ A a1 { 5 };// { dg-error "invalid conversion from 'int' to
'A'" }
+ B b1 { 7 };// { dg-error "invalid conversion from 'int' to
'B'" "" { target c++14_down } }
C c1 { s };
- D d1 { D(t) };// { dg-error "invalid cast from type 'T' to type
'D {enum}'" }
- D d2 { t };// { dg-error "cannot convert 'T' to 'D {enum}'
in initialization" "" { target c++14_down } }
+ D d1 { D(t) };// { dg-error "invalid cast from type 'T' to type
'D'" }
+ D d2 { t };// { dg-error "cannot convert 'T' to 'D' in
initialization" "" { target c++14_down } }
// { dg-error "invalid cast from type 'T' to type 'D'" ""
{ target c++1z } .-1 }
- D d3 { 9 };// { dg-error "cannot
Re: [PATCH] Improved diagnostics for casts and enums
So here's the patch that reverts the special enum handling in
type_to_string and uses %q#T instead of %qT for two casting-related
diagnostics.
Bootstrapped and regtested on x86_64-pc-linux-gnu.
OK for trunk?
The "E {enum}'" notation is still on trunk so it seems that this
patch has never been committed and I can't find approval of it
in the archive. To make sure it doesn't get forgotten, please
consider this a ping on Volker's behalf:
https://gcc.gnu.org/ml/gcc-patches/2017-04/msg01576.html
Thanks
Martin
As one can see from the testsuite changes, there are several
casting- and conversion-related messages like "invalid conversion from",
"cannot convert", "invalid cast" that still use the simple %qT
form. I'll give it a try to use %q#T there as well and prepare a
separate patch if this really improves the diagnostics.
Regards,
Volker
2017-04-30 Volker Reichelt
* parser.c (cp_parser_cast_expression): Use %q#T instead of %qT
in old-style cast diagnostic.
* typeck.c (maybe_warn_about_useless_cast): Use %q#T instead of %qT
in useless cast diagnostic.
* error.c (type_to_string): Remove enum special handling.
Index: gcc/cp/parser.c
===
--- gcc/cp/parser.c (revision 247394)
+++ gcc/cp/parser.c (working copy)
@@ -8764,7 +8764,7 @@
&& !VOID_TYPE_P (type)
&& current_lang_name != lang_name_c)
warning (OPT_Wold_style_cast,
-"use of old-style cast to %qT", type);
+"use of old-style cast to %q#T", type);
/* Only type conversions to integral or enumeration types
can be used in constant-expressions. */
Index: gcc/cp/typeck.c
===
--- gcc/cp/typeck.c (revision 247394)
+++ gcc/cp/typeck.c (working copy)
@@ -6631,7 +6631,7 @@
? xvalue_p (expr) : lvalue_p (expr))
&& same_type_p (TREE_TYPE (expr), TREE_TYPE (type)))
|| same_type_p (TREE_TYPE (expr), type))
- warning (OPT_Wuseless_cast, "useless cast to type %qT", type);
+ warning (OPT_Wuseless_cast, "useless cast to type %q#T", type);
}
}
Index: gcc/cp/error.c
===
--- gcc/cp/error.c (revision 247394)
+++ gcc/cp/error.c (working copy)
@@ -3134,10 +3134,6 @@
if (len == aka_len && memcmp (p, p+aka_start, len) == 0)
p[len] = '\0';
}
-
- if (typ && TYPE_P (typ) && TREE_CODE (typ) == ENUMERAL_TYPE)
-pp_string (cxx_pp, M_(" {enum}"));
-
return pp_ggc_formatted_text (cxx_pp);
}
===
2017-04-30 Volker Reichelt
* g++.dg/cpp1z/direct-enum-init1.C: Rever special enum handling.
* g++.dg/warn/pr12242.C: Likewise.
Index: gcc/testsuite/g++.dg/cpp1z/direct-enum-init1.C
===
--- gcc/testsuite/g++.dg/cpp1z/direct-enum-init1.C (revision 247394)
+++ gcc/testsuite/g++.dg/cpp1z/direct-enum-init1.C (working copy)
@@ -17,67 +17,67 @@
void
foo ()
{
- A a1 { 5 }; // { dg-error "invalid conversion from 'int' to 'A
{enum}'" }
- B b1 { 7 }; // { dg-error "invalid conversion from 'int' to 'B {enum}'"
"" { target c++14_down } }
+ A a1 { 5 }; // { dg-error "invalid conversion from 'int' to 'A'" }
+ B b1 { 7 }; // { dg-error "invalid conversion from 'int' to 'B'" ""
{ target c++14_down } }
C c1 { s };
- D d1 { D(t) }; // { dg-error "invalid cast from type 'T' to type 'D
{enum}'" }
- D d2 { t }; // { dg-error "cannot convert 'T' to 'D {enum}' in
initialization" "" { target c++14_down } }
+ D d1 { D(t) }; // { dg-error "invalid cast from type 'T' to type 'D'" }
+ D d2 { t }; // { dg-error "cannot convert 'T' to 'D' in initialization"
"" { target c++14_down } }
// { dg-error "invalid cast from type 'T' to type 'D'"
"" { target c++1z } .-1 }
- D d3 { 9 }; // { dg-error "cannot convert 'int' to 'D {enum}' in
initialization" "" { target c++14_down } }
- D d4 { l }; // { dg-error "cannot convert 'long int' to 'D {enum}' in
initialization" "" { target c++14_down } }
+ D d3 { 9 }; // { dg-error "cannot convert 'int' to 'D' in initialization"
"" { target c++14_down } }
+ D d4 { l }; // { dg-error "cannot convert 'long int' to 'D' in
initialization" "" { target c++14_down } }
D d5 { D(l) };
- D d6 { G }; // { dg-error "cannot convert 'A {enum}' to 'D {enum}' in
initialization" "" { target c++14_down } }
- E e1 { 5 }; // { dg-error "cannot convert 'int' to 'E {enum}' in
initialization" "" { target c++14_down } }
- E e2 { -1 }; // { dg-error
Re: [PATCH] Improved diagnostics for casts and enums
On 28 Apr, Volker Reichelt wrote:
> 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
So here's the patch that reverts the special enum handling in
type_to_string and uses %q#T instead of %qT for two casting-related
diagnostics.
Bootstrapped and regtested on x86_64-pc-linux-gnu.
OK for trunk?
As one can see from the testsuite changes, there are several
casting- and conversion-related messages like "invalid conversion from",
"cannot convert", "invalid cast" that still use the simple %qT
form. I'll give it a try to use %q#T there as well and prepare a
separate patch if this really improves the diagnostics.
Regards,
Volker
2017-04-30 Volker Reichelt
* parser.c (cp_parser_cast_expression): Use %q#T instead of %qT
in old-style cast diagnostic.
* typeck.c (maybe_warn_about_useless_cast): Use %q#T instead of %qT
in useless cast diagnostic.
* error.c (type_to_string): Remove enum special handling.
Index: gcc/cp/parser.c
===
--- gcc/cp/parser.c (revision 247394)
+++ gcc/cp/parser.c (working copy)
@@ -8764,7 +8764,7 @@
&& !VOID_TYPE_P (type)
&& current_lang_name != lang_name_c)
warning (OPT_Wold_style_cast,
-"use of old-style cast to %qT", type);
+"use of old-style cast to %q#T", type);
/* Only type conversions to integral or enumeration types
can be used in constant-expressions. */
Index: gcc/cp/typeck.c
===
--- gcc/cp/typeck.c (revision 247394)
+++ gcc/cp/typeck.c (working copy)
@@ -6631,7 +6631,7 @@
? xvalue_p (expr) : lvalue_p (expr))
&& same_type_p (TREE_TYPE (expr), TREE_TYPE (type)))
|| same_type_p (TREE_TYPE (expr), type))
- warning (OPT_Wuseless_cast, "useless cast to type %qT", type);
+ warning (OPT_Wuseless_cast, "useless cast to type %q#T", type);
}
}
Index: gcc/cp/error.c
===
--- gcc/cp/error.c (revision 247394)
+++ gcc/cp/error.c (working copy)
@@ -3134,10 +3134,6 @@
if (len == aka_len && memcmp (p, p+aka_start, len) == 0)
p[len] = '\0';
}
-
- if (typ && TYPE_P (typ) && TREE_CODE (typ) == ENUMERAL_TYPE)
-pp_string (cxx_pp, M_(" {enum}"));
-
Re: [PATCH] Improved diagnostics for casts and enums
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
Re: [PATCH] Improved diagnostics for casts and enums
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.
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
Re: [PATCH] Improved diagnostics for casts and enums
On 04/27/2017 03: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.
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).
Bootstrapped and regtested on x86_64-pc-linux-gnu.
OK for trunk (as two patches or as one)?
ok. One commit is fine.
nathan
--
Nathan Sidwell
[PATCH] Improved diagnostics for casts and enums
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.
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).
Bootstrapped and regtested on x86_64-pc-linux-gnu.
OK for trunk (as two patches or as one)?
Regards,
Volker
2017-04-27 Volker Reichelt
* parser.c (cp_parser_cast_expression): Add target type of cast to
diagnostic.
Index: gcc/cp/parser.c
===
--- gcc/cp/parser.c (revision 247283)
+++ gcc/cp/parser.c (working copy)
@@ -8763,7 +8763,8 @@
&& !in_system_header_at (input_location)
&& !VOID_TYPE_P (type)
&& current_lang_name != lang_name_c)
- warning (OPT_Wold_style_cast, "use of old-style cast");
+ warning (OPT_Wold_style_cast,
+"use of old-style cast to %qT", type);
/* Only type conversions to integral or enumeration types
can be used in constant-expressions. */
===
2017-04-27 Volker Reichelt
* error.c (type_to_string): Add '{enum}' suffix to enumeration types.
Index: gcc/cp/error.c
===
--- gcc/cp/error.c (revision 247283)
+++ gcc/cp/error.c (working copy)
@@ -3134,6 +3134,10 @@
if (len == aka_len && memcmp (p, p+aka_start, len) == 0)
p[len] = '\0';
}
+
+ if (typ && TYPE_P (typ) && TREE_CODE (typ) == ENUMERAL_TYPE)
+pp_string (cxx_pp, M_(" {enum}"));
+
return pp_ggc_formatted_text (cxx_pp);
}
===
2017-04-27 Volker Reichelt
* g++.dg/cpp1z/direct-enum-init1.C: Adjust for more verbose enum
diagnostics.
* g++.dg/warn/pr12242.C: Likewise.
Index: gcc/testsuite/g++.dg/cpp1z/direct-enum-init1.C
===
--- gcc/testsuite/g++.dg/cpp1z/direct-enum-init1.C (revision 247283)
+++ gcc/testsuite/g++.dg/cpp1z/direct-enum-init1.C (working copy)
@@ -17,67 +17,67 @@
void
foo ()
{
- A a1 { 5 }; // { dg-error "invalid conversion from 'int' to 'A'" }
- B b1 { 7 }; // { dg-error "invalid conversion from 'int' to 'B'" ""
{ target c++14_down } }
+ A a1 { 5 }; // { dg-error "invalid conversion from 'int' to 'A
{enum}'" }
+ B b1 { 7 }; // { dg-error "invalid conversion from 'int' to 'B
{enum}'" "" { target c++14_down } }
C c1 { s };
- D d1 { D(t) }; // { dg-error "invalid cast from type 'T' to type 'D'" }
- D d2 { t }; // { dg-error "cannot convert 'T' to 'D' in
initialization" "" { target c++14_down } }
+ D d1 { D(t) }; // { dg-error "invalid cast from type 'T' to type 'D
{enum}'" }
+ D d2 { t }; // { dg-error "cannot convert 'T' to 'D {enum}' in
initialization" "" { target c++14_down } }
// { dg-error "invalid cast from type 'T' to type 'D'"
"" { target c++1z } .-1 }
- D d3 { 9 }; // { dg-error "cannot convert 'int' to 'D' in
initialization" "" { target c++14_down } }
- D d4 { l }; // { dg-error "cannot convert 'long int' to 'D' in
initialization" "" { target c++14_down } }
+ D d3 { 9 }; // { dg-error "cannot convert 'int' to 'D {enum}' in
initialization" "" { target c++14_down } }
+ D d4 { l }; // { dg-error "cannot convert 'long int' to 'D {enum}'
in initialization" "" { target c++14_down } }
D d5 { D(l) };
- D d6 { G }; // { dg-error "cannot convert 'A' to 'D' in
initialization" "" { target c++14_down } }
- E e1 { 5 }; // { dg-error "cannot convert 'int' to 'E' in
initialization" "" { target c++14_down } }
- E e2 { -1 }; // { dg-error "cannot convert 'int' to 'E' in
initialization" "" { target c++14_down } }
+ D d6 { G }; // { dg-error "cannot convert 'A {enum}' to 'D {enum}'
in initialization" "" { target c++14_down } }
+ E e1 { 5 }; // { dg-error "cannot convert 'int' to 'E {enum}' in
initialization" "" { target c++14_down } }
+ E e2 { -1 }; // { dg-error "cannot
