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

llvm-svn: 200423

clang/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

clang/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 {

clang/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">;

clang/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);
 

clang/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

clang/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.

clang/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:

clang/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}}
+}