Skip to content

Commit 33771cf

Browse files
authored
Merge pull request swiftlang#34387 from CodaFi/a-giraffe-on-a-beach
Add Compilation "Wave" Assertion in +Asserts Builds
2 parents 64ec60b + e2e19dc commit 33771cf

File tree

6 files changed

+70
-66
lines changed

6 files changed

+70
-66
lines changed

include/swift/AST/AbstractSourceFileDepGraphFactory.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#ifndef SWIFT_AST_SOURCE_FILE_DEP_GRAPH_CONSTRUCTOR_H
1414
#define SWIFT_AST_SOURCE_FILE_DEP_GRAPH_CONSTRUCTOR_H
1515

16+
#include "swift/AST/Decl.h"
1617
#include "swift/AST/DeclContext.h"
1718
#include "swift/AST/FineGrainedDependencies.h"
1819

@@ -72,6 +73,21 @@ class AbstractSourceFileDepGraphFactory {
7273
Optional<StringRef> fingerprint);
7374

7475
void addAUsedDecl(const DependencyKey &def, const DependencyKey &use);
76+
77+
static Optional<std::string> getFingerprintIfAny(
78+
std::pair<const NominalTypeDecl *, const ValueDecl *>) {
79+
return None;
80+
}
81+
82+
static Optional<std::string> getFingerprintIfAny(const Decl *d) {
83+
if (const auto *idc = dyn_cast<IterableDeclContext>(d)) {
84+
auto result = idc->getBodyFingerprint();
85+
assert((!result || !result->empty()) &&
86+
"Fingerprint should never be empty");
87+
return result;
88+
}
89+
return None;
90+
}
7591
};
7692

7793
} // namespace fine_grained_dependencies

include/swift/AST/Decl.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -950,9 +950,6 @@ class alignas(1 << DeclAlignInBits) Decl {
950950
/// If this returns true, the decl can be safely casted to ValueDecl.
951951
bool isPotentiallyOverridable() const;
952952

953-
/// Returns true if this Decl cannot be seen by any other source file
954-
bool isPrivateToEnclosingFile() const;
955-
956953
/// Retrieve the global actor attribute that applies to this declaration,
957954
/// if any.
958955
///

include/swift/Driver/Job.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,23 @@ class Job {
308308
/// The modification time of the main input file, if any.
309309
llvm::sys::TimePoint<> InputModTime = llvm::sys::TimePoint<>::max();
310310

311+
#ifndef NDEBUG
312+
/// The "wave" of incremental jobs that this \c Job was scheduled into.
313+
///
314+
/// The first "wave" of jobs is computed by the driver from the set of inputs
315+
/// and external files that have been mutated by the user. From there, as
316+
/// jobs from the first wave finish executing, we reload their \c swiftdeps
317+
/// files and re-integrate them into the dependency graph to discover
318+
/// the jobs for the second "wave".
319+
///
320+
/// In +asserts builds, we ensure that no more than two "waves" occur for
321+
/// any given incremental compilation session. This is a consequence of
322+
/// 1) transitivity in dependency arcs
323+
/// 2) dependency tracing from uses that affect a def's interfaces to that
324+
/// def's uses.
325+
mutable unsigned Wave = 1;
326+
#endif
327+
311328
public:
312329
Job(const JobAction &Source, SmallVectorImpl<const Job *> &&Inputs,
313330
std::unique_ptr<CommandOutput> Output, const char *Executable,
@@ -399,6 +416,11 @@ class Job {
399416
/// Assumes that, if a compile job, has one primary swift input
400417
/// May return empty if none.
401418
StringRef getFirstSwiftPrimaryInput() const;
419+
420+
#ifndef NDEBUG
421+
unsigned getWave() const { return Wave; }
422+
void setWave(unsigned WaveNum) const { Wave = WaveNum; }
423+
#endif
402424
};
403425

404426
/// A BatchJob comprises a _set_ of jobs, each of which is sufficiently similar

lib/AST/FrontendSourceFileDepGraphFactory.cpp

Lines changed: 11 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,13 @@ std::string FrontendSourceFileDepGraphFactory::getFingerprint(SourceFile *SF) {
229229
return getInterfaceHash(SF);
230230
}
231231

232+
std::string
233+
FrontendSourceFileDepGraphFactory::getInterfaceHash(SourceFile *SF) {
234+
llvm::SmallString<32> interfaceHash;
235+
SF->getInterfaceHash(interfaceHash);
236+
return interfaceHash.str().str();
237+
}
238+
232239
//==============================================================================
233240
// MARK: FrontendSourceFileDepGraphFactory - adding collections of defined Decls
234241
//==============================================================================
@@ -407,7 +414,8 @@ template <NodeKind kind, typename ContentsT>
407414
void FrontendSourceFileDepGraphFactory::addAllDefinedDeclsOfAGivenType(
408415
std::vector<ContentsT> &contentsVec) {
409416
for (const auto &declOrPair : contentsVec) {
410-
Optional<std::string> fp = getFingerprintIfAny(declOrPair);
417+
Optional<std::string> fp =
418+
AbstractSourceFileDepGraphFactory::getFingerprintIfAny(declOrPair);
411419
addADefinedDecl(
412420
DependencyKey::createForProvidedEntityInterface<kind>(declOrPair),
413421
fp ? StringRef(fp.getValue()) : Optional<StringRef>());
@@ -519,33 +527,6 @@ void FrontendSourceFileDepGraphFactory::addAllUsedDecls() {
519527
.enumerateAllUses();
520528
}
521529

522-
//==============================================================================
523-
// MARK: FrontendSourceFileDepGraphFactory - adding individual defined Decls
524-
//==============================================================================
525-
526-
std::string
527-
FrontendSourceFileDepGraphFactory::getInterfaceHash(SourceFile *SF) {
528-
llvm::SmallString<32> interfaceHash;
529-
SF->getInterfaceHash(interfaceHash);
530-
return interfaceHash.str().str();
531-
}
532-
533-
/// At present, only \c NominalTypeDecls have (body) fingerprints
534-
Optional<std::string> FrontendSourceFileDepGraphFactory::getFingerprintIfAny(
535-
std::pair<const NominalTypeDecl *, const ValueDecl *>) {
536-
return None;
537-
}
538-
Optional<std::string>
539-
FrontendSourceFileDepGraphFactory::getFingerprintIfAny(const Decl *d) {
540-
if (const auto *idc = dyn_cast<IterableDeclContext>(d)) {
541-
auto result = idc->getBodyFingerprint();
542-
assert((!result || !result->empty()) &&
543-
"Fingerprint should never be empty");
544-
return result;
545-
}
546-
return None;
547-
}
548-
549530
//==============================================================================
550531
// MARK: ModuleDepGraphFactory
551532
//==============================================================================
@@ -591,29 +572,10 @@ template <NodeKind kind, typename ContentsT>
591572
void ModuleDepGraphFactory::addAllDefinedDeclsOfAGivenType(
592573
std::vector<ContentsT> &contentsVec) {
593574
for (const auto &declOrPair : contentsVec) {
594-
Optional<std::string> fp = getFingerprintIfAny(declOrPair);
575+
Optional<std::string> fp =
576+
AbstractSourceFileDepGraphFactory::getFingerprintIfAny(declOrPair);
595577
addADefinedDecl(
596578
DependencyKey::createForProvidedEntityInterface<kind>(declOrPair),
597579
fp ? StringRef(fp.getValue()) : Optional<StringRef>());
598580
}
599581
}
600-
601-
//==============================================================================
602-
// MARK: ModuleDepGraphFactory - adding individual defined Decls
603-
//==============================================================================
604-
605-
/// At present, only \c NominalTypeDecls have (body) fingerprints
606-
Optional<std::string> ModuleDepGraphFactory::getFingerprintIfAny(
607-
std::pair<const NominalTypeDecl *, const ValueDecl *>) {
608-
return None;
609-
}
610-
Optional<std::string>
611-
ModuleDepGraphFactory::getFingerprintIfAny(const Decl *d) {
612-
if (const auto *idc = dyn_cast<IterableDeclContext>(d)) {
613-
auto result = idc->getBodyFingerprint();
614-
assert((!result || !result->empty()) &&
615-
"Fingerprint should never be empty");
616-
return result;
617-
}
618-
return None;
619-
}

lib/AST/FrontendSourceFileDepGraphFactory.h

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,6 @@ class FrontendSourceFileDepGraphFactory
4444
/// create node pairs for context and name
4545
template <NodeKind kind, typename ContentsT>
4646
void addAllDefinedDeclsOfAGivenType(std::vector<ContentsT> &contentsVec);
47-
48-
/// At present, only nominals, protocols, and extensions have (body)
49-
/// fingerprints
50-
static Optional<std::string>
51-
getFingerprintIfAny(std::pair<const NominalTypeDecl *, const ValueDecl *>);
52-
static Optional<std::string> getFingerprintIfAny(const Decl *d);
5347
};
5448

5549
class ModuleDepGraphFactory : public AbstractSourceFileDepGraphFactory {
@@ -68,12 +62,6 @@ class ModuleDepGraphFactory : public AbstractSourceFileDepGraphFactory {
6862
/// create node pairs for context and name
6963
template <NodeKind kind, typename ContentsT>
7064
void addAllDefinedDeclsOfAGivenType(std::vector<ContentsT> &contentsVec);
71-
72-
/// At present, only nominals, protocols, and extensions have (body)
73-
/// fingerprints
74-
static Optional<std::string> getFingerprintIfAny(
75-
std::pair<const NominalTypeDecl *, const ValueDecl *>);
76-
static Optional<std::string> getFingerprintIfAny(const Decl *d);
7765
};
7866

7967
} // namespace fine_grained_dependencies

lib/Driver/Compilation.cpp

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,17 @@ namespace driver {
323323
return;
324324
}
325325

326+
#ifndef NDEBUG
327+
// If we can, assert that no compile jobs are scheduled beyond the second
328+
// wave. If this assertion fails, it indicates one of:
329+
// 1) A failure of the driver's job tracing machinery to follow a
330+
// dependency arc.
331+
// 2) A failure of the frontend to emit a dependency arc.
332+
if (isa<CompileJobAction>(Cmd->getSource()) && Cmd->getWave() > 2) {
333+
llvm_unreachable("Scheduled a command into a third wave!");
334+
}
335+
#endif
336+
326337
// Adding to scheduled means we've committed to its completion (not
327338
// distinguished from skipping). We never remove it once inserted.
328339
ScheduledCommands.insert(Cmd);
@@ -694,8 +705,16 @@ namespace driver {
694705
"because of dependencies discovered later");
695706

696707
scheduleCommandsInSortedOrder(DependentsInEffect);
697-
for (const Job *Cmd : DependentsInEffect)
698-
DeferredCommands.erase(Cmd);
708+
for (const Job *Cmd : DependentsInEffect) {
709+
if (DeferredCommands.erase(Cmd)) {
710+
#ifndef NDEBUG
711+
if (isa<CompileJobAction>(FinishedCmd->getSource()))
712+
Cmd->setWave(FinishedCmd->getWave() + 1);
713+
#else
714+
continue;
715+
#endif
716+
}
717+
}
699718
return TaskFinishedResponse::ContinueExecution;
700719
}
701720

0 commit comments

Comments
 (0)