On 2/20/20 5:55 PM, Martin Sebor wrote:
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.
+ && 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;
This much is common between the enum and class functions and could be
factored out, but I don't feel strongly about it. Also, why
LRK_MACRO_DEFINITION_LOCATION rather than LRK_SPELLING_LOCATION?
+ /* 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
This looks like a rebase glitch adding the same code again. OK without
this hunk.
Jason
