r348233 - Fix -Wmismatched-tags to not warn on redeclarations of structs in system
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 3 18:45:29 PST 2018
Author: rsmith
Date: Mon Dec 3 18:45:28 2018
New Revision: 348233
URL: http://llvm.org/viewvc/llvm-project?rev=348233&view=rev
Log:
Fix -Wmismatched-tags to not warn on redeclarations of structs in system
headers.
Previously, we would only check whether the new declaration is in a
system header, but that requires the user to be able to correctly guess
whether a declaration in a system header is declared as a struct or a
class when specializing standard library traits templates.
We now entirely ignore declarations for which the warning was disabled
when determining whether to warn on a tag mismatch.
Also extend the diagnostic message to clarify that
a) code containing such a tag mismatch is in fact valid and correct,
and
b) the (non-coding-style) reason to emit such a warning is that the
Microsoft C++ ABI is broken and includes the tag kind in decorated
names,
as it seems a lot of users are confused by our diagnostic here (either
not understanding why we produce it, or believing that it represents an
actual language rule).
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/SemaCXX/struct-class-redecl.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=348233&r1=348232&r2=348233&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Dec 3 18:45:28 2018
@@ -4806,16 +4806,18 @@ def err_nested_redefinition : Error<"nes
def err_use_with_wrong_tag : Error<
"use of %0 with tag type that does not match previous declaration">;
def warn_struct_class_tag_mismatch : Warning<
- "%select{struct|interface|class}0%select{| template}1 %2 was previously "
- "declared as a %select{struct|interface|class}3%select{| template}1">,
- InGroup<MismatchedTags>, DefaultIgnore;
+ "%select{struct|interface|class}0%select{| template}1 %2 was previously "
+ "declared as a %select{struct|interface|class}3%select{| template}1; "
+ "this is valid, but may result in linker errors under the Microsoft C++ ABI">,
+ InGroup<MismatchedTags>, DefaultIgnore;
def warn_struct_class_previous_tag_mismatch : Warning<
- "%2 defined as %select{a struct|an interface|a class}0%select{| template}1 "
- "here but previously declared as "
- "%select{a struct|an interface|a class}3%select{| template}1">,
- InGroup<MismatchedTags>, DefaultIgnore;
+ "%2 defined as %select{a struct|an interface|a class}0%select{| template}1 "
+ "here but previously declared as "
+ "%select{a struct|an interface|a class}3%select{| template}1; "
+ "this is valid, but may result in linker errors under the Microsoft C++ ABI">,
+ InGroup<MismatchedTags>, DefaultIgnore;
def note_struct_class_suggestion : Note<
- "did you mean %select{struct|interface|class}0 here?">;
+ "did you mean %select{struct|interface|class}0 here?">;
def ext_forward_ref_enum : Extension<
"ISO C forbids forward references to 'enum' types">;
def err_forward_ref_enum : Error<
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=348233&r1=348232&r2=348233&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Dec 3 18:45:28 2018
@@ -13803,76 +13803,106 @@ bool Sema::isAcceptableTagRedeclaration(
// struct class-key shall be used to refer to a class (clause 9)
// declared using the class or struct class-key.
TagTypeKind OldTag = Previous->getTagKind();
- if (!isDefinition || !isClassCompatTagKind(NewTag))
- if (OldTag == NewTag)
+ if (OldTag != NewTag &&
+ !(isClassCompatTagKind(OldTag) && isClassCompatTagKind(NewTag)))
+ return false;
+
+ // Tags are compatible, but we might still want to warn on mismatched tags.
+ // Non-class tags can't be mismatched at this point.
+ if (!isClassCompatTagKind(NewTag))
+ return true;
+
+ // Declarations for which -Wmismatched-tags is disabled are entirely ignored
+ // by our warning analysis. We don't want to warn about mismatches with (eg)
+ // declarations in system headers that are designed to be specialized, but if
+ // a user asks us to warn, we should warn if their code contains mismatched
+ // declarations.
+ auto IsIgnoredLoc = [&](SourceLocation Loc) {
+ return getDiagnostics().isIgnored(diag::warn_struct_class_tag_mismatch,
+ Loc);
+ };
+ if (IsIgnoredLoc(NewTagLoc))
+ return true;
+
+ auto IsIgnored = [&](const TagDecl *Tag) {
+ return IsIgnoredLoc(Tag->getLocation());
+ };
+ while (IsIgnored(Previous)) {
+ Previous = Previous->getPreviousDecl();
+ if (!Previous)
return true;
+ OldTag = Previous->getTagKind();
+ }
- if (isClassCompatTagKind(OldTag) && isClassCompatTagKind(NewTag)) {
- // Warn about the struct/class tag mismatch.
- bool isTemplate = false;
- if (const CXXRecordDecl *Record = dyn_cast<CXXRecordDecl>(Previous))
- isTemplate = Record->getDescribedClassTemplate();
+ bool isTemplate = false;
+ if (const CXXRecordDecl *Record = dyn_cast<CXXRecordDecl>(Previous))
+ isTemplate = Record->getDescribedClassTemplate();
- if (inTemplateInstantiation()) {
+ if (inTemplateInstantiation()) {
+ if (OldTag != NewTag) {
// In a template instantiation, do not offer fix-its for tag mismatches
// since they usually mess up the template instead of fixing the problem.
Diag(NewTagLoc, diag::warn_struct_class_tag_mismatch)
<< getRedeclDiagFromTagKind(NewTag) << isTemplate << Name
<< getRedeclDiagFromTagKind(OldTag);
- return true;
+ // FIXME: Note previous location?
}
+ return true;
+ }
- if (isDefinition) {
- // On definitions, check previous tags and issue a fix-it for each
- // one that doesn't match the current tag.
- if (Previous->getDefinition()) {
- // Don't suggest fix-its for redefinitions.
- return true;
- }
-
- bool previousMismatch = false;
- for (auto I : Previous->redecls()) {
- if (I->getTagKind() != NewTag) {
- if (!previousMismatch) {
- previousMismatch = true;
- Diag(NewTagLoc, diag::warn_struct_class_previous_tag_mismatch)
- << getRedeclDiagFromTagKind(NewTag) << isTemplate << Name
- << getRedeclDiagFromTagKind(I->getTagKind());
- }
- Diag(I->getInnerLocStart(), diag::note_struct_class_suggestion)
- << getRedeclDiagFromTagKind(NewTag)
- << FixItHint::CreateReplacement(I->getInnerLocStart(),
- TypeWithKeyword::getTagTypeKindName(NewTag));
- }
- }
+ if (isDefinition) {
+ // On definitions, check all previous tags and issue a fix-it for each
+ // one that doesn't match the current tag.
+ if (Previous->getDefinition()) {
+ // Don't suggest fix-its for redefinitions.
return true;
}
- // Check for a previous definition. If current tag and definition
- // are same type, do nothing. If no definition, but disagree with
- // with previous tag type, give a warning, but no fix-it.
- const TagDecl *Redecl = Previous->getDefinition() ?
- Previous->getDefinition() : Previous;
- if (Redecl->getTagKind() == NewTag) {
- return true;
+ bool previousMismatch = false;
+ for (const TagDecl *I : Previous->redecls()) {
+ if (I->getTagKind() != NewTag) {
+ // Ignore previous declarations for which the warning was disabled.
+ if (IsIgnored(I))
+ continue;
+
+ if (!previousMismatch) {
+ previousMismatch = true;
+ Diag(NewTagLoc, diag::warn_struct_class_previous_tag_mismatch)
+ << getRedeclDiagFromTagKind(NewTag) << isTemplate << Name
+ << getRedeclDiagFromTagKind(I->getTagKind());
+ }
+ Diag(I->getInnerLocStart(), diag::note_struct_class_suggestion)
+ << getRedeclDiagFromTagKind(NewTag)
+ << FixItHint::CreateReplacement(I->getInnerLocStart(),
+ TypeWithKeyword::getTagTypeKindName(NewTag));
+ }
}
+ return true;
+ }
+ // Identify the prevailing tag kind: this is the kind of the definition (if
+ // there is a non-ignored definition), or otherwise the kind of the prior
+ // (non-ignored) declaration.
+ const TagDecl *PrevDef = Previous->getDefinition();
+ if (PrevDef && IsIgnored(PrevDef))
+ PrevDef = nullptr;
+ const TagDecl *Redecl = PrevDef ? PrevDef : Previous;
+ if (Redecl->getTagKind() != NewTag) {
Diag(NewTagLoc, diag::warn_struct_class_tag_mismatch)
<< getRedeclDiagFromTagKind(NewTag) << isTemplate << Name
<< getRedeclDiagFromTagKind(OldTag);
Diag(Redecl->getLocation(), diag::note_previous_use);
// If there is a previous definition, suggest a fix-it.
- if (Previous->getDefinition()) {
- Diag(NewTagLoc, diag::note_struct_class_suggestion)
- << getRedeclDiagFromTagKind(Redecl->getTagKind())
- << FixItHint::CreateReplacement(SourceRange(NewTagLoc),
- TypeWithKeyword::getTagTypeKindName(Redecl->getTagKind()));
+ if (PrevDef) {
+ Diag(NewTagLoc, diag::note_struct_class_suggestion)
+ << getRedeclDiagFromTagKind(Redecl->getTagKind())
+ << FixItHint::CreateReplacement(SourceRange(NewTagLoc),
+ TypeWithKeyword::getTagTypeKindName(Redecl->getTagKind()));
}
-
- return true;
}
- return false;
+
+ return true;
}
/// Add a minimal nested name specifier fixit hint to allow lookup of a tag name
Modified: cfe/trunk/test/SemaCXX/struct-class-redecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/struct-class-redecl.cpp?rev=348233&r1=348232&r2=348233&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/struct-class-redecl.cpp (original)
+++ cfe/trunk/test/SemaCXX/struct-class-redecl.cpp Mon Dec 3 18:45:28 2018
@@ -47,14 +47,43 @@ class E;
struct F;
struct F;
-struct F {};
+struct F {}; // expected-note {{previous use}}
struct F;
+class F; // expected-warning {{previously declared as a struct}} expected-note {{did you mean struct}}
template<class U> class G; // expected-note{{previous use is here}}\
// expected-note{{did you mean struct here?}}
template<class U> struct G; // expected-warning{{struct template 'G' was previously declared as a class template}}
template<class U> struct G {}; // expected-warning{{'G' defined as a struct template here but previously declared as a class template}}
+// Declarations from contexts where the warning is disabled are entirely
+// ignored for the purpose of this warning.
+struct J;
+struct K; // expected-note {{previous use}}
+struct L;
+struct M; // expected-note {{previous use}}
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wmismatched-tags"
+struct H;
+class I {};
+class J;
+class K;
+class L;
+class M {};
+#pragma clang diagnostic pop
+
+class H; // expected-note {{previous use}}
+struct H; // expected-warning {{previously declared as a class}}
+
+struct I; // expected-note {{previous use}}
+class I; // expected-warning {{previously declared as a struct}}
+
+struct J;
+class K; // expected-warning {{previously declared as a struct}}
+struct L;
+class M; // expected-warning {{previously declared as a struct}}
+
/*
*** 'X' messages ***
CHECK: warning: struct 'X' was previously declared as a class
More information about the cfe-commits
mailing list