Hi rnk, rsmith, aaron.ballman,
We would previously allow inappropriate inheritance keywords to appear
on class declarations. We would also allow inheritance keywords on
templates which were not fully specialized; this was divergent from
MSVC.
http://llvm-reviews.chandlerc.com/D2585
Files:
include/clang/AST/DeclCXX.h
include/clang/Basic/Attr.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/AST/MicrosoftCXXABI.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
test/SemaCXX/member-pointer-ms.cpp
Index: include/clang/AST/DeclCXX.h
===================================================================
--- include/clang/AST/DeclCXX.h
+++ include/clang/AST/DeclCXX.h
@@ -1602,6 +1602,8 @@
MSInheritanceAttr::Spelling getMSInheritanceModel() const;
/// \brief Locks-in the inheritance model for this class.
void setMSInheritanceModel();
+ /// \brief Calculate what the inheritance model would be for this class.
+ MSInheritanceAttr::Spelling calculateInheritanceModel() const;
/// \brief Determine whether this lambda expression was known to be dependent
/// at the time it was created, even if its context does not appear to be
Index: include/clang/Basic/Attr.td
===================================================================
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1379,10 +1379,6 @@
Keyword<"__multiple_inheritance">,
Keyword<"__virtual_inheritance">,
Keyword<"__unspecified_inheritance">];
- let Accessors = [Accessor<"IsSingle", [Keyword<"__single_inheritance">]>,
- Accessor<"IsMultiple", [Keyword<"__multiple_inheritance">]>,
- Accessor<"IsVirtual", [Keyword<"__virtual_inheritance">]>,
- Accessor<"IsUnspecified", [Keyword<"__unspecified_inheritance">]>];
}
def Unaligned : IgnoredAttr {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2341,6 +2341,18 @@
InGroup<DiagGroup<"unsupported-visibility">>;
def err_mismatched_visibility: Error<"visibility does not match previous declaration">;
def note_previous_attribute : Note<"previous attribute is here">;
+def err_mismatched_ms_inheritance : Error<
+ "inheritance model does not match previous declaration">;
+def warn_ignored_ms_inheritance_on_partial_spec : Warning<
+ "inheritance model ignored on partial specialization">,
+ InGroup<IgnoredAttributes>;
+def warn_ignored_ms_inheritance_on_primary_template : Warning<
+ "inheritance model ignored on primary template">,
+ InGroup<IgnoredAttributes>;
+def err_inconsistent_ms_inheritance : Error<
+ "inheritance model does not match definition">;
+def note_previous_ms_inheritance : Note<
+ "previous inheritance model specified here">;
def err_machine_mode : Error<"%select{unknown|unsupported}0 machine mode %1">;
def err_mode_not_primitive : Error<
"mode attribute only supported for integer and floating-point types">;
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -1861,6 +1861,8 @@
unsigned AttrSpellingListIndex);
DLLExportAttr *mergeDLLExportAttr(Decl *D, SourceRange Range,
unsigned AttrSpellingListIndex);
+ MSInheritanceAttr *mergeMSInheritanceAttr(Decl *D, SourceRange Range,
+ unsigned AttrSpellingListIndex);
FormatAttr *mergeFormatAttr(Decl *D, SourceRange Range,
IdentifierInfo *Format, int FormatIdx,
int FirstArg, unsigned AttrSpellingListIndex);
@@ -2570,6 +2572,8 @@
bool checkStringLiteralArgumentAttr(const AttributeList &Attr,
unsigned ArgNum, StringRef &Str,
SourceLocation *ArgLocation = 0);
+ bool checkMSInheritanceAttrOnDefinition(CXXRecordDecl *RD, SourceRange Range,
+ unsigned AttrSpellingListIndex);
void CheckAlignasUnderalignment(Decl *D);
Index: lib/AST/MicrosoftCXXABI.cpp
===================================================================
--- lib/AST/MicrosoftCXXABI.cpp
+++ lib/AST/MicrosoftCXXABI.cpp
@@ -92,43 +92,29 @@
return false;
}
-static MSInheritanceAttr::Spelling
-MSInheritanceAttrToModel(const MSInheritanceAttr *Attr) {
- if (Attr->IsSingle())
- return MSInheritanceAttr::Keyword_single_inheritance;
- else if (Attr->IsMultiple())
- return MSInheritanceAttr::Keyword_multiple_inheritance;
- else if (Attr->IsVirtual())
- return MSInheritanceAttr::Keyword_virtual_inheritance;
-
- assert(Attr->IsUnspecified() && "Expected unspecified inheritance attr");
- return MSInheritanceAttr::Keyword_unspecified_inheritance;
-}
-
-static MSInheritanceAttr::Spelling
-calculateInheritanceModel(const CXXRecordDecl *RD) {
- if (!RD->hasDefinition())
+MSInheritanceAttr::Spelling CXXRecordDecl::calculateInheritanceModel() const {
+ if (!hasDefinition())
return MSInheritanceAttr::Keyword_unspecified_inheritance;
- if (RD->getNumVBases() > 0)
+ if (getNumVBases() > 0)
return MSInheritanceAttr::Keyword_virtual_inheritance;
- if (usesMultipleInheritanceModel(RD))
+ if (usesMultipleInheritanceModel(this))
return MSInheritanceAttr::Keyword_multiple_inheritance;
return MSInheritanceAttr::Keyword_single_inheritance;
}
MSInheritanceAttr::Spelling
CXXRecordDecl::getMSInheritanceModel() const {
MSInheritanceAttr *IA = getAttr<MSInheritanceAttr>();
assert(IA && "Expected MSInheritanceAttr on the CXXRecordDecl!");
- return MSInheritanceAttrToModel(IA);
+ return (MSInheritanceAttr::Spelling)IA->getSpellingListIndex();
}
void CXXRecordDecl::setMSInheritanceModel() {
if (hasAttr<MSInheritanceAttr>())
return;
addAttr(MSInheritanceAttr::CreateImplicit(
- getASTContext(), calculateInheritanceModel(this), getSourceRange()));
+ getASTContext(), calculateInheritanceModel(), getSourceRange()));
}
// Returns the number of pointer and integer slots used to represent a member
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -1993,6 +1993,9 @@
else if (SectionAttr *SA = dyn_cast<SectionAttr>(Attr))
NewAttr = S.mergeSectionAttr(D, SA->getRange(), SA->getName(),
AttrSpellingListIndex);
+ else if (MSInheritanceAttr *IA = dyn_cast<MSInheritanceAttr>(Attr))
+ NewAttr =
+ S.mergeMSInheritanceAttr(D, IA->getRange(), AttrSpellingListIndex);
else if (isa<AlignedAttr>(Attr))
// AlignedAttrs are handled separately, because we need to handle all
// such attributes on a declaration at the same time.
@@ -12144,9 +12147,15 @@
if (!Completed)
Record->completeDefinition();
- if (Record->hasAttrs())
+ if (Record->hasAttrs()) {
CheckAlignasUnderalignment(Record);
+ if (MSInheritanceAttr *IA = Record->getAttr<MSInheritanceAttr>())
+ checkMSInheritanceAttrOnDefinition(cast<CXXRecordDecl>(Record),
+ IA->getRange(),
+ IA->getSpellingListIndex());
+ }
+
// Check if the structure/union declaration is a type that can have zero
// size in C. For C this is a language extension, for C++ it may cause
// compatibility problems.
Index: lib/Sema/SemaDeclAttr.cpp
===================================================================
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -2862,6 +2862,23 @@
}
}
+bool Sema::checkMSInheritanceAttrOnDefinition(CXXRecordDecl *RD,
+ SourceRange Range,
+ unsigned AttrSpellingListIndex) {
+ assert(RD->hasDefinition() && "RD has no definition!");
+
+ if (AttrSpellingListIndex !=
+ MSInheritanceAttr::Keyword_unspecified_inheritance &&
+ RD->calculateInheritanceModel() != AttrSpellingListIndex) {
+ Diag(Range.getBegin(), diag::err_inconsistent_ms_inheritance);
+ Diag(RD->getDefinition()->getLocation(), diag::note_defined_here)
+ << RD->getNameAsString();
+ return true;
+ }
+
+ return false;
+}
+
/// handleModeAttr - This attribute modifies the width of a decl with primitive
/// type.
///
@@ -3664,6 +3681,18 @@
Attr.getAttributeSpellingListIndex()));
}
+static void handleMSInheritanceAttr(Sema &S, Decl *D, const AttributeList &Attr) {
+ if (!S.LangOpts.CPlusPlus) {
+ S.Diag(Attr.getLoc(), diag::err_attribute_not_supported_in_lang)
+ << Attr.getName() << AttributeLangSupport::C;
+ return;
+ }
+ MSInheritanceAttr *IA = S.mergeMSInheritanceAttr(
+ D, Attr.getRange(), Attr.getAttributeSpellingListIndex());
+ if (IA)
+ D->addAttr(IA);
+}
+
static void handleARMInterruptAttr(Sema &S, Decl *D,
const AttributeList &Attr) {
// Check the attribute arguments.
@@ -3840,6 +3869,37 @@
D->addAttr(NewAttr);
}
+MSInheritanceAttr *
+Sema::mergeMSInheritanceAttr(Decl *D, SourceRange Range,
+ unsigned AttrSpellingListIndex) {
+ if (MSInheritanceAttr *IA = D->getAttr<MSInheritanceAttr>()) {
+ if (IA->getSpellingListIndex() == AttrSpellingListIndex)
+ return 0;
+ Diag(IA->getLocation(), diag::err_mismatched_ms_inheritance);
+ Diag(Range.getBegin(), diag::note_previous_ms_inheritance);
+ D->dropAttr<MSInheritanceAttr>();
+ }
+
+ CXXRecordDecl *RD = cast<CXXRecordDecl>(D);
+ if (RD->hasDefinition()) {
+ if (checkMSInheritanceAttrOnDefinition(RD, Range, AttrSpellingListIndex)) {
+ return 0;
+ }
+ } else {
+ if (isa<ClassTemplatePartialSpecializationDecl>(RD)) {
+ Diag(Range.getBegin(), diag::warn_ignored_ms_inheritance_on_partial_spec);
+ return 0;
+ }
+ if (RD->getDescribedClassTemplate()) {
+ Diag(Range.getBegin(), diag::warn_ignored_ms_inheritance_on_primary_template);
+ return 0;
+ }
+ }
+
+ return ::new (Context)
+ MSInheritanceAttr(Range, Context, AttrSpellingListIndex);
+}
+
/// Handles semantic checking for features that are common to all attributes,
/// such as checking whether a parameter was properly specified, or the correct
/// number of arguments were passed, etc.
@@ -4121,7 +4181,7 @@
handleUuidAttr(S, D, Attr);
break;
case AttributeList::AT_MSInheritance:
- handleSimpleAttribute<MSInheritanceAttr>(S, D, Attr); break;
+ handleMSInheritanceAttr(S, D, Attr); break;
case AttributeList::AT_ForceInline:
handleSimpleAttribute<ForceInlineAttr>(S, D, Attr); break;
case AttributeList::AT_SelectAny:
Index: test/SemaCXX/member-pointer-ms.cpp
===================================================================
--- test/SemaCXX/member-pointer-ms.cpp
+++ test/SemaCXX/member-pointer-ms.cpp
@@ -117,9 +117,7 @@
static_assert(sizeof(variable_forces_sizing) == kUnspecifiedDataSize, "");
static_assert(sizeof(MemPtr1) == kUnspecifiedDataSize, "");
-// FIXME: Clang fails this assert because it locks in the inheritance model at
-// the point of the typedef instead of the first usage, while MSVC does not.
-//static_assert(sizeof(MemPtr2) == kSingleDataSize, "");
+static_assert(sizeof(MemPtr2) == kSingleDataSize, "");
struct MemPtrInBody {
typedef int MemPtrInBody::*MemPtr;
@@ -166,3 +164,17 @@
int Virtual::*CastTest = reinterpret_cast<int Virtual::*>(&AA::x);
// expected-error@-1 {{cannot reinterpret_cast from member pointer type}}
+
+namespace ErrorTest {
+template <typename T, typename U> struct __single_inheritance A;
+ // expected-warning@-1 {{inheritance model ignored on primary template}}
+template <typename T> struct __multiple_inheritance A<T, T>;
+ // expected-warning@-1 {{inheritance model ignored on partial specialization}}
+template <> struct __single_inheritance A<int, float>;
+
+struct B {}; // expected-note {{B defined here}}
+struct __multiple_inheritance B; // expected-error{{inheritance model does not match definition}}
+
+struct __multiple_inheritance C {}; // expected-error{{inheritance model does not match definition}}
+ // expected-note@-1 {{C defined here}}
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
