Skip to content

Commit ee723cd

Browse files
committed
Document Some Whys
* Document a number of legacy conditions and edge cases * Add lexicon definitions for "dependency source", "dependency sink", "cascading dependency" and "private dependency"
1 parent 42cfc7e commit ee723cd

File tree

6 files changed

+78
-9
lines changed

6 files changed

+78
-9
lines changed

docs/Lexicon.rst

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,17 @@ source code, tests, and commit messages. See also the `LLVM lexicon`_.
7777
same; the exception is when generics get involved. In this case you'll need
7878
a `generic environment`. Contrast with `sugared type`.
7979

80+
cascading dependency
81+
A kind of dependency edge relevant to the incremental name tracking
82+
subsystem. A cascading dependency (as opposed to a
83+
`private dependency <private dependency>`) requires the Swift driver to
84+
transitively consider dependency edges in the file that defines the used
85+
name when incremental compilation is enabled. A cascading dependency is much
86+
safer to produce than its private counterpart, but it comes at the cost of
87+
increased usage of compilation resources - even if those resources are being
88+
wasted on rebuilding a file that didn't actually require rebuilding.
89+
See :doc:`DependencyAnalysis.rst <DependencyAnalysis>`.
90+
8091
Clang importer
8192
The part of the compiler that reads C and Objective-C declarations and
8293
exposes them as Swift. Essentially contains a small instance of Clang
@@ -100,6 +111,24 @@ source code, tests, and commit messages. See also the `LLVM lexicon`_.
100111
i.e. one that conforming types don't *have* to implement but have the option
101112
to "customize".
102113

114+
dependency sink
115+
Any request that uses a matching dependency source to write dependency
116+
edges into the referenced name trackers. For example, a request that
117+
performs direct lookup will write the name being looked up into the
118+
name tracker associated with the file that issued the lookup request.
119+
The request evaluator automatically determines the appropriate tracker
120+
for a dependency sink to write into based on the current active
121+
`dependency source <dependency source>` request.
122+
123+
dependency source
124+
Any request that defines a scope under which reference dependencies may be
125+
registered. For example, a request to type check an entire file is a
126+
dependency source. Dependency sources are automatically managed by the
127+
request evaluator as request evaluation proceeds. Dependency sources provide
128+
one half of the necessary information to complete a full dependency edge.
129+
The other half is provided by corresponding
130+
`dependency sink <dependency sink>` requests.
131+
103132
DI (definite initialization / definitive initialization)
104133
The feature that no uninitialized variables, constants, or properties will
105134
be read by a program, or the analysis pass that operates on SIL to
@@ -327,6 +356,17 @@ source code, tests, and commit messages. See also the `LLVM lexicon`_.
327356
only needed for context. See also
328357
`Whole-Module Optimization <WMO (whole-module optimization)>`.
329358

359+
private dependency
360+
A kind of dependency edge relevant to the incremental name tracking
361+
subsystem. A private dependency (as opposed to a
362+
`cascading dependency <cascading dependency>`) declares a dependency edge
363+
from one file to a name referenced in that file that does not
364+
require further transitive evaluation of dependency edges by the Swift
365+
driver. Private dependencies are therefore cheaper than cascading
366+
dependencies, but must be used with the utmost care or dependent files will
367+
fail to rebuild and the result will most certainly be a miscompile.
368+
See :doc:`DependencyAnalysis.rst <DependencyAnalysis>`.
369+
330370
QoI
331371
"Quality of implementation." The term is meant to describe not how
332372
well-engineered a particular implementation is, but how much value it

include/swift/AST/Evaluator.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@ class Evaluator {
483483
/// If there is an active dependency source, returns its
484484
/// \c ReferencedNameTracker. Else, returns \c nullptr.
485485
ReferencedNameTracker *getActiveDependencyTracker() const {
486-
if (auto *source = getActiveDependencySource())
486+
if (auto *source = getActiveDependencySourceOrNull())
487487
return source->getRequestBasedReferencedNameTracker();
488488
return nullptr;
489489
}
@@ -508,7 +508,12 @@ class Evaluator {
508508

509509
/// Returns the active dependency's source file, or \c nullptr if no
510510
/// dependency source is active.
511-
SourceFile *getActiveDependencySource() const {
511+
///
512+
/// The use of this accessor is strongly discouraged, as it implies that a
513+
/// dependency sink is seeking to filter out names based on the files they
514+
/// come from. Existing callers are being migrated to more reasonable ways
515+
/// of judging the relevancy of a dependency.
516+
SourceFile *getActiveDependencySourceOrNull() const {
512517
if (dependencySources.empty())
513518
return nullptr;
514519
return dependencySources.back().getPointer();

include/swift/AST/EvaluatorDependencies.h

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,24 @@ enum class DependencyScope : bool {
6161
};
6262

6363
/// Returns a \c DependencyScope appropriate for the given (formal) access level.
64+
///
65+
/// :warning: This function exists to bridge the old manual reference
66+
/// dependencies code to the new evaluator-based reference dependencies code.
67+
/// The manual code often made private/cascading scope judgements based on the
68+
/// access level of a declaration. While this makes some sense intuitively, it
69+
/// does not necessarily capture an accurate picture of where real incremental
70+
/// dependencies lie. For example, references to formally private types can
71+
/// "escape" to contexts that have no reference to the private name if, say,
72+
/// the layout of that private type is taken into consideration by
73+
/// SILGen or IRGen in a separate file that references the declaration
74+
/// transitively. However, due to the density of the current dependency
75+
/// graph, redundancy in registered dependency edges, and the liberal use of
76+
/// cascading edges, we may be saved from the worst consequences of this
77+
/// modelling choice.
78+
///
79+
/// The use of access-levels for dependency decisions is an anti-pattern that
80+
/// should be revisited once finer-grained dependencies are explored more
81+
/// thoroughly.
6482
inline DependencyScope getScopeForAccessLevel(AccessLevel l) {
6583
switch (l) {
6684
case AccessLevel::Private:

include/swift/AST/SimpleRequest.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,19 @@ enum class RequestFlags {
4646
///
4747
/// This bit is optional. High-level requests
4848
/// (e.g. \c TypeCheckSourceFileRequest) will require it.
49+
///
50+
/// For further discussion on incremental dependencies
51+
/// see DependencyAnalysis.rst.
4952
DependencySource = 1 << 3,
5053
/// This request introduces the sink component of a source-sink
5154
/// incremental dependency pair and is a consumer of the current
5255
/// dependency scope.
5356
///
5457
/// This bit is optional. Name lookup requests
5558
/// (e.g. \c DirectLookupRequest) will require it.
59+
///
60+
/// For further discussion on incremental dependencies
61+
/// see DependencyAnalysis.rst.
5662
DependencySink = 1 << 4,
5763
};
5864

lib/AST/NameLookupRequests.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ InheritedProtocolsRequest::readDependencySource(Evaluator &e) const {
9898
if (!PD->getParentSourceFile())
9999
return { nullptr, e.getActiveSourceScope() };
100100
return {
101-
e.getActiveDependencySource(),
101+
e.getActiveDependencySourceOrNull(),
102102
evaluator::getScopeForAccessLevel(PD->getFormalAccess())
103103
};
104104
}
@@ -186,7 +186,7 @@ void ExtendedNominalRequest::writeDependencySink(
186186
auto *SF = std::get<0>(getStorage())->getParentSourceFile();
187187
if (!SF)
188188
return;
189-
if (SF != eval.getActiveDependencySource())
189+
if (SF != eval.getActiveDependencySourceOrNull())
190190
return;
191191
tracker.addUsedMember({value, Identifier()},
192192
eval.isActiveSourceCascading());
@@ -217,7 +217,7 @@ GetDestructorRequest::readDependencySource(Evaluator &eval) const {
217217
// valid 'deinit' declaration cannot occur outside of the
218218
// definition of a type.
219219
return {
220-
eval.getActiveDependencySource(),
220+
eval.getActiveDependencySourceOrNull(),
221221
evaluator::DependencyScope::Private
222222
};
223223
}
@@ -393,7 +393,7 @@ void LookupConformanceInModuleRequest::writeDependencySink(
393393
if (!Adoptee)
394394
return;
395395

396-
auto *source = eval.getActiveDependencySource();
396+
auto *source = eval.getActiveDependencySourceOrNull();
397397
assert(source && "Missing dependency source?");
398398

399399
// Decline to record conformances defined outside of the active module.

lib/AST/TypeCheckRequests.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ evaluator::DependencySource
161161
SuperclassTypeRequest::readDependencySource(Evaluator &e) const {
162162
const auto access = std::get<0>(getStorage())->getFormalAccess();
163163
return {
164-
e.getActiveDependencySource(),
164+
e.getActiveDependencySourceOrNull(),
165165
evaluator::getScopeForAccessLevel(access)
166166
};
167167
}
@@ -1348,7 +1348,7 @@ LookupAllConformancesInContextRequest::readDependencySource(
13481348
const NominalTypeDecl *nominal = ext->getExtendedNominal();
13491349
if (!nominal) {
13501350
return {
1351-
eval.getActiveDependencySource(),
1351+
eval.getActiveDependencySourceOrNull(),
13521352
evaluator::DependencyScope::Cascading
13531353
};
13541354
}
@@ -1357,7 +1357,7 @@ LookupAllConformancesInContextRequest::readDependencySource(
13571357
defaultAccess = cast<NominalTypeDecl>(dc)->getFormalAccess();
13581358
}
13591359
return {
1360-
eval.getActiveDependencySource(),
1360+
eval.getActiveDependencySourceOrNull(),
13611361
evaluator::getScopeForAccessLevel(defaultAccess)
13621362
};
13631363
}

0 commit comments

Comments
 (0)