Skip to content

Commit 679ee11

Browse files
committed
[Clang importer] Eliminate retain cycle involving the Clang importer implementation.
The Clang importer implementation held on to a Clang instance, which held a list of module file extensions, which included the Clang importer implementation... break the cycle by separating out the module file extension from the Clang importer. Fixes SR-562 / rdar://problem/24225173.
1 parent b7ea3b9 commit 679ee11

File tree

2 files changed

+50
-36
lines changed

2 files changed

+50
-36
lines changed

lib/ClangImporter/ClangImporter.cpp

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -239,11 +239,10 @@ ClangImporter::ClangImporter(ASTContext &ctx,
239239
: ClangModuleLoader(tracker),
240240
Impl(*new Implementation(ctx, clangImporterOpts))
241241
{
242-
Impl.Retain();
243242
}
244243

245244
ClangImporter::~ClangImporter() {
246-
Impl.Release();
245+
delete &Impl;
247246
}
248247

249248
void ClangImporter::setTypeResolver(LazyResolver &resolver) {
@@ -548,7 +547,8 @@ ClangImporter::create(ASTContext &ctx,
548547
sourceBuffer.release());
549548

550549
// Install a Clang module file extension to build Swift name lookup tables.
551-
invocation->getFrontendOpts().ModuleFileExtensions.push_back(&importer->Impl);
550+
invocation->getFrontendOpts().ModuleFileExtensions.push_back(
551+
new Implementation::SwiftNameLookupExtension(importer->Impl));
552552

553553
// Create a compiler instance.
554554
auto PCHContainerOperations =
@@ -4106,7 +4106,8 @@ void ClangImporter::getMangledName(raw_ostream &os,
41064106
// ---------------------------------------------------------------------------
41074107

41084108
clang::ModuleFileExtensionMetadata
4109-
ClangImporter::Implementation::getExtensionMetadata() const {
4109+
ClangImporter::Implementation::SwiftNameLookupExtension::
4110+
getExtensionMetadata() const {
41104111
clang::ModuleFileExtensionMetadata metadata;
41114112
metadata.BlockName = "swift.lookup";
41124113
metadata.MajorVersion = SWIFT_LOOKUP_TABLE_VERSION_MAJOR;
@@ -4115,18 +4116,20 @@ ClangImporter::Implementation::getExtensionMetadata() const {
41154116
return metadata;
41164117
}
41174118

4118-
llvm::hash_code ClangImporter::Implementation::hashExtension(
4119-
llvm::hash_code code) const {
4119+
llvm::hash_code
4120+
ClangImporter::Implementation::SwiftNameLookupExtension::hashExtension(
4121+
llvm::hash_code code) const {
41204122
return llvm::hash_combine(code, StringRef("swift.lookup"),
41214123
SWIFT_LOOKUP_TABLE_VERSION_MAJOR,
41224124
SWIFT_LOOKUP_TABLE_VERSION_MINOR,
4123-
OmitNeedlessWords,
4124-
InferDefaultArguments);
4125+
Impl.OmitNeedlessWords,
4126+
Impl.InferDefaultArguments);
41254127
}
41264128

41274129
std::unique_ptr<clang::ModuleFileExtensionWriter>
4128-
ClangImporter::Implementation::createExtensionWriter(clang::ASTWriter &writer) {
4129-
// Local function to populate the lookup table.
4130+
ClangImporter::Implementation::SwiftNameLookupExtension::createExtensionWriter(
4131+
clang::ASTWriter &writer) {
4132+
// Local function to populate the lookup table.
41304133
auto populateTable = [this](clang::Sema &sema, SwiftLookupTable &table) {
41314134
for (auto decl
41324135
: sema.Context.getTranslationUnitDecl()->noload_decls()) {
@@ -4138,19 +4141,19 @@ ClangImporter::Implementation::createExtensionWriter(clang::ASTWriter &writer) {
41384141
if (!named) continue;
41394142

41404143
// Add this entry to the lookup table.
4141-
addEntryToLookupTable(sema, table, named);
4144+
Impl.addEntryToLookupTable(sema, table, named);
41424145
}
41434146

41444147
// Add macros to the lookup table.
4145-
addMacrosToLookupTable(sema.Context, sema.getPreprocessor(), table);
4148+
Impl.addMacrosToLookupTable(sema.Context, sema.getPreprocessor(), table);
41464149
};
41474150

41484151
return std::unique_ptr<clang::ModuleFileExtensionWriter>(
41494152
new SwiftLookupTableWriter(this, populateTable, writer));
41504153
}
41514154

41524155
std::unique_ptr<clang::ModuleFileExtensionReader>
4153-
ClangImporter::Implementation::createExtensionReader(
4156+
ClangImporter::Implementation::SwiftNameLookupExtension::createExtensionReader(
41544157
const clang::ModuleFileExtensionMetadata &metadata,
41554158
clang::ASTReader &reader,
41564159
clang::serialization::ModuleFile &mod,
@@ -4163,13 +4166,13 @@ ClangImporter::Implementation::createExtensionReader(
41634166
assert(metadata.MinorVersion == SWIFT_LOOKUP_TABLE_VERSION_MINOR);
41644167

41654168
// Check whether we already have an entry in the set of lookup tables.
4166-
auto &entry = LookupTables[mod.ModuleName];
4169+
auto &entry = Impl.LookupTables[mod.ModuleName];
41674170
if (entry) return nullptr;
41684171

41694172
// Local function used to remove this entry when the reader goes away.
41704173
std::string moduleName = mod.ModuleName;
41714174
auto onRemove = [this, moduleName]() {
4172-
LookupTables.erase(moduleName);
4175+
Impl.LookupTables.erase(moduleName);
41734176
};
41744177

41754178
// Create the reader.

lib/ClangImporter/ImporterImpl.h

Lines changed: 32 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -220,10 +220,31 @@ using api_notes::FactoryAsInitKind;
220220

221221
/// \brief Implementation of the Clang importer.
222222
class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
223-
: public LazyMemberLoader, public clang::ModuleFileExtension
223+
: public LazyMemberLoader
224224
{
225225
friend class ClangImporter;
226226

227+
class SwiftNameLookupExtension : public clang::ModuleFileExtension {
228+
Implementation &Impl;
229+
230+
public:
231+
SwiftNameLookupExtension(Implementation &impl) : Impl(impl) { }
232+
233+
clang::ModuleFileExtensionMetadata getExtensionMetadata() const override;
234+
llvm::hash_code hashExtension(llvm::hash_code code) const override;
235+
236+
std::unique_ptr<clang::ModuleFileExtensionWriter>
237+
createExtensionWriter(clang::ASTWriter &writer) override;
238+
239+
std::unique_ptr<clang::ModuleFileExtensionReader>
240+
createExtensionReader(const clang::ModuleFileExtensionMetadata &metadata,
241+
clang::ASTReader &reader,
242+
clang::serialization::ModuleFile &mod,
243+
const llvm::BitstreamCursor &stream) override;
244+
245+
};
246+
friend class SwiftNameLookupExtension;
247+
227248
public:
228249
/// \brief Describes how a particular C enumeration type will be imported
229250
/// into Swift. All of the possibilities have the same storage
@@ -261,6 +282,16 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
261282
"<bridging-header-import>";
262283

263284
private:
285+
/// The Swift lookup table for the bridging header.
286+
SwiftLookupTable BridgingHeaderLookupTable;
287+
288+
/// The Swift lookup tables, per module.
289+
///
290+
/// Annoyingly, we list this table early so that it gets torn down after
291+
/// the underlying Clang instances that reference it
292+
/// (through the Swift name lookup module file extension).
293+
llvm::StringMap<std::unique_ptr<SwiftLookupTable>> LookupTables;
294+
264295
/// \brief A count of the number of load module operations.
265296
/// FIXME: Horrible, horrible hack for \c loadModule().
266297
unsigned ImportCounter = 0;
@@ -292,12 +323,6 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
292323
/// if type checking has begun.
293324
llvm::PointerIntPair<LazyResolver *, 1, bool> typeResolver;
294325

295-
/// The Swift lookup table for the bridging header.
296-
SwiftLookupTable BridgingHeaderLookupTable;
297-
298-
/// The Swift lookup tables, per module.
299-
llvm::StringMap<std::unique_ptr<SwiftLookupTable>> LookupTables;
300-
301326
public:
302327
/// \brief Mapping of already-imported declarations.
303328
llvm::DenseMap<const clang::Decl *, Decl *> ImportedDecls;
@@ -1283,20 +1308,6 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
12831308
return D;
12841309
}
12851310

1286-
// Module file extension overrides
1287-
1288-
clang::ModuleFileExtensionMetadata getExtensionMetadata() const override;
1289-
llvm::hash_code hashExtension(llvm::hash_code code) const override;
1290-
1291-
std::unique_ptr<clang::ModuleFileExtensionWriter>
1292-
createExtensionWriter(clang::ASTWriter &writer) override;
1293-
1294-
std::unique_ptr<clang::ModuleFileExtensionReader>
1295-
createExtensionReader(const clang::ModuleFileExtensionMetadata &metadata,
1296-
clang::ASTReader &reader,
1297-
clang::serialization::ModuleFile &mod,
1298-
const llvm::BitstreamCursor &stream) override;
1299-
13001311
/// Find the lookup table that corresponds to the given Clang module.
13011312
///
13021313
/// \param clangModule The module, or null to indicate that we're talking

0 commit comments

Comments
 (0)