On 2/19/20 5:09 PM, Jason Merrill wrote:
On 2/19/20 1:02 AM, Martin Sebor wrote:
PR 93804 points out that issuing -Wredundant-tags for declarations
in C headers included in C++ isn't helpful because the tags cannot
be removed without breaking C programs that depend on the headers.
Attached is a patch that avoids the warning in these cases tested
on x86_64-linux. While strictly not a regression (and even though
I initially considered it a GCC 11 enhancement), since most C++
code includes some C headers, without the patch the warning would
likely cause too much noise to be widely useful.
+ const line_map_ordinary *map = NULL;
+ linemap_resolve_location (line_table, key_loc,
+ LRK_MACRO_DEFINITION_LOCATION,
+ &map);
+ if (!MAIN_FILE_P (map))
+ key_redundant = false;
Checking which file it came from seems like unnecessary complication; is
it important to still warn in extern "C" blocks in the main source file?
It's only important if someone is relying on it to avoid the redundant
tags in all their C++ code, e.g., as part of cleaning up -Wmismatched-
tags. The latter will complain about mismatches in extern "C" blocks
and suggest either dropping the tag or replacing it, whichever is
appropriate. I'd view it as a bug if -Wredundant-tags didn't do
the same since that's its one and only job.
I attach a slightly revised patch that also handles enums (as pointed
out by Stephan), and with beefed up tests. Retested on x86_64-linux.
If you find the linemap code distracting, or even the warning code,
I can factor it out and into a helper function.
Martin
PR c++/93804 - exempt extern C headers from -Wredundant-tags
gcc/cp/ChangeLog:
PR c++/93804
* parser.c (cp_parser_check_class_key): Avoid issuing -Wredundant-tags
in shared C/C++ code in headers.
gcc/testsuite/ChangeLog:
PR c++/93804
* g++.dg/warn/Wredundant-tags-4.C: New test.
* g++.dg/warn/Wredundant-tags-5.C: New test.
* g++.dg/warn/Wredundant-tags-5.h: New test.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index ee534b5db21..21ce05ea05a 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -30826,15 +30826,31 @@ cp_parser_maybe_warn_enum_key (cp_parser *parser, location_t key_loc,
/* The enum-key is redundant for uses of the TYPE that are not
declarations and for which name lookup returns just the type
itself. */
- if (decl == type_decl)
- {
- gcc_rich_location richloc (key_loc);
- richloc.add_fixit_remove (key_loc);
- warning_at (&richloc, OPT_Wredundant_tags,
- "redundant enum-key %<enum%s%> in reference to %q#T",
- (scoped_key == RID_CLASS ? " class"
- : scoped_key == RID_STRUCT ? " struct" : ""), type);
+ if (decl != type_decl)
+ return;
+
+ if (scoped_key != RID_CLASS
+ && scoped_key != RID_STRUCT
+ && current_lang_name != lang_name_cplusplus
+ && current_namespace == global_namespace)
+ {
+ /* Avoid issuing the diagnostic for apparently redundant (unscoped)
+ enum tag in shared C/C++ code in files (such as headers) included
+ in the main source file. */
+ const line_map_ordinary *map = NULL;
+ linemap_resolve_location (line_table, key_loc,
+ LRK_MACRO_DEFINITION_LOCATION,
+ &map);
+ if (!MAIN_FILE_P (map))
+ return;
}
+
+ gcc_rich_location richloc (key_loc);
+ richloc.add_fixit_remove (key_loc);
+ warning_at (&richloc, OPT_Wredundant_tags,
+ "redundant enum-key %<enum%s%> in reference to %q#T",
+ (scoped_key == RID_CLASS ? " class"
+ : scoped_key == RID_STRUCT ? " struct" : ""), type);
}
/* Describes the set of declarations of a struct, class, or class template
@@ -30995,6 +31011,13 @@ cp_parser_check_class_key (cp_parser *parser, location_t key_loc,
tree decl = cp_parser_lookup_name_simple (parser, name, input_location);
pop_deferring_access_checks ();
+ /* Only consider the true class-keys below and ignore typename_type,
+ etc. that are not C++ class-keys. */
+ if (class_key != class_type
+ && class_key != record_type
+ && class_key != union_type)
+ return;
+
/* Only consider the true class-keys below and ignore typename_type,
etc. that are not C++ class-keys. */
if (class_key != class_type
@@ -31006,15 +31029,32 @@ cp_parser_check_class_key (cp_parser *parser, location_t key_loc,
neither definitions of it nor declarations, and for which name
lookup returns just the type itself. */
bool key_redundant = !def_p && !decl_p && decl == type_decl;
+
+ if (key_redundant
+ && class_key != class_type
+ && current_lang_name != lang_name_cplusplus
+ && current_namespace == global_namespace)
+ {
+ /* Avoid issuing the diagnostic for apparently redundant struct
+ and union class-keys in shared C/C++ code in files (such as
+ headers) included in the main source file. */
+ const line_map_ordinary *map = NULL;
+ linemap_resolve_location (line_table, key_loc,
+ LRK_MACRO_DEFINITION_LOCATION,
+ &map);
+ if (!MAIN_FILE_P (map))
+ key_redundant = false;
+ }
+
if (key_redundant)
{
gcc_rich_location richloc (key_loc);
richloc.add_fixit_remove (key_loc);
warning_at (&richloc, OPT_Wredundant_tags,
- "redundant class-key %qs in reference to %q#T",
- class_key == union_type ? "union"
- : class_key == record_type ? "struct" : "class",
- type);
+ "redundant class-key %qs in reference to %q#T",
+ class_key == union_type ? "union"
+ : class_key == record_type ? "struct" : "class",
+ type);
}
if (seen_as_union || !warn_mismatched_tags)
diff --git a/gcc/testsuite/g++.dg/warn/Wredundant-tags-4.C b/gcc/testsuite/g++.dg/warn/Wredundant-tags-4.C
new file mode 100644
index 00000000000..c56153aaf71
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wredundant-tags-4.C
@@ -0,0 +1,142 @@
+/* PR c++/93804 - exempt extern "C" headers from -Wredundant-tags
+ Verify that -Wredundant-tags is not issued for redundant class-key
+ in extern "C" references in a header file.
+ { dg-do compile }
+ { dg-options "-Wredundant-tags -ftrack-macro-expansion=0" } */
+
+# 1 "Wredundant-tags-4.C"
+# 1 "Wredundant-tags-4.h" 1
+# line 10
+
+#if __cplusplus >= 201103L
+# define enum_struct enum struct
+#else
+# define enum_struct class
+#endif
+
+extern "C" {
+
+ class C1 { };
+ enum E1 { };
+ enum_struct ES1 { };
+ struct S1 { enum E1 e1; };
+ union U1 { enum E1 e1; struct S1 s1; };
+
+ /* The warning should be issued for the class-key class even in
+ an extern "C" block. */
+ void f0 (class C1); // { dg-warning "\\\[-Wredundant-tags" }
+ void f1 (enum E1); // { dg-bogus "\\\[-Wredundant-tags" }
+
+ /* Ditto for a scoped enum. */
+ void f2 (enum_struct ES1); // { dg-warning "\\\[-Wredundant-tags" }
+ // { dg-warning "must not use the 'struct' keyword" "enum struct" { target { c++11 } } .-1 }
+
+ void f3 (struct S1); // { dg-bogus "\\\[-Wredundant-tags" }
+ void f4 (union U1); // { dg-bogus "\\\[-Wredundant-tags" }
+
+ inline int
+ finline1 (class C1) // { dg-warning "\\\[-Wredundant-tags" }
+ { return sizeof (class C1); } // { dg-warning "\\\[-Wredundant-tags" }
+
+ inline int
+ finline2 (enum E1) // { dg-bogus "\\\[-Wredundant-tags" }
+ { return sizeof (enum E1); } // { dg-bogus "\\\[-Wredundant-tags" }
+
+ inline int
+ finline3 (enum_struct ES1) // { dg-warning "\\\[-Wredundant-tags" }
+ { return sizeof (ES1); }
+
+ inline int
+ finline4 (struct S1) // { dg-bogus "\\\[-Wredundant-tags" }
+ { return sizeof (struct S1); }
+
+ inline int
+ finline5 (union U1) // { dg-bogus "\\\[-Wredundant-tags" }
+ { return sizeof (union U1); }
+
+ extern class C1 c1; // { dg-warning "\\\[-Wredundant-tags" }
+ extern enum E1 e1; // { dg-bogus "\\\[-Wredundant-tags" }
+ extern enum_struct ES1 es1; // { dg-warning "\\\[-Wredundant-tags" }
+ extern struct S1 s1; // { dg-bogus "\\\[-Wredundant-tags" }
+ extern union U1 u1; // { dg-bogus "\\\[-Wredundant-tags" }
+
+ namespace N1 {
+ /* Verify that -Wredundant-tags is issued in a namespace enclosed
+ in an extern "C" block. (Such code cannot be shared with C.) */
+ extern class C1 n1c1; // { dg-warning "\\\[-Wredundant-tags" }
+ extern enum E1 n1e1; // { dg-warning "\\\[-Wredundant-tags" }
+ extern enum_struct ES1 n1es1; // { dg-warning "\\\[-Wredundant-tags" }
+ extern struct S1 n1s1; // { dg-warning "\\\[-Wredundant-tags" }
+ extern union U1 n1u1; // { dg-warning "\\\[-Wredundant-tags" }
+ }
+} // extern "C"
+
+
+extern "C++" {
+
+ class C2 { };
+ enum E2 { };
+ enum_struct ES2 { };
+ struct S2 {
+ enum E2 e21; // { dg-warning "\\\[-Wredundant-tags" }
+ E2 e22;
+ enum_struct ES2 es21; // { dg-warning "\\\[-Wredundant-tags" }
+ ES2 es22;
+ };
+ union U2 { };
+
+ void f5 (class C2); // { dg-warning "\\\[-Wredundant-tags" }
+ void f6 (enum E2); // { dg-warning "\\\[-Wredundant-tags" }
+ void f7 (enum_struct ES2); // { dg-warning "\\\[-Wredundant-tags" }
+ void f8 (struct S2); // { dg-warning "\\\[-Wredundant-tags" }
+ void f9 (union U2); // { dg-warning "\\\[-Wredundant-tags" }
+
+ extern class C2 c2; // { dg-warning "\\\[-Wredundant-tags" }
+ extern enum E2 e2; // { dg-warning "\\\[-Wredundant-tags" }
+ extern enum_struct ES2 es2; // { dg-warning "\\\[-Wredundant-tags" }
+ extern struct S2 s2; // { dg-warning "\\\[-Wredundant-tags" }
+ extern union U2 u2; // { dg-warning "\\\[-Wredundant-tags" }
+} // extern "C++"
+
+
+namespace N {
+
+class C3 { };
+enum E3 { };
+enum_struct ES3 { };
+struct S3 { };
+union U3 { };
+
+void f10 (class C3); // { dg-warning "\\\[-Wredundant-tags" }
+void f11 (enum E3); // { dg-warning "\\\[-Wredundant-tags" }
+void f12 (enum_struct ES3); // { dg-warning "\\\[-Wredundant-tags" }
+void f13 (struct S3); // { dg-warning "\\\[-Wredundant-tags" }
+void f14 (union U3); // { dg-warning "\\\[-Wredundant-tags" }
+
+extern class C3 c3; // { dg-warning "\\\[-Wredundant-tags" }
+extern enum E3 e3; // { dg-warning "\\\[-Wredundant-tags" }
+extern enum_struct ES3 es3; // { dg-warning "\\\[-Wredundant-tags" }
+extern struct S3 s3; // { dg-warning "\\\[-Wredundant-tags" }
+extern union U3 u3; // { dg-warning "\\\[-Wredundant-tags" }
+
+extern "C" {
+
+ /* Verify that -Wredundant-tags is issued in an extern "C" block
+ enclosed within a namespace. (Such code cannot be shared with
+ C.) */
+ class C4 { };
+ enum E4 { };
+ enum_struct ES4 { };
+ struct S4 { };
+ union U4 { };
+
+ extern class C4 c4; // { dg-warning "\\\[-Wredundant-tags" }
+ extern enum E4 e4; // { dg-warning "\\\[-Wredundant-tags" }
+ extern enum_struct ES4 es4; // { dg-warning "\\\[-Wredundant-tags" }
+ extern struct S4 s4; // { dg-warning "\\\[-Wredundant-tags" }
+ extern union U4 u4; // { dg-warning "\\\[-Wredundant-tags" }
+}
+
+} // namespace N
+
+// { dg-prune-output "must not use the 'struct' keyword" }
diff --git a/gcc/testsuite/g++.dg/warn/Wredundant-tags-5.C b/gcc/testsuite/g++.dg/warn/Wredundant-tags-5.C
new file mode 100644
index 00000000000..a3676d8d070
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wredundant-tags-5.C
@@ -0,0 +1,109 @@
+// PR c++/93804 - exempt extern "C" headers from -Wredundant-tags
+// Verify that -Wredundant-tags is issued even for redundant class-key
+// in references in the main source file to extern "C" classes defined
+// in headers.
+// { dg-do compile }
+// { dg-options "-Wredundant-tags -ftrack-macro-expansion=0" }
+
+#include "Wredundant-tags-5.h"
+
+extern "C" {
+
+ class C1 // { dg-warning "\\\[-Wredundant-tags" }
+ fc1 (C1)
+ {
+ return C1 ();
+ }
+
+ EC1
+ fce1 (enum_class EC1) // { dg-warning "\\\[-Wredundant-tags" }
+ {
+ return EC1 ();
+ }
+
+ E1
+ fe1 (E1)
+ {
+ return (enum E1)0; // { dg-warning "\\\[-Wredundant-tags" }
+ }
+
+ struct S1 // { dg-warning "\\\[-Wredundant-tags" }
+ fs1 (S1)
+ {
+ return S1 ();
+ }
+
+ U1
+ fu1 (union U1) // { dg-warning "\\\[-Wredundant-tags" }
+ {
+ return U1 ();
+ }
+
+} // extern "C"
+
+
+extern "C++" {
+
+ class C2 // { dg-warning "\\\[-Wredundant-tags" }
+ fc2 (C2)
+ {
+ return C2 ();
+ }
+
+ EC2
+ fce2 (enum_class EC2) // { dg-warning "\\\[-Wredundant-tags" }
+ {
+ return EC2 ();
+ }
+
+ E2
+ fe2 (E2)
+ {
+ return (enum E2)0; // { dg-warning "\\\[-Wredundant-tags" }
+ }
+
+ struct S2 // { dg-warning "\\\[-Wredundant-tags" }
+ fs2 (S2)
+ {
+ return S2 ();
+ }
+
+ U2
+ fu2 (union U2) // { dg-warning "\\\[-Wredundant-tags" }
+ {
+ return U2 ();
+ }
+
+} // extern "C++"
+
+
+class C3 // { dg-warning "\\\[-Wredundant-tags" }
+fc3 (C3)
+{
+ return C3 ();
+}
+
+EC3
+fce3 (enum_class EC3) // { dg-warning "\\\[-Wredundant-tags" }
+{
+ return EC3 ();
+}
+
+E3 fe3 (E3)
+{
+ return (enum E3)0; // { dg-warning "\\\[-Wredundant-tags" }
+}
+
+struct S3 // { dg-warning "\\\[-Wredundant-tags" }
+fs3 (S3)
+{
+ return S3 ();
+}
+
+U3
+fu3 (union U3) // { dg-warning "\\\[-Wredundant-tags" }
+{
+ return U3 ();
+}
+
+// { dg-prune-output "must not use the 'class' keyword" }
diff --git a/gcc/testsuite/g++.dg/warn/Wredundant-tags-5.h b/gcc/testsuite/g++.dg/warn/Wredundant-tags-5.h
new file mode 100644
index 00000000000..c72aee6e01f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wredundant-tags-5.h
@@ -0,0 +1,80 @@
+#ifndef WREDUNDANT_TAGS_H
+#define WREDUNDANT_TAGS_H
+
+#if __cplusplus >= 201103L
+# define enum_class enum class
+#else
+# define enum_class class
+#endif
+
+extern "C" {
+
+ class C1 { };
+ enum_class EC1 { };
+ enum E1 { };
+ struct S1 { };
+ union U1 { };
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wredundant-tags"
+ class C1 fc1 (class C1); // -Wredundant-tags
+ enum_class EC1 fce1 (enum_class EC1);
+#pragma GCC diagnostic pop
+
+ enum E1 fe1 (enum E1);
+ struct S1 fs1 (struct S1);
+ union U1 fu1 (union U1);
+
+ C1 fc1 (C1);
+ EC1 fce1 (EC1);
+ E1 fe1 (E1);
+ S1 fs1 (S1);
+ U1 fu1 (U1);
+}
+
+
+extern "C++" {
+
+ class C2 { };
+ enum_class EC2 { };
+ enum E2 { };
+ struct S2 { };
+ union U2 { };
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wredundant-tags"
+ class C2 fc2 (class C2); // -Wredundant-tags
+ enum_class EC2 fce2 (enum_class EC2); // -Wredundant-tags
+ struct S2 fs2 (struct S2); // -Wredundant-tags
+ union U2 fu2 (union U2); // -Wredundant-tags
+#pragma GCC diagnostic pop
+
+ C2 fc2 (C2);
+ EC2 fce2 (EC2);
+ E2 fe2 (E2);
+ S2 fs2 (S2);
+ U2 fu2 (U2);
+}
+
+
+class C3 { };
+enum_class EC3 { };
+enum E3 { };
+struct S3 { };
+union U3 { };
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wredundant-tags"
+class C3 fc3 (class C3); // -Wredundant-tags
+enum_class EC3 fce3 (enum_class EC3); // -Wredundant-tags
+struct S3 fs3 (struct S3); // -Wredundant-tags
+union U3 fu3 (union U3); // -Wredundant-tags
+#pragma GCC diagnostic pop
+
+C3 fc3 (C3);
+EC3 fce3 (EC3);
+E3 fe3 (E3);
+S3 fs3 (S3);
+U3 fu3 (U3);
+
+#endif // WREDUNDANT_TAGS_H
