Skip to content

Commit 31cf1f2

Browse files
CodaFiCodaFi
authored
Merge pull request #32198 from CodaFi/depen-dots
A Raft of Fixes To Private Dependencies
2 parents fadfb6c + 975ff75 commit 31cf1f2

File tree

13 files changed

+147
-31
lines changed

13 files changed

+147
-31
lines changed

‎include/swift/AST/Evaluator.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,7 @@ class Evaluator {
439439
!Request::isDependencySink>::type * = nullptr>
440440
void reportEvaluatedResult(const Request &r,
441441
const typename Request::OutputType &o) {
442-
recorder.replay(ActiveRequest(r));
442+
recorder.replay(activeRequests, ActiveRequest(r));
443443
}
444444

445445
// Report the result of evaluating a request that is a dependency sink.

‎include/swift/AST/EvaluatorDependencies.h

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -267,10 +267,16 @@ struct DependencyRecorder {
267267
void realize(const DependencyCollector::Reference &ref);
268268

269269
public:
270-
void replay(const swift::ActiveRequest &req);
270+
void replay(const llvm::SetVector<swift::ActiveRequest> &stack,
271+
const swift::ActiveRequest &req);
271272
void record(const llvm::SetVector<swift::ActiveRequest> &stack,
272273
llvm::function_ref<void(DependencyCollector &)> rec);
273274

275+
private:
276+
void
277+
unionNearestCachedRequest(ArrayRef<swift::ActiveRequest> stack,
278+
const DependencyCollector::ReferenceSet &scratch);
279+
274280
public:
275281
using ReferenceEnumerator =
276282
llvm::function_ref<void(const DependencyCollector::Reference &)>;
@@ -298,7 +304,12 @@ struct DependencyRecorder {
298304
SourceFile *getActiveDependencySourceOrNull() const {
299305
if (dependencySources.empty())
300306
return nullptr;
301-
return dependencySources.back().getPointer();
307+
switch (mode) {
308+
case Mode::StatusQuo:
309+
return dependencySources.back().getPointer();
310+
case Mode::ExperimentalPrivateDependencies:
311+
return dependencySources.front().getPointer();
312+
}
302313
}
303314

304315
public:
@@ -331,14 +342,6 @@ struct DependencyRecorder {
331342
};
332343

333344
private:
334-
/// Returns the first dependency source registered with the tracker, or
335-
/// \c nullptr if no dependency sources have been registered.
336-
SourceFile *getFirstDependencySourceOrNull() const {
337-
if (dependencySources.empty())
338-
return nullptr;
339-
return dependencySources.front().getPointer();
340-
}
341-
342345
/// Returns \c true if the scope of the current active source cascades.
343346
///
344347
/// If there is no active scope, the result always cascades.

‎include/swift/AST/NameLookupRequests.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,7 @@ class LookupInModuleRequest
447447
QualifiedLookupResult(
448448
const DeclContext *, DeclName, NLKind,
449449
namelookup::ResolutionKind, const DeclContext *),
450-
RequestFlags::Uncached> {
450+
RequestFlags::Uncached | RequestFlags::DependencySink> {
451451
public:
452452
using SimpleRequest::SimpleRequest;
453453

@@ -459,6 +459,11 @@ class LookupInModuleRequest
459459
evaluate(Evaluator &evaluator, const DeclContext *moduleOrFile, DeclName name,
460460
NLKind lookupKind, namelookup::ResolutionKind resolutionKind,
461461
const DeclContext *moduleScopeContext) const;
462+
463+
public:
464+
// Incremental dependencies
465+
void writeDependencySink(evaluator::DependencyCollector &tracker,
466+
QualifiedLookupResult l) const;
462467
};
463468

464469
/// Perform \c AnyObject lookup for a given member.

‎lib/AST/Evaluator.cpp

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -446,23 +446,12 @@ void evaluator::DependencyRecorder::record(
446446
return;
447447
}
448448

449-
assert(mode != Mode::StatusQuo);
450-
for (const auto &request : stack) {
451-
if (!request.isCached()) {
452-
continue;
453-
}
454-
455-
auto entry = requestReferences.find_as(request);
456-
if (entry == requestReferences.end()) {
457-
requestReferences.insert({AnyRequest(request), collector.scratch});
458-
continue;
459-
}
460-
461-
entry->second.insert(collector.scratch.begin(), collector.scratch.end());
462-
}
449+
return unionNearestCachedRequest(stack.getArrayRef(), collector.scratch);
463450
}
464451

465-
void evaluator::DependencyRecorder::replay(const swift::ActiveRequest &req) {
452+
void evaluator::DependencyRecorder::replay(
453+
const llvm::SetVector<swift::ActiveRequest> &stack,
454+
const swift::ActiveRequest &req) {
466455
assert(!isRecording && "Probably not a good idea to allow nested recording");
467456

468457
auto *source = getActiveDependencySourceOrNull();
@@ -482,6 +471,51 @@ void evaluator::DependencyRecorder::replay(const swift::ActiveRequest &req) {
482471
for (const auto &ref : entry->second) {
483472
realize(ref);
484473
}
474+
475+
// N.B. This is a particularly subtle detail of the replay unioning step. The
476+
// evaluator does not push cached requests onto the active request stack,
477+
// so it is possible (and, in fact, quite likely) we'll wind up with an
478+
// empty request stack. The remaining troublesome case is when we have a
479+
// cached request being run through the uncached path - take the
480+
// InterfaceTypeRequest, which involves many component requests, most of which
481+
// are themselves cached. In such a case, the active stack will look like
482+
//
483+
// -> TypeCheckSourceFileRequest
484+
// -> ...
485+
// -> InterfaceTypeRequest
486+
// -> ...
487+
// -> UnderlyingTypeRequest
488+
//
489+
// We want the UnderlyingTypeRequest to union its names into the
490+
// InterfaceTypeRequest, and if we were to just start searching the active
491+
// stack backwards for a cached request we would find...
492+
// the UnderlyingTypeRequest! So, we'll just drop it from consideration.
493+
//
494+
// We do *not* have to consider this during the recording step because none
495+
// of the name lookup requests (or any dependency sinks in general) are
496+
// cached. Should this change in the future, we will need to sink this logic
497+
// into the union step itself.
498+
const size_t d = (!stack.empty() && req == stack.back()) ? 1 : 0;
499+
return unionNearestCachedRequest(stack.getArrayRef().drop_back(d),
500+
entry->second);
501+
}
502+
503+
void evaluator::DependencyRecorder::unionNearestCachedRequest(
504+
ArrayRef<swift::ActiveRequest> stack,
505+
const DependencyCollector::ReferenceSet &scratch) {
506+
assert(mode != Mode::StatusQuo);
507+
auto nearest = std::find_if(stack.rbegin(), stack.rend(),
508+
[](const auto &req){ return req.isCached(); });
509+
if (nearest == stack.rend()) {
510+
return;
511+
}
512+
513+
auto entry = requestReferences.find_as(*nearest);
514+
if (entry == requestReferences.end()) {
515+
requestReferences.insert({AnyRequest(*nearest), scratch});
516+
} else {
517+
entry->second.insert(scratch.begin(), scratch.end());
518+
}
485519
}
486520

487521
using namespace swift;

‎lib/AST/NameLookupRequests.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,24 @@ void DirectLookupRequest::writeDependencySink(
319319
tracker.addUsedMember(desc.DC, desc.Name.getBaseName());
320320
}
321321

322+
//----------------------------------------------------------------------------//
323+
// LookupInModuleRequest computation.
324+
//----------------------------------------------------------------------------//
325+
326+
void LookupInModuleRequest::writeDependencySink(
327+
evaluator::DependencyCollector &reqTracker, QualifiedLookupResult l) const {
328+
auto *module = std::get<0>(getStorage());
329+
auto member = std::get<1>(getStorage());
330+
auto *DC = std::get<4>(getStorage());
331+
332+
// Decline to record lookups outside our module.
333+
if (!DC->getParentSourceFile() ||
334+
module->getParentModule() != DC->getParentModule()) {
335+
return;
336+
}
337+
reqTracker.addTopLevelName(member.getBaseName());
338+
}
339+
322340
//----------------------------------------------------------------------------//
323341
// LookupConformanceInModuleRequest computation.
324342
//----------------------------------------------------------------------------//

‎test/Incremental/Verifier/multi-file-private/Inputs/Derived.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
11
final public class OpenSubclass: OpenBase {} // expected-provides {{OpenSubclass}} expected-private-superclass {{main.OpenBase}}
22
// expected-provides {{OpenBase}}
33
// expected-private-member {{main.OpenBase.init}}
4+
// expected-private-member {{main.OpenBase.deinit}}
45
// expected-private-member {{main.OpenSubclass.init}}
56
// expected-private-member {{main.OpenSubclass.deinit}}
67

78
final public class PublicSubclass: PublicBase {} // expected-provides {{PublicSubclass}} expected-private-superclass {{main.PublicBase}}
89
// expected-provides {{PublicBase}}
910
// expected-private-member {{main.PublicBase.init}}
11+
// expected-private-member {{main.PublicBase.deinit}}
1012
// expected-private-member {{main.PublicSubclass.init}}
1113
// expected-private-member {{main.PublicSubclass.deinit}}
1214

1315
final internal class InternalSubclass: InternalBase {} // expected-provides {{InternalSubclass}} expected-private-superclass {{main.InternalBase}}
1416
// expected-provides {{InternalBase}}
1517
// expected-private-member {{main.InternalBase.init}}
18+
// expected-private-member {{main.InternalBase.deinit}}
1619
// expected-private-member {{main.InternalSubclass.init}}
1720
// expected-private-member {{main.InternalSubclass.deinit}}
1821

‎test/Incremental/Verifier/multi-file-private/Inputs/Inner.swift

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// expected-provides{{Inner}}
2+
// expected-private-member{{main.Inner.init}}
3+
public struct Inner {}
4+
5+
// expected-provides{{Foo}}
6+
public typealias Foo = () -> (Inner)
7+
8+
// expected-provides{{blah}}
9+
public func blah(foo: Foo) {}
10+
11+
// expected-provides{{defaultFoo}}
12+
public var defaultFoo: Foo = {
13+
return Inner()
14+
}

‎test/Incremental/Verifier/multi-file-private/Inputs/UsesInner.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// expected-provides{{Inner}}
2+
// expected-provides{{defaultFoo}}
3+
// expected-provides{{blah}}
4+
// expected-provides{{Foo}}
5+
// expected-provides{{??}}
6+
public func blah(foo: Foo?) {
7+
blah(foo: foo ?? defaultFoo)
8+
}

‎test/Incremental/Verifier/multi-file-private/main.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// RUN: %empty-directory(%t)
22
// RUN: %{python} %S/../gen-output-file-map.py -o %t %S/Inputs -r %t.resp
3-
// RUN: cd %t && %target-swiftc_driver -typecheck -output-file-map %t/output.json -incremental -module-name main -experimental-private-intransitive-dependencies -verify-incremental-dependencies @%t.resp
4-
// RUN: cd %t && %target-swiftc_driver -typecheck -output-file-map %t/output.json -incremental -enable-batch-mode -module-name main -experimental-private-intransitive-dependencies -verify-incremental-dependencies @%t.resp
3+
// RUN: cd %t && %target-swiftc_driver -c -output-file-map %t/output.json -incremental -module-name main -experimental-private-intransitive-dependencies -verify-incremental-dependencies @%t.resp
4+
// RUN: cd %t && %target-swiftc_driver -c -output-file-map %t/output.json -incremental -enable-batch-mode -module-name main -experimental-private-intransitive-dependencies -verify-incremental-dependencies @%t.resp
55

66
// N.B. These tests are meant to continue to expand to more and more input files
77
// as more kinds of cross-type dependencies are discovered. This will naturally

‎test/Incremental/Verifier/multi-file/Inputs/Derived.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
11
final public class OpenSubclass: OpenBase {} // expected-provides {{OpenSubclass}} expected-cascading-superclass {{main.OpenBase}}
22
// expected-provides {{OpenBase}}
33
// expected-cascading-member {{main.OpenBase.init}}
4+
// expected-private-member {{main.OpenBase.deinit}}
45
// expected-cascading-member {{main.OpenSubclass.init}}
56
// expected-private-member {{main.OpenSubclass.deinit}}
67

78
final public class PublicSubclass: PublicBase {} // expected-provides {{PublicSubclass}} expected-cascading-superclass {{main.PublicBase}}
89
// expected-provides {{PublicBase}}
910
// expected-cascading-member {{main.PublicBase.init}}
11+
// expected-private-member {{main.PublicBase.deinit}}
1012
// expected-cascading-member {{main.PublicSubclass.init}}
1113
// expected-private-member {{main.PublicSubclass.deinit}}
1214

1315
final internal class InternalSubclass: InternalBase {} // expected-provides {{InternalSubclass}} expected-cascading-superclass {{main.InternalBase}}
1416
// expected-provides {{InternalBase}}
1517
// expected-cascading-member {{main.InternalBase.init}}
18+
// expected-private-member {{main.InternalBase.deinit}}
1619
// expected-cascading-member {{main.InternalSubclass.init}}
1720
// expected-private-member {{main.InternalSubclass.deinit}}
1821

‎test/Incremental/Verifier/multi-file/Inputs/Inner.swift

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// expected-provides{{Inner}}
2+
// expected-cascading-member{{main.Inner.init}}
3+
public struct Inner {}
4+
5+
// expected-provides{{Foo}}
6+
public typealias Foo = () -> (Inner)
7+
8+
// expected-provides{{blah}}
9+
public func blah(foo: Foo) {}
10+
11+
// expected-provides{{defaultFoo}}
12+
public var defaultFoo: Foo = {
13+
return Inner()
14+
}

‎test/Incremental/Verifier/multi-file/Inputs/UsesInner.swift

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// FIXME: This dependency ONLY occurs with private dependencies. Otherwise we
2+
// rely on the interface hash changing to cause this file to be rebuilt, which
3+
// will *not* work with type fingerprints enabled.
4+
// See rdar://63984581
5+
// fixme-provides{{Inner}}
6+
7+
// expected-provides{{defaultFoo}}
8+
// expected-provides{{blah}}
9+
// expected-provides{{Optional}}
10+
// expected-provides{{Foo}}
11+
// expected-provides{{??}}
12+
public func blah(foo: Optional<Foo>) {
13+
blah(foo: foo ?? defaultFoo)
14+
}

‎test/Incremental/Verifier/multi-file/main.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// RUN: %empty-directory(%t)
22
// RUN: %{python} %S/../gen-output-file-map.py -o %t %S/Inputs -r %t.resp
3-
// RUN: cd %t && %target-swiftc_driver -typecheck -output-file-map %t/output.json -incremental -module-name main -verify-incremental-dependencies @%t.resp
4-
// RUN: cd %t && %target-swiftc_driver -typecheck -output-file-map %t/output.json -incremental -enable-batch-mode -module-name main -verify-incremental-dependencies @%t.resp
3+
// RUN: cd %t && %target-swiftc_driver -c -output-file-map %t/output.json -incremental -module-name main -verify-incremental-dependencies @%t.resp
4+
// RUN: cd %t && %target-swiftc_driver -c -output-file-map %t/output.json -incremental -enable-batch-mode -module-name main -verify-incremental-dependencies @%t.resp
55

66
// N.B. These tests are meant to continue to expand to more and more input files
77
// as more kinds of cross-type dependencies are discovered. This will naturally

0 commit comments

Comments
 (0)