bruno updated this revision to Diff 96080.
bruno marked 2 inline comments as done.
bruno added a comment.

Update the patch after Richard's suggestions


https://reviews.llvm.org/D31778

Files:
  include/clang/AST/ASTStructuralEquivalence.h
  include/clang/Basic/DiagnosticASTKinds.td
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/AST/ASTStructuralEquivalence.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseExpr.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaType.cpp
  test/Modules/Inputs/F.framework/Headers/F.h
  test/Modules/Inputs/F.framework/Modules/module.modulemap
  test/Modules/Inputs/F.framework/Modules/module.private.modulemap
  test/Modules/Inputs/F.framework/PrivateHeaders/NS.h
  test/Modules/elaborated-type-specifier-from-hidden-module.m
  test/Modules/redefinition-c-tagtypes.m

Index: test/Modules/redefinition-c-tagtypes.m
===================================================================
--- /dev/null
+++ test/Modules/redefinition-c-tagtypes.m
@@ -0,0 +1,48 @@
+// RUN: rm -rf %t.cache
+// RUN: %clang_cc1 -fsyntax-only %s -fmodules -fmodules-cache-path=%t.cache \
+// RUN:   -fimplicit-module-maps -F%S/Inputs -verify
+// RUN: %clang_cc1 -fsyntax-only %s -fmodules -fmodules-cache-path=%t.cache \
+// RUN:   -fimplicit-module-maps -F%S/Inputs -DCHANGE_TAGS -verify
+#include "F/F.h"
+
+#ifndef CHANGE_TAGS
+// expected-no-diagnostics
+#endif
+
+struct NS {
+  int a;
+#ifndef CHANGE_TAGS
+  int b;
+#else
+  int c; // expected-note {{field has name 'c' here}}
+  // expected-error@redefinition-c-tagtypes.m:12 {{type 'struct NS' has incompatible definitions}}
+  // expected-note@Inputs/F.framework/PrivateHeaders/NS.h:3 {{field has name 'b' here}}
+#endif
+};
+
+enum NSE {
+  FST = 22,
+#ifndef CHANGE_TAGS
+  SND = 43,
+#else
+  SND = 44, // expected-note {{enumerator 'SND' with value 44 here}}
+  // expected-error@redefinition-c-tagtypes.m:23 {{type 'enum NSE' has incompatible definitions}}
+  // expected-note@Inputs/F.framework/PrivateHeaders/NS.h:8 {{enumerator 'SND' with value 43 here}}
+#endif
+  TRD = 55
+};
+
+#define NS_ENUM(_type, _name) \
+  enum _name : _type _name;   \
+  enum _name : _type
+
+typedef NS_ENUM(int, NSMyEnum) {
+  MinX = 11,
+#ifndef CHANGE_TAGS
+  MinXOther = MinX,
+#else
+  MinXOther = TRD, // expected-note {{enumerator 'MinXOther' with value 55 here}}
+  // expected-error@redefinition-c-tagtypes.m:39 {{type 'enum NSMyEnum' has incompatible definitions}}
+  // expected-note@Inputs/F.framework/PrivateHeaders/NS.h:18 {{enumerator 'MinXOther' with value 11 here}}
+#endif
+};
Index: test/Modules/elaborated-type-specifier-from-hidden-module.m
===================================================================
--- test/Modules/elaborated-type-specifier-from-hidden-module.m
+++ test/Modules/elaborated-type-specifier-from-hidden-module.m
@@ -4,12 +4,11 @@
 @import ElaboratedTypeStructs.Empty; // The structs are now hidden.
 struct S1 *x;
 struct S2 *y;
-// FIXME: compatible definition should not be an error.
-struct S2 { int x; }; // expected-error {{redefinition}}
+struct S2 { int x; };
 struct S3 *z;
 // Incompatible definition.
-struct S3 { float y; }; // expected-error {{redefinition}}
-// expected-note@elaborated-type-structs.h:* 2 {{previous definition is here}}
+struct S3 { float y; }; // expected-error {{has incompatible definitions}} // expected-note {{field has name}}
+// expected-note@Inputs/elaborated-type-structs.h:3 {{field has name}}
 
 @import ElaboratedTypeStructs.Structs;
 
Index: test/Modules/Inputs/F.framework/PrivateHeaders/NS.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/F.framework/PrivateHeaders/NS.h
@@ -0,0 +1,19 @@
+struct NS {
+  int a;
+  int b;
+};
+
+enum NSE {
+  FST = 22,
+  SND = 43,
+  TRD = 55
+};
+
+#define NS_ENUM(_type, _name) \
+  enum _name : _type _name;   \
+  enum _name : _type
+
+typedef NS_ENUM(int, NSMyEnum) {
+  MinX = 11,
+  MinXOther = MinX,
+};
Index: test/Modules/Inputs/F.framework/Modules/module.private.modulemap
===================================================================
--- /dev/null
+++ test/Modules/Inputs/F.framework/Modules/module.private.modulemap
@@ -0,0 +1,7 @@
+module F.Private [system] {
+  explicit module NS {
+      header "NS.h"
+      export *
+  }
+  export *
+}
Index: test/Modules/Inputs/F.framework/Modules/module.modulemap
===================================================================
--- /dev/null
+++ test/Modules/Inputs/F.framework/Modules/module.modulemap
@@ -0,0 +1,7 @@
+framework module F [extern_c] [system] {
+  umbrella header "F.h"
+  module * {
+      export *
+  }
+  export *
+}
Index: test/Modules/Inputs/F.framework/Headers/F.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/F.framework/Headers/F.h
@@ -0,0 +1 @@
+// F.h
Index: lib/Sema/SemaType.cpp
===================================================================
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -15,6 +15,7 @@
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTMutationListener.h"
+#include "clang/AST/ASTStructuralEquivalence.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
@@ -7081,6 +7082,20 @@
   return false;
 }
 
+bool Sema::hasStructuralCompatLayout(Decl *D, Decl *Suggested) {
+  llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls;
+  if (!Suggested)
+    return false;
+
+  // FIXME: Add a specific mode for C11 6.2.7/1 in StructuralEquivalenceContext
+  // and isolate from other C++ specific checks.
+  StructuralEquivalenceContext Ctx(
+      D->getASTContext(), Suggested->getASTContext(), NonEquivalentDecls,
+      false /*StrictTypeSpelling*/, true /*Complain*/,
+      true /*ErrorOnTagTypeMismatch*/);
+  return Ctx.IsStructurallyEquivalent(D, Suggested);
+}
+
 /// \brief Determine whether there is any declaration of \p D that was ever a
 ///        definition (perhaps before module merging) and is currently visible.
 /// \param D The definition of the entity.
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -12822,8 +12822,7 @@
                      MultiTemplateParamsArg TemplateParameterLists,
                      bool &OwnedDecl, bool &IsDependent,
                      SourceLocation ScopedEnumKWLoc,
-                     bool ScopedEnumUsesClassTag,
-                     TypeResult UnderlyingType,
+                     bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
                      bool IsTypeSpecifier, SkipBodyInfo *SkipBody) {
   // If this is not a definition, it must have a name.
   IdentifierInfo *OrigName = Name;
@@ -12923,6 +12922,55 @@
   if (TUK == TUK_Friend || TUK == TUK_Reference)
     Redecl = NotForRedeclaration;
 
+  /// Create a new tag decl in C/ObjC. Since the ODR-like semantics for ObjC/C
+  /// implemented asks for structural equivalence checking, the returned decl
+  /// here is passed back to the parser, allowing the tag body to be parsed.
+  auto createTagFromNewDecl = [&]() -> TagDecl * {
+    assert(!getLangOpts().CPlusPlus && "not meant for C++ usage");
+    // If there is an identifier, use the location of the identifier as the
+    // location of the decl, otherwise use the location of the struct/union
+    // keyword.
+    SourceLocation Loc = NameLoc.isValid() ? NameLoc : KWLoc;
+    TagDecl *New = nullptr;
+
+    if (Kind == TTK_Enum) {
+      New = EnumDecl::Create(Context, SearchDC, KWLoc, Loc, Name, nullptr,
+                             ScopedEnum, ScopedEnumUsesClassTag,
+                             !EnumUnderlying.isNull());
+      // If this is an undefined enum, bail.
+      if (TUK != TUK_Definition && !Invalid)
+        return nullptr;
+      if (EnumUnderlying) {
+        EnumDecl *ED = cast<EnumDecl>(New);
+        if (TypeSourceInfo *TI = EnumUnderlying.dyn_cast<TypeSourceInfo *>())
+          ED->setIntegerTypeSourceInfo(TI);
+        else
+          ED->setIntegerType(QualType(EnumUnderlying.get<const Type *>(), 0));
+        ED->setPromotionType(ED->getIntegerType());
+      }
+    } else { // struct/union
+      New = RecordDecl::Create(Context, Kind, SearchDC, KWLoc, Loc, Name,
+                               nullptr);
+    }
+
+    if (RecordDecl *RD = dyn_cast<RecordDecl>(New)) {
+      // Add alignment attributes if necessary; these attributes are checked
+      // when the ASTContext lays out the structure.
+      //
+      // It is important for implementing the correct semantics that this
+      // happen here (in ActOnTag). The #pragma pack stack is
+      // maintained as a result of parser callbacks which can occur at
+      // many points during the parsing of a struct declaration (because
+      // the #pragma tokens are effectively skipped over during the
+      // parsing of the struct).
+      if (TUK == TUK_Definition) {
+        AddAlignmentAttributesForRecord(RD);
+        AddMsStructLayoutForRecord(RD);
+      }
+    }
+    return New;
+  };
+
   LookupResult Previous(*this, Name, NameLoc, LookupTagName, Redecl);
   if (Name && SS.isNotEmpty()) {
     // We have a nested-name tag ('struct foo::bar').
@@ -13330,16 +13378,29 @@
                     TSK_ExplicitSpecialization;
               }
 
+              // Allow ODR for ObjC/C in the sense that we won't keep more that
+              // one definition around (merge them). However, ensure the decl
+              // passes the structural compatibility check in C11 6.2.7/1.
+              bool AllowODR = getLangOpts().CPlusPlus || getLangOpts().ObjC1 ||
+                              getLangOpts().C11;
               NamedDecl *Hidden = nullptr;
-              if (SkipBody && getLangOpts().CPlusPlus &&
-                  !hasVisibleDefinition(Def, &Hidden)) {
+              if (SkipBody && AllowODR && !hasVisibleDefinition(Def, &Hidden)) {
                 // There is a definition of this tag, but it is not visible. We
                 // explicitly make use of C++'s one definition rule here, and
                 // assume that this definition is identical to the hidden one
                 // we already have. Make the existing definition visible and
                 // use it in place of this one.
-                SkipBody->ShouldSkip = true;
-                makeMergedDefinitionVisible(Hidden, KWLoc);
+                if (!getLangOpts().CPlusPlus) {
+                  // Postpone making the old definition visible until after we
+                  // complete parsing the new one and do the structural
+                  // comparison.
+                  SkipBody->CheckSameAsPrevious = true;
+                  SkipBody->New = createTagFromNewDecl();
+                  SkipBody->Previous = Hidden;
+                } else {
+                  SkipBody->ShouldSkip = true;
+                  makeMergedDefinitionVisible(Hidden, KWLoc);
+                }
                 return Def;
               } else if (!IsExplicitSpecializationAfterInstantiation) {
                 // A redeclaration in function prototype scope in C isn't
@@ -13564,7 +13625,7 @@
     // the ASTContext lays out the structure.
     //
     // It is important for implementing the correct semantics that this
-    // happen here (in act on tag decl). The #pragma pack stack is
+    // happen here (in ActOnTag). The #pragma pack stack is
     // maintained as a result of parser callbacks which can occur at
     // many points during the parsing of a struct declaration (because
     // the #pragma tokens are effectively skipped over during the
@@ -15093,8 +15154,8 @@
 
 Decl *Sema::ActOnEnumConstant(Scope *S, Decl *theEnumDecl, Decl *lastEnumConst,
                               SourceLocation IdLoc, IdentifierInfo *Id,
-                              AttributeList *Attr,
-                              SourceLocation EqualLoc, Expr *Val) {
+                              AttributeList *Attr, SourceLocation EqualLoc,
+                              Expr *Val, bool IgnorePrevDef) {
   EnumDecl *TheEnumDecl = cast<EnumDecl>(theEnumDecl);
   EnumConstantDecl *LastEnumConst =
     cast_or_null<EnumConstantDecl>(lastEnumConst);
@@ -15119,18 +15180,20 @@
   // different from T:
   // - every enumerator of every member of class T that is an unscoped
   // enumerated type
-  if (!TheEnumDecl->isScoped())
+  if (getLangOpts().CPlusPlus && !TheEnumDecl->isScoped())
     DiagnoseClassNameShadow(TheEnumDecl->getDeclContext(),
                             DeclarationNameInfo(Id, IdLoc));
 
   EnumConstantDecl *New =
     CheckEnumConstant(TheEnumDecl, LastEnumConst, IdLoc, Id, Val);
   if (!New)
     return nullptr;
 
-  if (PrevDecl) {
+  if (!IgnorePrevDef && PrevDecl) {
     // When in C++, we may get a TagDecl with the same name; in this case the
-    // enum constant will 'hide' the tag.
+    // enum constant will 'hide' the tag. In C/ObjC callers of this action
+    // might set IgnorePrevDef in order to speculatively generate a new def,
+    // which will never be actually added to the scope.
     assert((getLangOpts().CPlusPlus || !isa<TagDecl>(PrevDecl)) &&
            "Received TagDecl when not in C++!");
     if (!isa<TagDecl>(PrevDecl) && isDeclInScope(PrevDecl, CurContext, S) &&
Index: lib/Parse/ParseExpr.cpp
===================================================================
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -192,7 +192,6 @@
   return ParseRHSOfBinaryExpression(R, prec::Assignment);
 }
 
-
 ExprResult Parser::ParseConstantExpression(TypeCastState isTypeCast) {
   // C++03 [basic.def.odr]p2:
   //   An expression is potentially evaluated unless it appears where an
@@ -475,10 +474,8 @@
                                        bool isAddressOfOperand,
                                        TypeCastState isTypeCast) {
   bool NotCastExpr;
-  ExprResult Res = ParseCastExpression(isUnaryExpression,
-                                       isAddressOfOperand,
-                                       NotCastExpr,
-                                       isTypeCast);
+  ExprResult Res = ParseCastExpression(isUnaryExpression, isAddressOfOperand,
+                                       NotCastExpr, isTypeCast);
   if (NotCastExpr)
     Diag(Tok, diag::err_expected_expression);
   return Res;
Index: lib/Parse/ParseDeclCXX.cpp
===================================================================
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -1905,8 +1905,25 @@
     else if (getLangOpts().CPlusPlus)
       ParseCXXMemberSpecification(StartLoc, AttrFixitLoc, attrs, TagType,
                                   TagOrTempResult.get());
-    else
-      ParseStructUnionBody(StartLoc, TagType, TagOrTempResult.get());
+    else {
+      Decl *D =
+          SkipBody.CheckSameAsPrevious ? SkipBody.New : TagOrTempResult.get();
+      // Parse the definition body.
+      ParseStructUnionBody(StartLoc, TagType, D);
+      // Perform ODR-like check for C/ObjC when merging tag types from modules.
+      // Differently from C++, actually parse the body and reject / error out
+      // in case of a structural mismatch.
+      if (SkipBody.CheckSameAsPrevious) {
+        if (!Actions.hasStructuralCompatLayout(TagOrTempResult.get(),
+                                               SkipBody.New)) {
+          // Bail out.
+          DS.SetTypeSpecError();
+          return;
+        }
+        // Make the previous decl visible.
+        Actions.makeMergedDefinitionVisible(SkipBody.Previous, StartLoc);
+      }
+    }
   }
 
   if (!TagOrTempResult.isInvalid())
Index: lib/Parse/ParseDecl.cpp
===================================================================
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -4244,12 +4244,11 @@
   bool IsDependent = false;
   const char *PrevSpec = nullptr;
   unsigned DiagID;
-  Decl *TagDecl = Actions.ActOnTag(getCurScope(), DeclSpec::TST_enum, TUK,
-                                   StartLoc, SS, Name, NameLoc, attrs.getList(),
-                                   AS, DS.getModulePrivateSpecLoc(), TParams,
-                                   Owned, IsDependent, ScopedEnumKWLoc,
-                                   IsScopedUsingClassTag, BaseType,
-                                   DSC == DSC_type_specifier, &SkipBody);
+  Decl *TagDecl = Actions.ActOnTag(
+      getCurScope(), DeclSpec::TST_enum, TUK, StartLoc, SS, Name, NameLoc,
+      attrs.getList(), AS, DS.getModulePrivateSpecLoc(), TParams, Owned,
+      IsDependent, ScopedEnumKWLoc, IsScopedUsingClassTag, BaseType,
+      DSC == DSC_type_specifier, &SkipBody);
 
   if (SkipBody.ShouldSkip) {
     assert(TUK == Sema::TUK_Definition && "can only skip a definition");
@@ -4303,8 +4302,24 @@
     return;
   }
 
-  if (Tok.is(tok::l_brace) && TUK != Sema::TUK_Reference)
-    ParseEnumBody(StartLoc, TagDecl);
+  if (Tok.is(tok::l_brace) && TUK != Sema::TUK_Reference) {
+    Decl *D = SkipBody.CheckSameAsPrevious ? SkipBody.New : TagDecl;
+    //ParseEnumBody(StartLoc, D);
+    ParseEnumBody(StartLoc, D,
+                  SkipBody.CheckSameAsPrevious /*IgnorePrevDef*/);
+    // Perform ODR-like check for C/ObjC when merging tag types from modules.
+    // Differently from C++, actually parse the body and reject / error out
+    // in case of a structural mismatch.
+    if (SkipBody.CheckSameAsPrevious) {
+      if (!Actions.hasStructuralCompatLayout(TagDecl, SkipBody.New)) {
+        // Bail out.
+        DS.SetTypeSpecError();
+        return;
+      }
+      // Make the previous decl visible.
+      Actions.makeMergedDefinitionVisible(SkipBody.Previous, StartLoc);
+    }
+  }
 
   if (DS.SetTypeSpecType(DeclSpec::TST_enum, StartLoc,
                          NameLoc.isValid() ? NameLoc : StartLoc,
@@ -4323,7 +4338,8 @@
 ///       enumeration-constant:
 ///         identifier
 ///
-void Parser::ParseEnumBody(SourceLocation StartLoc, Decl *EnumDecl) {
+void Parser::ParseEnumBody(SourceLocation StartLoc, Decl *EnumDecl,
+                           bool IgnorePrevDef) {
   // Enter the scope of the enum body and start the definition.
   ParseScope EnumScope(this, Scope::DeclScope | Scope::EnumScope);
   Actions.ActOnTagStartDefinition(getCurScope(), EnumDecl);
@@ -4370,17 +4386,15 @@
     EnumAvailabilityDiags.emplace_back(*this);
 
     if (TryConsumeToken(tok::equal, EqualLoc)) {
-      AssignedVal = ParseConstantExpression();
+      AssignedVal = ParseConstantExpression(NotTypeCast /*isTypeCast*/);
       if (AssignedVal.isInvalid())
         SkipUntil(tok::comma, tok::r_brace, StopBeforeMatch);
     }
 
     // Install the enumerator constant into EnumDecl.
-    Decl *EnumConstDecl = Actions.ActOnEnumConstant(getCurScope(), EnumDecl,
-                                                    LastEnumConstDecl,
-                                                    IdentLoc, Ident,
-                                                    attrs.getList(), EqualLoc,
-                                                    AssignedVal.get());
+    Decl *EnumConstDecl = Actions.ActOnEnumConstant(
+        getCurScope(), EnumDecl, LastEnumConstDecl, IdentLoc, Ident,
+        attrs.getList(), EqualLoc, AssignedVal.get(), IgnorePrevDef);
     EnumAvailabilityDiags.back().done();
 
     EnumConstantDecls.push_back(EnumConstDecl);
Index: lib/AST/ASTStructuralEquivalence.cpp
===================================================================
--- lib/AST/ASTStructuralEquivalence.cpp
+++ lib/AST/ASTStructuralEquivalence.cpp
@@ -734,13 +734,28 @@
   // Check for equivalent field names.
   IdentifierInfo *Name1 = Field1->getIdentifier();
   IdentifierInfo *Name2 = Field2->getIdentifier();
-  if (!::IsStructurallyEquivalent(Name1, Name2))
+  if (!::IsStructurallyEquivalent(Name1, Name2)) {
+    if (Context.Complain) {
+      Context.Diag2(Owner2->getLocation(),
+                    Context.ErrorOnTagTypeMismatch
+                        ? diag::err_odr_tag_type_inconsistent
+                        : diag::warn_odr_tag_type_inconsistent)
+          << Context.C2.getTypeDeclType(Owner2);
+      Context.Diag2(Field2->getLocation(), diag::note_odr_field_name)
+          << Field2->getDeclName();
+      Context.Diag1(Field1->getLocation(), diag::note_odr_field_name)
+          << Field1->getDeclName();
+    }
     return false;
+  }
 
   if (!IsStructurallyEquivalent(Context, Field1->getType(),
                                 Field2->getType())) {
     if (Context.Complain) {
-      Context.Diag2(Owner2->getLocation(), diag::warn_odr_tag_type_inconsistent)
+      Context.Diag2(Owner2->getLocation(),
+                    Context.ErrorOnTagTypeMismatch
+                        ? diag::err_odr_tag_type_inconsistent
+                        : diag::warn_odr_tag_type_inconsistent)
           << Context.C2.getTypeDeclType(Owner2);
       Context.Diag2(Field2->getLocation(), diag::note_odr_field)
           << Field2->getDeclName() << Field2->getType();
@@ -752,7 +767,10 @@
 
   if (Field1->isBitField() != Field2->isBitField()) {
     if (Context.Complain) {
-      Context.Diag2(Owner2->getLocation(), diag::warn_odr_tag_type_inconsistent)
+      Context.Diag2(Owner2->getLocation(),
+                    Context.ErrorOnTagTypeMismatch
+                        ? diag::err_odr_tag_type_inconsistent
+                        : diag::warn_odr_tag_type_inconsistent)
           << Context.C2.getTypeDeclType(Owner2);
       if (Field1->isBitField()) {
         Context.Diag1(Field1->getLocation(), diag::note_odr_bit_field)
@@ -779,7 +797,9 @@
     if (Bits1 != Bits2) {
       if (Context.Complain) {
         Context.Diag2(Owner2->getLocation(),
-                      diag::warn_odr_tag_type_inconsistent)
+                      Context.ErrorOnTagTypeMismatch
+                          ? diag::err_odr_tag_type_inconsistent
+                          : diag::warn_odr_tag_type_inconsistent)
             << Context.C2.getTypeDeclType(Owner2);
         Context.Diag2(Field2->getLocation(), diag::note_odr_bit_field)
             << Field2->getDeclName() << Field2->getType() << Bits2;
@@ -798,7 +818,10 @@
                                      RecordDecl *D1, RecordDecl *D2) {
   if (D1->isUnion() != D2->isUnion()) {
     if (Context.Complain) {
-      Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent)
+      Context.Diag2(D2->getLocation(),
+                    Context.ErrorOnTagTypeMismatch
+                        ? diag::err_odr_tag_type_inconsistent
+                        : diag::warn_odr_tag_type_inconsistent)
           << Context.C2.getTypeDeclType(D2);
       Context.Diag1(D1->getLocation(), diag::note_odr_tag_kind_here)
           << D1->getDeclName() << (unsigned)D1->getTagKind();
@@ -921,7 +944,10 @@
        Field1 != Field1End; ++Field1, ++Field2) {
     if (Field2 == Field2End) {
       if (Context.Complain) {
-        Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent)
+        Context.Diag2(D2->getLocation(),
+                      Context.ErrorOnTagTypeMismatch
+                          ? diag::err_odr_tag_type_inconsistent
+                          : diag::warn_odr_tag_type_inconsistent)
             << Context.C2.getTypeDeclType(D2);
         Context.Diag1(Field1->getLocation(), diag::note_odr_field)
             << Field1->getDeclName() << Field1->getType();
@@ -936,7 +962,10 @@
 
   if (Field2 != Field2End) {
     if (Context.Complain) {
-      Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent)
+      Context.Diag2(D2->getLocation(),
+                    Context.ErrorOnTagTypeMismatch
+                        ? diag::err_odr_tag_type_inconsistent
+                        : diag::warn_odr_tag_type_inconsistent)
           << Context.C2.getTypeDeclType(D2);
       Context.Diag2(Field2->getLocation(), diag::note_odr_field)
           << Field2->getDeclName() << Field2->getType();
@@ -958,7 +987,10 @@
        EC1 != EC1End; ++EC1, ++EC2) {
     if (EC2 == EC2End) {
       if (Context.Complain) {
-        Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent)
+        Context.Diag2(D2->getLocation(),
+                      Context.ErrorOnTagTypeMismatch
+                          ? diag::err_odr_tag_type_inconsistent
+                          : diag::warn_odr_tag_type_inconsistent)
             << Context.C2.getTypeDeclType(D2);
         Context.Diag1(EC1->getLocation(), diag::note_odr_enumerator)
             << EC1->getDeclName() << EC1->getInitVal().toString(10);
@@ -972,7 +1004,10 @@
     if (!llvm::APSInt::isSameValue(Val1, Val2) ||
         !IsStructurallyEquivalent(EC1->getIdentifier(), EC2->getIdentifier())) {
       if (Context.Complain) {
-        Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent)
+        Context.Diag2(D2->getLocation(),
+                      Context.ErrorOnTagTypeMismatch
+                          ? diag::err_odr_tag_type_inconsistent
+                          : diag::warn_odr_tag_type_inconsistent)
             << Context.C2.getTypeDeclType(D2);
         Context.Diag2(EC2->getLocation(), diag::note_odr_enumerator)
             << EC2->getDeclName() << EC2->getInitVal().toString(10);
@@ -985,7 +1020,10 @@
 
   if (EC2 != EC2End) {
     if (Context.Complain) {
-      Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent)
+      Context.Diag2(D2->getLocation(),
+                    Context.ErrorOnTagTypeMismatch
+                        ? diag::err_odr_tag_type_inconsistent
+                        : diag::warn_odr_tag_type_inconsistent)
           << Context.C2.getTypeDeclType(D2);
       Context.Diag2(EC2->getLocation(), diag::note_odr_enumerator)
           << EC2->getDeclName() << EC2->getInitVal().toString(10);
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -1461,6 +1461,10 @@
 
   bool hasVisibleMergedDefinition(NamedDecl *Def);
 
+  /// Determine if \p D and \p Suggested have a structurally compatibale
+  /// layout as described in C11 6.2.7/1.
+  bool hasStructuralCompatLayout(Decl *D, Decl *Suggested);
+
   /// Determine if \p D has a visible definition. If not, suggest a declaration
   /// that should be made visible to expose the definition.
   bool hasVisibleDefinition(NamedDecl *D, NamedDecl **Suggested,
@@ -1542,9 +1546,13 @@
   //
 
   struct SkipBodyInfo {
-    SkipBodyInfo() : ShouldSkip(false), Previous(nullptr) {}
+    SkipBodyInfo()
+        : ShouldSkip(false), CheckSameAsPrevious(false), Previous(nullptr),
+          New(nullptr) {}
     bool ShouldSkip;
+    bool CheckSameAsPrevious;
     NamedDecl *Previous;
+    NamedDecl *New;
   };
 
   DeclGroupPtrTy ConvertDeclToDeclGroup(Decl *Ptr, Decl *OwnedType = nullptr);
@@ -2037,13 +2045,11 @@
   };
 
   Decl *ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
-                 SourceLocation KWLoc, CXXScopeSpec &SS,
-                 IdentifierInfo *Name, SourceLocation NameLoc,
-                 AttributeList *Attr, AccessSpecifier AS,
-                 SourceLocation ModulePrivateLoc,
-                 MultiTemplateParamsArg TemplateParameterLists,
-                 bool &OwnedDecl, bool &IsDependent,
-                 SourceLocation ScopedEnumKWLoc,
+                 SourceLocation KWLoc, CXXScopeSpec &SS, IdentifierInfo *Name,
+                 SourceLocation NameLoc, AttributeList *Attr,
+                 AccessSpecifier AS, SourceLocation ModulePrivateLoc,
+                 MultiTemplateParamsArg TemplateParameterLists, bool &OwnedDecl,
+                 bool &IsDependent, SourceLocation ScopedEnumKWLoc,
                  bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
                  bool IsTypeSpecifier, SkipBodyInfo *SkipBody = nullptr);
 
@@ -2163,8 +2169,8 @@
 
   Decl *ActOnEnumConstant(Scope *S, Decl *EnumDecl, Decl *LastEnumConstant,
                           SourceLocation IdLoc, IdentifierInfo *Id,
-                          AttributeList *Attrs,
-                          SourceLocation EqualLoc, Expr *Val);
+                          AttributeList *Attrs, SourceLocation EqualLoc,
+                          Expr *Val, bool IgnorePrevDef = false);
   void ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
                      Decl *EnumDecl,
                      ArrayRef<Decl *> Elements,
Index: include/clang/Parse/Parser.h
===================================================================
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -1447,8 +1447,7 @@
   ExprResult ParseRHSOfBinaryExpression(ExprResult LHS,
                                         prec::Level MinPrec);
   ExprResult ParseCastExpression(bool isUnaryExpression,
-                                 bool isAddressOfOperand,
-                                 bool &NotCastExpr,
+                                 bool isAddressOfOperand, bool &NotCastExpr,
                                  TypeCastState isTypeCast);
   ExprResult ParseCastExpression(bool isUnaryExpression,
                                  bool isAddressOfOperand = false,
@@ -1918,7 +1917,8 @@
   void ParseEnumSpecifier(SourceLocation TagLoc, DeclSpec &DS,
                           const ParsedTemplateInfo &TemplateInfo,
                           AccessSpecifier AS, DeclSpecContext DSC);
-  void ParseEnumBody(SourceLocation StartLoc, Decl *TagDecl);
+  void ParseEnumBody(SourceLocation StartLoc, Decl *TagDecl,
+                     bool IgnorePrevDef = false);
   void ParseStructUnionBody(SourceLocation StartLoc, unsigned TagType,
                             Decl *TagDecl);
 
Index: include/clang/Basic/DiagnosticASTKinds.td
===================================================================
--- include/clang/Basic/DiagnosticASTKinds.td
+++ include/clang/Basic/DiagnosticASTKinds.td
@@ -200,12 +200,17 @@
 def err_odr_function_type_inconsistent : Error<
   "external function %0 declared with incompatible types in different "
   "translation units (%1 vs. %2)">;
-def warn_odr_tag_type_inconsistent : Warning<
-  "type %0 has incompatible definitions in different translation units">,
-  InGroup<DiagGroup<"odr">>;
+def warn_odr_tag_type_inconsistent
+    : Warning<"type %0 has incompatible definitions in different translation "
+              "units">,
+      InGroup<DiagGroup<"odr">>;
+def err_odr_tag_type_inconsistent
+    : Error<"type %0 has incompatible definitions in different translation "
+            "units">;
 def note_odr_tag_kind_here: Note<
   "%0 is a %select{struct|interface|union|class|enum}1 here">;
 def note_odr_field : Note<"field %0 has type %1 here">;
+def note_odr_field_name : Note<"field has name %0 here">;
 def note_odr_missing_field : Note<"no corresponding field here">;
 def note_odr_bit_field : Note<"bit-field %0 with type %1 and length %2 here">;
 def note_odr_not_bit_field : Note<"field %0 is not a bit-field">;
Index: include/clang/AST/ASTStructuralEquivalence.h
===================================================================
--- include/clang/AST/ASTStructuralEquivalence.h
+++ include/clang/AST/ASTStructuralEquivalence.h
@@ -61,9 +61,11 @@
   StructuralEquivalenceContext(
       ASTContext &C1, ASTContext &C2,
       llvm::DenseSet<std::pair<Decl *, Decl *>> &NonEquivalentDecls,
-      bool StrictTypeSpelling = false, bool Complain = true)
+      bool StrictTypeSpelling = false, bool Complain = true,
+      bool ErrorOnTagTypeMismatch = false)
       : C1(C1), C2(C2), NonEquivalentDecls(NonEquivalentDecls),
-        StrictTypeSpelling(StrictTypeSpelling), Complain(Complain),
+        StrictTypeSpelling(StrictTypeSpelling),
+        ErrorOnTagTypeMismatch(ErrorOnTagTypeMismatch), Complain(Complain),
         LastDiagFromC2(false) {}
 
   DiagnosticBuilder Diag1(SourceLocation Loc, unsigned DiagID);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to