Index: include/clang/AST/ASTStructuralEquivalence.h =================================================================== --- include/clang/AST/ASTStructuralEquivalence.h +++ include/clang/AST/ASTStructuralEquivalence.h @@ -49,6 +49,9 @@ /// unifying two types. bool StrictTypeSpelling; + /// \brief Whether warn or error on tag type mismatches. + bool ErrorOnTagTypeMismatch; + /// \brief Whether to complain about failures. bool Complain; @@ -58,9 +61,11 @@ StructuralEquivalenceContext( ASTContext &C1, ASTContext &C2, llvm::DenseSet> &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); 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>; +def warn_odr_tag_type_inconsistent + : Warning<"type %0 has incompatible definitions in different translation " + "units">, + InGroup>; +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/Parse/Parser.h =================================================================== --- include/clang/Parse/Parser.h +++ include/clang/Parse/Parser.h @@ -1429,7 +1429,8 @@ }; ExprResult ParseExpression(TypeCastState isTypeCast = NotTypeCast); - ExprResult ParseConstantExpression(TypeCastState isTypeCast = NotTypeCast); + ExprResult ParseConstantExpression(TypeCastState isTypeCast = NotTypeCast, + bool IgnorePrevDef = false); ExprResult ParseConstraintExpression(); // Expr that doesn't include commas. ExprResult ParseAssignmentExpression(TypeCastState isTypeCast = NotTypeCast); @@ -1447,12 +1448,13 @@ ExprResult ParseRHSOfBinaryExpression(ExprResult LHS, prec::Level MinPrec); ExprResult ParseCastExpression(bool isUnaryExpression, - bool isAddressOfOperand, - bool &NotCastExpr, - TypeCastState isTypeCast); + bool isAddressOfOperand, bool &NotCastExpr, + TypeCastState isTypeCast, + bool IgnorePrevDef = false); ExprResult ParseCastExpression(bool isUnaryExpression, bool isAddressOfOperand = false, - TypeCastState isTypeCast = NotTypeCast); + TypeCastState isTypeCast = NotTypeCast, + bool IgnorePrevDef = false); /// Returns true if the next token cannot start an expression. bool isNotExpressionStart(); @@ -1918,7 +1920,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/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 abd \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, @@ -1547,6 +1551,12 @@ NamedDecl *Previous; }; + struct CheckCompatTagInfo { + CheckCompatTagInfo() : CheckStructuralEquivalence(false), NewDef(nullptr) {} + bool CheckStructuralEquivalence; + NamedDecl *NewDef; + }; + DeclGroupPtrTy ConvertDeclToDeclGroup(Decl *Ptr, Decl *OwnedType = nullptr); void DiagnoseUseOfUnimplementedSelectors(); @@ -2037,15 +2047,14 @@ }; 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); + bool IsTypeSpecifier, SkipBodyInfo *SkipBody = nullptr, + CheckCompatTagInfo *CheckCompatTag = nullptr); Decl *ActOnTemplatedFriendTag(Scope *S, SourceLocation FriendLoc, unsigned TagSpec, SourceLocation TagLoc, @@ -2163,8 +2172,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 Elements, @@ -3908,7 +3917,8 @@ Scope *S, CXXScopeSpec &SS, SourceLocation TemplateKWLoc, UnqualifiedId &Id, bool HasTrailingLParen, bool IsAddressOfOperand, std::unique_ptr CCC = nullptr, - bool IsInlineAsmIdentifier = false, Token *KeywordReplacement = nullptr); + bool IsInlineAsmIdentifier = false, Token *KeywordReplacement = nullptr, + bool IgnorePrevDef = false); void DecomposeUnqualifiedId(const UnqualifiedId &Id, TemplateArgumentListInfo &Buffer, 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: lib/Parse/ParseDecl.cpp =================================================================== --- lib/Parse/ParseDecl.cpp +++ lib/Parse/ParseDecl.cpp @@ -4234,6 +4234,7 @@ stripTypeAttributesOffDeclSpec(attrs, DS, TUK); Sema::SkipBodyInfo SkipBody; + Sema::CheckCompatTagInfo CheckCompatTag; if (!Name && TUK == Sema::TUK_Definition && Tok.is(tok::l_brace) && NextToken().is(tok::identifier)) SkipBody = Actions.shouldSkipAnonEnumBody(getCurScope(), @@ -4244,12 +4245,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, &CheckCompatTag); if (SkipBody.ShouldSkip) { assert(TUK == Sema::TUK_Definition && "can only skip a definition"); @@ -4303,8 +4303,20 @@ 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 = CheckCompatTag.CheckStructuralEquivalence ? CheckCompatTag.NewDef + : TagDecl; + ParseEnumBody(StartLoc, D, + CheckCompatTag.CheckStructuralEquivalence /*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 (CheckCompatTag.CheckStructuralEquivalence && + !Actions.hasStructuralCompatLayout(TagDecl, CheckCompatTag.NewDef)) { + DS.SetTypeSpecError(); // Bail out... + return; + } + } if (DS.SetTypeSpecType(DeclSpec::TST_enum, StartLoc, NameLoc.isValid() ? NameLoc : StartLoc, @@ -4323,7 +4335,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 +4383,16 @@ EnumAvailabilityDiags.emplace_back(*this); if (TryConsumeToken(tok::equal, EqualLoc)) { - AssignedVal = ParseConstantExpression(); + AssignedVal = + ParseConstantExpression(NotTypeCast /*isTypeCast*/, IgnorePrevDef); 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/Parse/ParseDeclCXX.cpp =================================================================== --- lib/Parse/ParseDeclCXX.cpp +++ lib/Parse/ParseDeclCXX.cpp @@ -1734,6 +1734,7 @@ bool Owned = false; Sema::SkipBodyInfo SkipBody; + Sema::CheckCompatTagInfo CheckCompatTag; if (TemplateId) { // Explicit specialization, class template partial specialization, // or explicit instantiation. @@ -1883,7 +1884,7 @@ SourceLocation(), false, clang::TypeResult(), DSC == DSC_type_specifier, - &SkipBody); + &SkipBody, &CheckCompatTag); // If ActOnTag said the type was dependent, try again with the // less common call. @@ -1905,8 +1906,22 @@ else if (getLangOpts().CPlusPlus) ParseCXXMemberSpecification(StartLoc, AttrFixitLoc, attrs, TagType, TagOrTempResult.get()); - else - ParseStructUnionBody(StartLoc, TagType, TagOrTempResult.get()); + else { + Decl *D = CheckCompatTag.CheckStructuralEquivalence + ? CheckCompatTag.NewDef + : 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 (CheckCompatTag.CheckStructuralEquivalence && + !Actions.hasStructuralCompatLayout(TagOrTempResult.get(), + CheckCompatTag.NewDef)) { + DS.SetTypeSpecError(); // Bail out... + return; + } + } } if (!TagOrTempResult.isInvalid()) Index: lib/Parse/ParseExpr.cpp =================================================================== --- lib/Parse/ParseExpr.cpp +++ lib/Parse/ParseExpr.cpp @@ -192,8 +192,8 @@ return ParseRHSOfBinaryExpression(R, prec::Assignment); } - -ExprResult Parser::ParseConstantExpression(TypeCastState isTypeCast) { +ExprResult Parser::ParseConstantExpression(TypeCastState isTypeCast, + bool IgnorePrevDef) { // C++03 [basic.def.odr]p2: // An expression is potentially evaluated unless it appears where an // integral constant expression is required (see 5.19) [...]. @@ -201,7 +201,7 @@ EnterExpressionEvaluationContext ConstantEvaluated(Actions, Sema::ConstantEvaluated); - ExprResult LHS(ParseCastExpression(false, false, isTypeCast)); + ExprResult LHS(ParseCastExpression(false, false, isTypeCast, IgnorePrevDef)); ExprResult Res(ParseRHSOfBinaryExpression(LHS, prec::Conditional)); return Actions.ActOnConstantExpression(Res); } @@ -473,12 +473,11 @@ /// ExprResult Parser::ParseCastExpression(bool isUnaryExpression, bool isAddressOfOperand, - TypeCastState isTypeCast) { + TypeCastState isTypeCast, + bool IgnorePrevDef) { bool NotCastExpr; - ExprResult Res = ParseCastExpression(isUnaryExpression, - isAddressOfOperand, - NotCastExpr, - isTypeCast); + ExprResult Res = ParseCastExpression(isUnaryExpression, isAddressOfOperand, + NotCastExpr, isTypeCast, IgnorePrevDef); if (NotCastExpr) Diag(Tok, diag::err_expected_expression); return Res; @@ -694,7 +693,8 @@ ExprResult Parser::ParseCastExpression(bool isUnaryExpression, bool isAddressOfOperand, bool &NotCastExpr, - TypeCastState isTypeCast) { + TypeCastState isTypeCast, + bool IgnorePrevDef) { ExprResult Res; tok::TokenKind SavedKind = Tok.getKind(); NotCastExpr = false; @@ -980,7 +980,7 @@ getCurScope(), ScopeSpec, TemplateKWLoc, Name, Tok.is(tok::l_paren), isAddressOfOperand, std::move(Validator), /*IsInlineAsmIdentifier=*/false, - Tok.is(tok::r_paren) ? nullptr : &Replacement); + Tok.is(tok::r_paren) ? nullptr : &Replacement, IgnorePrevDef); if (!Res.isInvalid() && !Res.get()) { UnconsumeToken(Replacement); return ParseCastExpression(isUnaryExpression, isAddressOfOperand, Index: lib/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -12822,9 +12822,9 @@ MultiTemplateParamsArg TemplateParameterLists, bool &OwnedDecl, bool &IsDependent, SourceLocation ScopedEnumKWLoc, - bool ScopedEnumUsesClassTag, - TypeResult UnderlyingType, - bool IsTypeSpecifier, SkipBodyInfo *SkipBody) { + bool ScopedEnumUsesClassTag, TypeResult UnderlyingType, + bool IsTypeSpecifier, SkipBodyInfo *SkipBody, + CheckCompatTagInfo *CheckCompatTag) { // If this is not a definition, it must have a name. IdentifierInfo *OrigName = Name; assert((Name != nullptr || TUK == TUK_Definition) && @@ -12923,6 +12923,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 = [&](void) -> 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(New); + if (TypeSourceInfo *TI = EnumUnderlying.dyn_cast()) + ED->setIntegerTypeSourceInfo(TI); + else + ED->setIntegerType(QualType(EnumUnderlying.get(), 0)); + ED->setPromotionType(ED->getIntegerType()); + } + } else { // struct/union + New = RecordDecl::Create(Context, Kind, SearchDC, KWLoc, Loc, Name, + nullptr); + } + + if (RecordDecl *RD = dyn_cast(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 act on tag decl). 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,15 +13379,24 @@ 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 + // pass 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; + if (!getLangOpts().CPlusPlus) { + CheckCompatTag->CheckStructuralEquivalence = true; + CheckCompatTag->NewDef = createTagFromNewDecl(); + } else { + SkipBody->ShouldSkip = true; + } makeMergedDefinitionVisible(Hidden, KWLoc); return Def; } else if (!IsExplicitSpecializationAfterInstantiation) { @@ -15093,8 +15151,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(theEnumDecl); EnumConstantDecl *LastEnumConst = cast_or_null(lastEnumConst); @@ -15119,7 +15177,7 @@ // 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)); @@ -15128,9 +15186,11 @@ 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(PrevDecl)) && "Received TagDecl when not in C++!"); if (!isa(PrevDecl) && isDeclInScope(PrevDecl, CurContext, S) && Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -2109,7 +2109,8 @@ SourceLocation TemplateKWLoc, UnqualifiedId &Id, bool HasTrailingLParen, bool IsAddressOfOperand, std::unique_ptr CCC, - bool IsInlineAsmIdentifier, Token *KeywordReplacement) { + bool IsInlineAsmIdentifier, Token *KeywordReplacement, + bool IgnorePrevDef) { assert(!(IsAddressOfOperand && HasTrailingLParen) && "cannot be direct & operand and have a trailing lparen"); if (SS.isInvalid()) @@ -2194,8 +2195,18 @@ } } - if (R.isAmbiguous()) - return ExprError(); + if (R.isAmbiguous()) { + if (!IgnorePrevDef) + return ExprError(); + // We already know that there's a hidden decl included in the lookup, + // ignore it and only use the first found (the local) one... + auto RF = R.makeFilter(); + NamedDecl *ND = R.getRepresentativeDecl(); + while (RF.hasNext()) + if (ND != RF.next()) + RF.erase(); + RF.done(); + } // This could be an implicitly declared function reference (legal in C90, // extension in C99, forbidden in C++). 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,22 @@ return false; } +/// \brief Determine if \p D abd \p Suggested have a structurally compatibale +/// layout as described by C11 6.2.7/1. +bool Sema::hasStructuralCompatLayout(Decl *D, Decl *Suggested) { + llvm::DenseSet> 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: test/Modules/Inputs/F.framework/Headers/F.h =================================================================== --- /dev/null +++ test/Modules/Inputs/F.framework/Headers/F.h @@ -0,0 +1 @@ +// F.h 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/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/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/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/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 +};