Skip to content

Commit 07bbf6c

Browse files
committed
Revert "[modules] Fix miscompilation when using two RecordDecl definitions with the same name."
This reverts commit 9454716. The patch being reverted results in swift-corelibs-foundation failing to build due to being unable to find `Glibc.pthread_mutex_t`. The declaration is defined in the module, but is also imported via CDispatch. This patch marks any complete redeclarations of a given declaration incomplete. Swift can't import incomplete declarations, so it is ignored in `ClangImporter::isVisibleFromModule`. We will need some mechanism of detecting redeclarations while not marking complete declarations as being incomplete.
1 parent e7bd8f7 commit 07bbf6c

File tree

15 files changed

+3
-211
lines changed

15 files changed

+3
-211
lines changed

clang/include/clang/Serialization/ASTReader.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,10 +1155,6 @@ class ASTReader
11551155
/// definitions. Only populated when using modules in C++.
11561156
llvm::DenseMap<EnumDecl *, EnumDecl *> EnumDefinitions;
11571157

1158-
/// A mapping from canonical declarations of records to their canonical
1159-
/// definitions. Doesn't cover CXXRecordDecl.
1160-
llvm::DenseMap<RecordDecl *, RecordDecl *> RecordDefinitions;
1161-
11621158
/// When reading a Stmt tree, Stmt operands are placed in this stack.
11631159
SmallVector<Stmt *, 16> StmtStack;
11641160

clang/lib/Serialization/ASTCommon.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,7 @@ bool serialization::needsAnonymousDeclarationNumber(const NamedDecl *D) {
474474
// Otherwise, we only care about anonymous class members / block-scope decls.
475475
// FIXME: We need to handle lambdas and blocks within inline / templated
476476
// variables too.
477-
if (D->getDeclName() || !isa<RecordDecl>(D->getLexicalDeclContext()))
477+
if (D->getDeclName() || !isa<CXXRecordDecl>(D->getLexicalDeclContext()))
478478
return false;
479479
return isa<TagDecl>(D) || isa<FieldDecl>(D);
480480
}

clang/lib/Serialization/ASTReaderDecl.cpp

Lines changed: 2 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ namespace clang {
332332
RedeclarableResult VisitTagDecl(TagDecl *TD);
333333
void VisitEnumDecl(EnumDecl *ED);
334334
RedeclarableResult VisitRecordDeclImpl(RecordDecl *RD);
335-
void VisitRecordDecl(RecordDecl *RD);
335+
void VisitRecordDecl(RecordDecl *RD) { VisitRecordDeclImpl(RD); }
336336
RedeclarableResult VisitCXXRecordDeclImpl(CXXRecordDecl *D);
337337
void VisitCXXRecordDecl(CXXRecordDecl *D) { VisitCXXRecordDeclImpl(D); }
338338
RedeclarableResult VisitClassTemplateSpecializationDeclImpl(
@@ -808,34 +808,6 @@ ASTDeclReader::VisitRecordDeclImpl(RecordDecl *RD) {
808808
return Redecl;
809809
}
810810

811-
void ASTDeclReader::VisitRecordDecl(RecordDecl *RD) {
812-
VisitRecordDeclImpl(RD);
813-
814-
// Maintain the invariant of a redeclaration chain containing only
815-
// a single definition.
816-
if (RD->isCompleteDefinition()) {
817-
RecordDecl *Canon = static_cast<RecordDecl *>(RD->getCanonicalDecl());
818-
RecordDecl *&OldDef = Reader.RecordDefinitions[Canon];
819-
if (!OldDef) {
820-
// This is the first time we've seen an imported definition. Look for a
821-
// local definition before deciding that we are the first definition.
822-
for (auto *D : merged_redecls(Canon)) {
823-
if (!D->isFromASTFile() && D->isCompleteDefinition()) {
824-
OldDef = D;
825-
break;
826-
}
827-
}
828-
}
829-
if (OldDef) {
830-
Reader.MergedDeclContexts.insert(std::make_pair(RD, OldDef));
831-
RD->setCompleteDefinition(false);
832-
Reader.mergeDefinitionVisibility(OldDef, RD);
833-
} else {
834-
OldDef = RD;
835-
}
836-
}
837-
}
838-
839811
void ASTDeclReader::VisitValueDecl(ValueDecl *VD) {
840812
VisitNamedDecl(VD);
841813
// For function declarations, defer reading the type in case the function has
@@ -2675,7 +2647,7 @@ static bool allowODRLikeMergeInC(NamedDecl *ND) {
26752647
if (!ND)
26762648
return false;
26772649
// TODO: implement merge for other necessary decls.
2678-
if (isa<EnumConstantDecl, FieldDecl, IndirectFieldDecl>(ND))
2650+
if (isa<EnumConstantDecl>(ND))
26792651
return true;
26802652
return false;
26812653
}
@@ -3347,9 +3319,6 @@ DeclContext *ASTDeclReader::getPrimaryContextForMerging(ASTReader &Reader,
33473319
return DD->Definition;
33483320
}
33493321

3350-
if (auto *RD = dyn_cast<RecordDecl>(DC))
3351-
return RD->getDefinition();
3352-
33533322
if (auto *ED = dyn_cast<EnumDecl>(DC))
33543323
return ED->getASTContext().getLangOpts().CPlusPlus? ED->getDefinition()
33553324
: nullptr;
@@ -3433,9 +3402,6 @@ ASTDeclReader::getPrimaryDCForAnonymousDecl(DeclContext *LexicalDC) {
34333402
if (auto *MD = dyn_cast<ObjCMethodDecl>(D))
34343403
if (MD->isThisDeclarationADefinition())
34353404
return MD;
3436-
if (auto *RD = dyn_cast<RecordDecl>(D))
3437-
if (RD->isThisDeclarationADefinition())
3438-
return RD;
34393405
}
34403406

34413407
// No merged definition yet.

clang/test/Modules/Inputs/merge-record-definition/RecordDef.framework/Headers/RecordDef.h

Lines changed: 0 additions & 21 deletions
This file was deleted.

clang/test/Modules/Inputs/merge-record-definition/RecordDef.framework/Modules/module.modulemap

Lines changed: 0 additions & 4 deletions
This file was deleted.

clang/test/Modules/Inputs/merge-record-definition/RecordDefCopy.framework/Headers/RecordDefCopy.h

Lines changed: 0 additions & 21 deletions
This file was deleted.

clang/test/Modules/Inputs/merge-record-definition/RecordDefCopy.framework/Modules/module.modulemap

Lines changed: 0 additions & 4 deletions
This file was deleted.

clang/test/Modules/Inputs/merge-record-definition/RecordDefHidden.framework/Headers/Hidden.h

Lines changed: 0 additions & 21 deletions
This file was deleted.

clang/test/Modules/Inputs/merge-record-definition/RecordDefHidden.framework/Headers/Visible.h

Lines changed: 0 additions & 1 deletion
This file was deleted.

clang/test/Modules/Inputs/merge-record-definition/RecordDefHidden.framework/Modules/module.modulemap

Lines changed: 0 additions & 9 deletions
This file was deleted.

clang/test/Modules/Inputs/merge-record-definition/RecordDefIncluder.framework/Headers/RecordDefIncluder.h

Lines changed: 0 additions & 1 deletion
This file was deleted.

clang/test/Modules/Inputs/merge-record-definition/RecordDefIncluder.framework/Modules/module.modulemap

Lines changed: 0 additions & 4 deletions
This file was deleted.

clang/test/Modules/merge-record-definition-nonmodular.m

Lines changed: 0 additions & 38 deletions
This file was deleted.

clang/test/Modules/merge-record-definition-visibility.m

Lines changed: 0 additions & 18 deletions
This file was deleted.

clang/test/Modules/merge-record-definition.m

Lines changed: 0 additions & 28 deletions
This file was deleted.

0 commit comments

Comments
 (0)