Skip to content

Commit f4f39fd

Browse files
bcardosolopesbcardosolopes
committed
[Modules] Implement ODR-like semantics for tag types in C/ObjC
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, for that, reuse the structural equivalence checks used by the ASTImporter. Few other considerations: - Create error diagnostics for tag types mismatches and thread them into the structural equivalence checks. - Note that by doing this we only support redefinition between types that are considered "compatible types" by C. This is mixed approach of the suggestions discussed in http://lists.llvm.org/pipermail/cfe-dev/2017-March/053257.html Differential Revision: https://reviews.llvm.org/D31778 rdar://problem/31909368 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@306918 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 9a86073 commit f4f39fd

15 files changed

+284
-53
lines changed

‎include/clang/AST/ASTStructuralEquivalence.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,11 @@ struct StructuralEquivalenceContext {
6262
StructuralEquivalenceContext(
6363
ASTContext &FromCtx, ASTContext &ToCtx,
6464
llvm::DenseSet<std::pair<Decl *, Decl *>> &NonEquivalentDecls,
65-
bool StrictTypeSpelling = false, bool Complain = true)
65+
bool StrictTypeSpelling = false, bool Complain = true,
66+
bool ErrorOnTagTypeMismatch = false)
6667
: FromCtx(FromCtx), ToCtx(ToCtx), NonEquivalentDecls(NonEquivalentDecls),
67-
StrictTypeSpelling(StrictTypeSpelling), Complain(Complain),
68+
StrictTypeSpelling(StrictTypeSpelling),
69+
ErrorOnTagTypeMismatch(ErrorOnTagTypeMismatch), Complain(Complain),
6870
LastDiagFromC2(false) {}
6971

7072
DiagnosticBuilder Diag1(SourceLocation Loc, unsigned DiagID);

‎include/clang/Basic/DiagnosticASTKinds.td

+8-3
Original file line numberDiff line numberDiff line change
@@ -200,12 +200,17 @@ def note_odr_defined_here : Note<"also defined here">;
200200
def err_odr_function_type_inconsistent : Error<
201201
"external function %0 declared with incompatible types in different "
202202
"translation units (%1 vs. %2)">;
203-
def warn_odr_tag_type_inconsistent : Warning<
204-
"type %0 has incompatible definitions in different translation units">,
205-
InGroup<DiagGroup<"odr">>;
203+
def warn_odr_tag_type_inconsistent
204+
: Warning<"type %0 has incompatible definitions in different translation "
205+
"units">,
206+
InGroup<DiagGroup<"odr">>;
207+
def err_odr_tag_type_inconsistent
208+
: Error<"type %0 has incompatible definitions in different translation "
209+
"units">;
206210
def note_odr_tag_kind_here: Note<
207211
"%0 is a %select{struct|interface|union|class|enum}1 here">;
208212
def note_odr_field : Note<"field %0 has type %1 here">;
213+
def note_odr_field_name : Note<"field has name %0 here">;
209214
def note_odr_missing_field : Note<"no corresponding field here">;
210215
def note_odr_bit_field : Note<"bit-field %0 with type %1 and length %2 here">;
211216
def note_odr_not_bit_field : Note<"field %0 is not a bit-field">;

‎include/clang/Sema/Sema.h

+22-10
Original file line numberDiff line numberDiff line change
@@ -1542,6 +1542,10 @@ class Sema {
15421542

15431543
bool hasVisibleMergedDefinition(NamedDecl *Def);
15441544

1545+
/// Determine if \p D and \p Suggested have a structurally compatible
1546+
/// layout as described in C11 6.2.7/1.
1547+
bool hasStructuralCompatLayout(Decl *D, Decl *Suggested);
1548+
15451549
/// Determine if \p D has a visible definition. If not, suggest a declaration
15461550
/// that should be made visible to expose the definition.
15471551
bool hasVisibleDefinition(NamedDecl *D, NamedDecl **Suggested,
@@ -1629,9 +1633,13 @@ class Sema {
16291633
//
16301634

16311635
struct SkipBodyInfo {
1632-
SkipBodyInfo() : ShouldSkip(false), Previous(nullptr) {}
1636+
SkipBodyInfo()
1637+
: ShouldSkip(false), CheckSameAsPrevious(false), Previous(nullptr),
1638+
New(nullptr) {}
16331639
bool ShouldSkip;
1640+
bool CheckSameAsPrevious;
16341641
NamedDecl *Previous;
1642+
NamedDecl *New;
16351643
};
16361644

16371645
DeclGroupPtrTy ConvertDeclToDeclGroup(Decl *Ptr, Decl *OwnedType = nullptr);
@@ -2145,13 +2153,11 @@ class Sema {
21452153
};
21462154

21472155
Decl *ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
2148-
SourceLocation KWLoc, CXXScopeSpec &SS,
2149-
IdentifierInfo *Name, SourceLocation NameLoc,
2150-
AttributeList *Attr, AccessSpecifier AS,
2151-
SourceLocation ModulePrivateLoc,
2152-
MultiTemplateParamsArg TemplateParameterLists,
2153-
bool &OwnedDecl, bool &IsDependent,
2154-
SourceLocation ScopedEnumKWLoc,
2156+
SourceLocation KWLoc, CXXScopeSpec &SS, IdentifierInfo *Name,
2157+
SourceLocation NameLoc, AttributeList *Attr,
2158+
AccessSpecifier AS, SourceLocation ModulePrivateLoc,
2159+
MultiTemplateParamsArg TemplateParameterLists, bool &OwnedDecl,
2160+
bool &IsDependent, SourceLocation ScopedEnumKWLoc,
21552161
bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
21562162
bool IsTypeSpecifier, bool IsTemplateParamOrArg,
21572163
SkipBodyInfo *SkipBody = nullptr);
@@ -2219,6 +2225,12 @@ class Sema {
22192225
/// struct, or union).
22202226
void ActOnTagStartDefinition(Scope *S, Decl *TagDecl);
22212227

2228+
/// Perform ODR-like check for C/ObjC when merging tag types from modules.
2229+
/// Differently from C++, actually parse the body and reject / error out
2230+
/// in case of a structural mismatch.
2231+
bool ActOnDuplicateDefinition(DeclSpec &DS, Decl *Prev,
2232+
SkipBodyInfo &SkipBody);
2233+
22222234
typedef void *SkippedDefinitionContext;
22232235

22242236
/// \brief Invoked when we enter a tag definition that we're skipping.
@@ -2272,8 +2284,8 @@ class Sema {
22722284

22732285
Decl *ActOnEnumConstant(Scope *S, Decl *EnumDecl, Decl *LastEnumConstant,
22742286
SourceLocation IdLoc, IdentifierInfo *Id,
2275-
AttributeList *Attrs,
2276-
SourceLocation EqualLoc, Expr *Val);
2287+
AttributeList *Attrs, SourceLocation EqualLoc,
2288+
Expr *Val);
22772289
void ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
22782290
Decl *EnumDecl,
22792291
ArrayRef<Decl *> Elements,

‎lib/AST/ASTStructuralEquivalence.cpp

+48-10
Original file line numberDiff line numberDiff line change
@@ -735,13 +735,28 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
735735
// Check for equivalent field names.
736736
IdentifierInfo *Name1 = Field1->getIdentifier();
737737
IdentifierInfo *Name2 = Field2->getIdentifier();
738-
if (!::IsStructurallyEquivalent(Name1, Name2))
738+
if (!::IsStructurallyEquivalent(Name1, Name2)) {
739+
if (Context.Complain) {
740+
Context.Diag2(Owner2->getLocation(),
741+
Context.ErrorOnTagTypeMismatch
742+
? diag::err_odr_tag_type_inconsistent
743+
: diag::warn_odr_tag_type_inconsistent)
744+
<< Context.ToCtx.getTypeDeclType(Owner2);
745+
Context.Diag2(Field2->getLocation(), diag::note_odr_field_name)
746+
<< Field2->getDeclName();
747+
Context.Diag1(Field1->getLocation(), diag::note_odr_field_name)
748+
<< Field1->getDeclName();
749+
}
739750
return false;
751+
}
740752

741753
if (!IsStructurallyEquivalent(Context, Field1->getType(),
742754
Field2->getType())) {
743755
if (Context.Complain) {
744-
Context.Diag2(Owner2->getLocation(), diag::warn_odr_tag_type_inconsistent)
756+
Context.Diag2(Owner2->getLocation(),
757+
Context.ErrorOnTagTypeMismatch
758+
? diag::err_odr_tag_type_inconsistent
759+
: diag::warn_odr_tag_type_inconsistent)
745760
<< Context.ToCtx.getTypeDeclType(Owner2);
746761
Context.Diag2(Field2->getLocation(), diag::note_odr_field)
747762
<< Field2->getDeclName() << Field2->getType();
@@ -753,7 +768,10 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
753768

754769
if (Field1->isBitField() != Field2->isBitField()) {
755770
if (Context.Complain) {
756-
Context.Diag2(Owner2->getLocation(), diag::warn_odr_tag_type_inconsistent)
771+
Context.Diag2(Owner2->getLocation(),
772+
Context.ErrorOnTagTypeMismatch
773+
? diag::err_odr_tag_type_inconsistent
774+
: diag::warn_odr_tag_type_inconsistent)
757775
<< Context.ToCtx.getTypeDeclType(Owner2);
758776
if (Field1->isBitField()) {
759777
Context.Diag1(Field1->getLocation(), diag::note_odr_bit_field)
@@ -780,7 +798,9 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
780798
if (Bits1 != Bits2) {
781799
if (Context.Complain) {
782800
Context.Diag2(Owner2->getLocation(),
783-
diag::warn_odr_tag_type_inconsistent)
801+
Context.ErrorOnTagTypeMismatch
802+
? diag::err_odr_tag_type_inconsistent
803+
: diag::warn_odr_tag_type_inconsistent)
784804
<< Context.ToCtx.getTypeDeclType(Owner2);
785805
Context.Diag2(Field2->getLocation(), diag::note_odr_bit_field)
786806
<< Field2->getDeclName() << Field2->getType() << Bits2;
@@ -799,7 +819,10 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
799819
RecordDecl *D1, RecordDecl *D2) {
800820
if (D1->isUnion() != D2->isUnion()) {
801821
if (Context.Complain) {
802-
Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent)
822+
Context.Diag2(D2->getLocation(),
823+
Context.ErrorOnTagTypeMismatch
824+
? diag::err_odr_tag_type_inconsistent
825+
: diag::warn_odr_tag_type_inconsistent)
803826
<< Context.ToCtx.getTypeDeclType(D2);
804827
Context.Diag1(D1->getLocation(), diag::note_odr_tag_kind_here)
805828
<< D1->getDeclName() << (unsigned)D1->getTagKind();
@@ -927,7 +950,10 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
927950
Field1 != Field1End; ++Field1, ++Field2) {
928951
if (Field2 == Field2End) {
929952
if (Context.Complain) {
930-
Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent)
953+
Context.Diag2(D2->getLocation(),
954+
Context.ErrorOnTagTypeMismatch
955+
? diag::err_odr_tag_type_inconsistent
956+
: diag::warn_odr_tag_type_inconsistent)
931957
<< Context.ToCtx.getTypeDeclType(D2);
932958
Context.Diag1(Field1->getLocation(), diag::note_odr_field)
933959
<< Field1->getDeclName() << Field1->getType();
@@ -942,7 +968,10 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
942968

943969
if (Field2 != Field2End) {
944970
if (Context.Complain) {
945-
Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent)
971+
Context.Diag2(D2->getLocation(),
972+
Context.ErrorOnTagTypeMismatch
973+
? diag::err_odr_tag_type_inconsistent
974+
: diag::warn_odr_tag_type_inconsistent)
946975
<< Context.ToCtx.getTypeDeclType(D2);
947976
Context.Diag2(Field2->getLocation(), diag::note_odr_field)
948977
<< Field2->getDeclName() << Field2->getType();
@@ -964,7 +993,10 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
964993
EC1 != EC1End; ++EC1, ++EC2) {
965994
if (EC2 == EC2End) {
966995
if (Context.Complain) {
967-
Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent)
996+
Context.Diag2(D2->getLocation(),
997+
Context.ErrorOnTagTypeMismatch
998+
? diag::err_odr_tag_type_inconsistent
999+
: diag::warn_odr_tag_type_inconsistent)
9681000
<< Context.ToCtx.getTypeDeclType(D2);
9691001
Context.Diag1(EC1->getLocation(), diag::note_odr_enumerator)
9701002
<< EC1->getDeclName() << EC1->getInitVal().toString(10);
@@ -978,7 +1010,10 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
9781010
if (!llvm::APSInt::isSameValue(Val1, Val2) ||
9791011
!IsStructurallyEquivalent(EC1->getIdentifier(), EC2->getIdentifier())) {
9801012
if (Context.Complain) {
981-
Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent)
1013+
Context.Diag2(D2->getLocation(),
1014+
Context.ErrorOnTagTypeMismatch
1015+
? diag::err_odr_tag_type_inconsistent
1016+
: diag::warn_odr_tag_type_inconsistent)
9821017
<< Context.ToCtx.getTypeDeclType(D2);
9831018
Context.Diag2(EC2->getLocation(), diag::note_odr_enumerator)
9841019
<< EC2->getDeclName() << EC2->getInitVal().toString(10);
@@ -991,7 +1026,10 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
9911026

9921027
if (EC2 != EC2End) {
9931028
if (Context.Complain) {
994-
Context.Diag2(D2->getLocation(), diag::warn_odr_tag_type_inconsistent)
1029+
Context.Diag2(D2->getLocation(),
1030+
Context.ErrorOnTagTypeMismatch
1031+
? diag::err_odr_tag_type_inconsistent
1032+
: diag::warn_odr_tag_type_inconsistent)
9951033
<< Context.ToCtx.getTypeDeclType(D2);
9961034
Context.Diag2(EC2->getLocation(), diag::note_odr_enumerator)
9971035
<< EC2->getDeclName() << EC2->getInitVal().toString(10);

‎lib/Parse/ParseDecl.cpp

+12-7
Original file line numberDiff line numberDiff line change
@@ -4319,8 +4319,15 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
43194319
return;
43204320
}
43214321

4322-
if (Tok.is(tok::l_brace) && TUK != Sema::TUK_Reference)
4323-
ParseEnumBody(StartLoc, TagDecl);
4322+
if (Tok.is(tok::l_brace) && TUK != Sema::TUK_Reference) {
4323+
Decl *D = SkipBody.CheckSameAsPrevious ? SkipBody.New : TagDecl;
4324+
ParseEnumBody(StartLoc, D);
4325+
if (SkipBody.CheckSameAsPrevious &&
4326+
!Actions.ActOnDuplicateDefinition(DS, TagDecl, SkipBody)) {
4327+
DS.SetTypeSpecError();
4328+
return;
4329+
}
4330+
}
43244331

43254332
if (DS.SetTypeSpecType(DeclSpec::TST_enum, StartLoc,
43264333
NameLoc.isValid() ? NameLoc : StartLoc,
@@ -4392,11 +4399,9 @@ void Parser::ParseEnumBody(SourceLocation StartLoc, Decl *EnumDecl) {
43924399
}
43934400

43944401
// Install the enumerator constant into EnumDecl.
4395-
Decl *EnumConstDecl = Actions.ActOnEnumConstant(getCurScope(), EnumDecl,
4396-
LastEnumConstDecl,
4397-
IdentLoc, Ident,
4398-
attrs.getList(), EqualLoc,
4399-
AssignedVal.get());
4402+
Decl *EnumConstDecl = Actions.ActOnEnumConstant(
4403+
getCurScope(), EnumDecl, LastEnumConstDecl, IdentLoc, Ident,
4404+
attrs.getList(), EqualLoc, AssignedVal.get());
44004405
EnumAvailabilityDiags.back().done();
44014406

44024407
EnumConstantDecls.push_back(EnumConstDecl);

‎lib/Parse/ParseDeclCXX.cpp

+12-2
Original file line numberDiff line numberDiff line change
@@ -1910,8 +1910,18 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,
19101910
else if (getLangOpts().CPlusPlus)
19111911
ParseCXXMemberSpecification(StartLoc, AttrFixitLoc, attrs, TagType,
19121912
TagOrTempResult.get());
1913-
else
1914-
ParseStructUnionBody(StartLoc, TagType, TagOrTempResult.get());
1913+
else {
1914+
Decl *D =
1915+
SkipBody.CheckSameAsPrevious ? SkipBody.New : TagOrTempResult.get();
1916+
// Parse the definition body.
1917+
ParseStructUnionBody(StartLoc, TagType, D);
1918+
if (SkipBody.CheckSameAsPrevious &&
1919+
!Actions.ActOnDuplicateDefinition(DS, TagOrTempResult.get(),
1920+
SkipBody)) {
1921+
DS.SetTypeSpecError();
1922+
return;
1923+
}
1924+
}
19151925
}
19161926

19171927
if (!TagOrTempResult.isInvalid())

0 commit comments

Comments
 (0)