Skip to content

Commit 4804f03

Browse files
committed
[NFC] Don't Store An llvm::function_ref
This class was relying on the caller to keep this member alive. In general, this will lead to startling memory ownership bugs if, say, the enumerators were returned from these functions. Pass it in as a parameter instead to formalize that contract.
1 parent 030ed2a commit 4804f03

File tree

1 file changed

+24
-29
lines changed

1 file changed

+24
-29
lines changed

lib/AST/FrontendSourceFileDepGraphFactory.cpp

Lines changed: 24 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -408,23 +408,17 @@ class UsedDeclEnumerator {
408408
/// Cache these for efficiency
409409
const DependencyKey sourceFileImplementation;
410410

411-
function_ref<void(const DependencyKey &, const DependencyKey &)> createDefUse;
412-
413411
public:
414-
UsedDeclEnumerator(
415-
const SourceFile *SF, const DependencyTracker &depTracker,
416-
StringRef swiftDeps,
417-
function_ref<void(const DependencyKey &, const DependencyKey &)>
418-
createDefUse)
412+
UsedDeclEnumerator(const SourceFile *SF, const DependencyTracker &depTracker,
413+
StringRef swiftDeps)
419414
: SF(SF), depTracker(depTracker), swiftDeps(swiftDeps),
420-
sourceFileInterface(DependencyKey::createKeyForWholeSourceFile(
421-
DeclAspect::interface, swiftDeps)),
422415
sourceFileImplementation(DependencyKey::createKeyForWholeSourceFile(
423-
DeclAspect::implementation, swiftDeps)),
424-
createDefUse(createDefUse) {}
416+
DeclAspect::implementation, swiftDeps)) {}
425417

426418
public:
427-
void enumerateAllUses() {
419+
using UseEnumerator =
420+
llvm::function_ref<void(const DependencyKey &, const DependencyKey &)>;
421+
void enumerateAllUses(UseEnumerator enumerator) {
428422
auto &Ctx = SF->getASTContext();
429423
Ctx.evaluator.enumerateReferencesInFile(SF, [&](const auto &ref) {
430424
std::string name = ref.name.userFacingName().str();
@@ -436,36 +430,37 @@ class UsedDeclEnumerator {
436430
case Kind::Tombstone:
437431
llvm_unreachable("Cannot enumerate dead reference!");
438432
case Kind::TopLevel:
439-
return enumerateUse<NodeKind::topLevel>("", name);
433+
return enumerateUse<NodeKind::topLevel>("", name, enumerator);
440434
case Kind::Dynamic:
441-
return enumerateUse<NodeKind::dynamicLookup>("", name);
435+
return enumerateUse<NodeKind::dynamicLookup>("", name, enumerator);
442436
case Kind::PotentialMember: {
443437
std::string context = DependencyKey::computeContextForProvidedEntity<
444438
NodeKind::potentialMember>(nominal);
445-
return enumerateUse<NodeKind::potentialMember>(context, "");
439+
return enumerateUse<NodeKind::potentialMember>(context, "", enumerator);
446440
}
447441
case Kind::UsedMember: {
448442
std::string context =
449443
DependencyKey::computeContextForProvidedEntity<NodeKind::member>(
450444
nominal);
451-
return enumerateUse<NodeKind::member>(context, name);
445+
return enumerateUse<NodeKind::member>(context, name, enumerator);
452446
}
453447
}
454448
});
455-
enumerateExternalUses();
456-
enumerateNominalUses();
449+
enumerateExternalUses(enumerator);
450+
enumerateNominalUses(enumerator);
457451
}
458452

459453
private:
460454
template <NodeKind kind>
461-
void enumerateUse(StringRef context, StringRef name) {
455+
void enumerateUse(StringRef context, StringRef name,
456+
UseEnumerator createDefUse) {
462457
// Assume that what is depended-upon is the interface
463458
createDefUse(
464459
DependencyKey(kind, DeclAspect::interface, context.str(), name.str()),
465460
sourceFileImplementation);
466461
}
467462

468-
void enumerateNominalUses() {
463+
void enumerateNominalUses(UseEnumerator enumerator) {
469464
auto &Ctx = SF->getASTContext();
470465
Ctx.evaluator.enumerateReferencesInFile(SF, [&](const auto &ref) {
471466
const NominalTypeDecl *subject = ref.subject;
@@ -476,26 +471,26 @@ class UsedDeclEnumerator {
476471
std::string context =
477472
DependencyKey::computeContextForProvidedEntity<NodeKind::nominal>(
478473
subject);
479-
enumerateUse<NodeKind::nominal>(context, "");
474+
enumerateUse<NodeKind::nominal>(context, "", enumerator);
480475
});
481476
}
482477

483-
void enumerateExternalUses() {
478+
void enumerateExternalUses(UseEnumerator enumerator) {
484479
for (StringRef s : depTracker.getIncrementalDependencies())
485-
enumerateUse<NodeKind::incrementalExternalDepend>("", s);
480+
enumerateUse<NodeKind::incrementalExternalDepend>("", s, enumerator);
486481

487482
for (StringRef s : depTracker.getDependencies())
488-
enumerateUse<NodeKind::externalDepend>("", s);
483+
enumerateUse<NodeKind::externalDepend>("", s, enumerator);
489484
}
490485
};
491486
} // end namespace
492487

493488
void FrontendSourceFileDepGraphFactory::addAllUsedDecls() {
494-
UsedDeclEnumerator(SF, depTracker, swiftDeps,
495-
[&](const DependencyKey &def, const DependencyKey &use) {
496-
addAUsedDecl(def, use);
497-
})
498-
.enumerateAllUses();
489+
UsedDeclEnumerator(SF, depTracker, swiftDeps)
490+
.enumerateAllUses(
491+
[&](const DependencyKey &def, const DependencyKey &use) {
492+
addAUsedDecl(def, use);
493+
});
499494
}
500495

501496
//==============================================================================

0 commit comments

Comments
 (0)