Sema: Diagnose improper application of inheritance keywords

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.

Differential Revision: http://llvm-reviews.chandlerc.com/D2585

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@200423 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h
index 8f5fa31d8802ac2d03484294b5a31571f03a9335..b15a46cbe5d51f94caafd720c0e50aa3829ab7f8 100644 (file)
--- a/include/clang/AST/DeclCXX.h
+++ b/include/clang/AST/DeclCXX.h
@@ -1602,6 +1602,8 @@ public:
   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

diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td
index a704f3bea313e5d7c70ff174ef9d18e2d63ed1b9..d9de72ddf9ddc37d08ec86fd524a0e5b198bda70 100644 (file)
--- a/include/clang/Basic/Attr.td
+++ b/include/clang/Basic/Attr.td
@@ -1401,10 +1401,6 @@ def MSInheritance : InheritableAttr {
                    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 {

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 67f5837246ff07ba873540671940040baf8a8299..406ad8691d2ecf3df164ea4f7fd306f8ca6a6a8c 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2351,6 +2351,13 @@ def warn_attribute_protected_visibility :
   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 %select{definition|previous declaration}0">;
+def warn_ignored_ms_inheritance : Warning<
+  "inheritance model ignored on %select{primary template|partial specialization}0">,
+  InGroup<IgnoredAttributes>;
+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">;

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 780937da928bcc51a6c7badad8fa09e15d661394..57f6c891a090b073e130d790212d641e22a39563 100644 (file)
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -1863,6 +1863,10 @@ public:
                                     unsigned AttrSpellingListIndex);
   DLLExportAttr *mergeDLLExportAttr(Decl *D, SourceRange Range,
                                     unsigned AttrSpellingListIndex);
+  MSInheritanceAttr *
+  mergeMSInheritanceAttr(Decl *D, SourceRange Range,
+                         unsigned AttrSpellingListIndex,
+                         MSInheritanceAttr::Spelling SemanticSpelling);
   FormatAttr *mergeFormatAttr(Decl *D, SourceRange Range,
                               IdentifierInfo *Format, int FormatIdx,
                               int FirstArg, unsigned AttrSpellingListIndex);
@@ -2572,6 +2576,9 @@ public:
   bool checkStringLiteralArgumentAttr(const AttributeList &Attr,
                                       unsigned ArgNum, StringRef &Str,
                                       SourceLocation *ArgLocation = 0);
+  bool checkMSInheritanceAttrOnDefinition(
+      CXXRecordDecl *RD, SourceRange Range,
+      MSInheritanceAttr::Spelling SemanticSpelling);
 
   void CheckAlignasUnderalignment(Decl *D);
 

diff --git a/lib/AST/MicrosoftCXXABI.cpp b/lib/AST/MicrosoftCXXABI.cpp
index c3478747a69b9d562d5cbde9195b6c7d8a2d002f..ee96e13e4b11adec51fcb5aa81c383a278364325 100644 (file)
--- a/lib/AST/MicrosoftCXXABI.cpp
+++ b/lib/AST/MicrosoftCXXABI.cpp
@@ -92,26 +92,12 @@ static bool usesMultipleInheritanceModel(const CXXRecordDecl *RD) {
   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;
 }
@@ -120,7 +106,7 @@ MSInheritanceAttr::Spelling
 CXXRecordDecl::getMSInheritanceModel() const {
   MSInheritanceAttr *IA = getAttr<MSInheritanceAttr>();
   assert(IA && "Expected MSInheritanceAttr on the CXXRecordDecl!");
-  return MSInheritanceAttrToModel(IA);
+  return IA->getSemanticSpelling();
 }
 
 void CXXRecordDecl::setMSInheritanceModel() {
@@ -128,7 +114,7 @@ void CXXRecordDecl::setMSInheritanceModel() {
     return;
 
   addAttr(MSInheritanceAttr::CreateImplicit(
-      getASTContext(), calculateInheritanceModel(this), getSourceRange()));
+      getASTContext(), calculateInheritanceModel(), getSourceRange()));
 }
 
 // Returns the number of pointer and integer slots used to represent a member

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index a054adfa731f5e5766c0dbb9a06d35ba7bf646c9..59539566e7dd13705aa35d9bf6a0ff5769150ba6 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -1962,6 +1962,9 @@ static bool mergeDeclAttribute(Sema &S, NamedDecl *D, InheritableAttr *Attr,
   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,
+                                       IA->getSemanticSpelling());
   else if (isa<AlignedAttr>(Attr))
     // AlignedAttrs are handled separately, because we need to handle all
     // such attributes on a declaration at the same time.
@@ -12104,9 +12107,15 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
     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->getSemanticSpelling());
+    }
+
     // 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.

diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index dd7aa477b17a5743de4eab1d0d21a570147cd7e7..50ec236fb7918a0f65cffbc976eb674676d55aeb 100644 (file)
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -2872,6 +2872,23 @@ void Sema::CheckAlignasUnderalignment(Decl *D) {
   }
 }
 
+bool Sema::checkMSInheritanceAttrOnDefinition(
+    CXXRecordDecl *RD, SourceRange Range,
+    MSInheritanceAttr::Spelling SemanticSpelling) {
+  assert(RD->hasDefinition() && "RD has no definition!");
+
+  if (SemanticSpelling != MSInheritanceAttr::Keyword_unspecified_inheritance &&
+      RD->calculateInheritanceModel() != SemanticSpelling) {
+    Diag(Range.getBegin(), diag::err_mismatched_ms_inheritance)
+        << 0 /*definition*/;
+    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.
 ///
@@ -3674,6 +3691,19 @@ static void handleUuidAttr(Sema &S, Decl *D, const AttributeList &Attr) {
                                         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(),
+      (MSInheritanceAttr::Spelling)Attr.getSemanticSpelling());
+  if (IA)
+    D->addAttr(IA);
+}
+
 static void handleARMInterruptAttr(Sema &S, Decl *D,
                                    const AttributeList &Attr) {
   // Check the attribute arguments.
@@ -3848,6 +3878,41 @@ static void handleDLLExportAttr(Sema &S, Decl *D, const AttributeList &Attr) {
     D->addAttr(NewAttr);
 }
 
+MSInheritanceAttr *
+Sema::mergeMSInheritanceAttr(Decl *D, SourceRange Range,
+                             unsigned AttrSpellingListIndex,
+                             MSInheritanceAttr::Spelling SemanticSpelling) {
+  if (MSInheritanceAttr *IA = D->getAttr<MSInheritanceAttr>()) {
+    if (IA->getSemanticSpelling() == SemanticSpelling)
+      return 0;
+    Diag(IA->getLocation(), diag::err_mismatched_ms_inheritance)
+        << 1 /*previous declaration*/;
+    Diag(Range.getBegin(), diag::note_previous_ms_inheritance);
+    D->dropAttr<MSInheritanceAttr>();
+  }
+
+  CXXRecordDecl *RD = cast<CXXRecordDecl>(D);
+  if (RD->hasDefinition()) {
+    if (checkMSInheritanceAttrOnDefinition(RD, Range, SemanticSpelling)) {
+      return 0;
+    }
+  } else {
+    if (isa<ClassTemplatePartialSpecializationDecl>(RD)) {
+      Diag(Range.getBegin(), diag::warn_ignored_ms_inheritance)
+          << 1 /*partial specialization*/;
+      return 0;
+    }
+    if (RD->getDescribedClassTemplate()) {
+      Diag(Range.getBegin(), diag::warn_ignored_ms_inheritance)
+          << 0 /*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.
@@ -4130,7 +4195,7 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,
     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:

diff --git a/test/SemaCXX/member-pointer-ms.cpp b/test/SemaCXX/member-pointer-ms.cpp
index 11260edac4e4c5de4ff2e8c28916f10ab5a08e40..ca37e07496a95c39c3c22d48220057c224a9a069 100644 (file)
--- a/test/SemaCXX/member-pointer-ms.cpp
+++ b/test/SemaCXX/member-pointer-ms.cpp
@@ -117,9 +117,7 @@ struct ForwardDecl2 : B {
 
 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 @@ struct MemPtrInTemplate {
 
 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}}
+}