Skip to content

Commit 74765a8

Browse files
CodaFiCodaFi
committed
Remove Type Body Fingerprints Flags
This infrastructure has more than proven itself. Drop the code paths and tests supporting the status quo.
1 parent e903630 commit 74765a8

File tree

48 files changed

+581
-2018
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+581
-2018
lines changed

‎include/swift/Basic/LangOptions.h

-6
Original file line numberDiff line numberDiff line change
@@ -330,12 +330,6 @@ namespace swift {
330330
/// of active clauses aren't hoisted into the enclosing scope.
331331
bool DisablePoundIfEvaluation = false;
332332

333-
/// Instead of hashing tokens inside of NominalType and ExtensionBodies into
334-
/// the interface hash, hash them into per-iterable-decl-context
335-
/// fingerprints. Fine-grained dependency types won't dirty every provides
336-
/// in a file when the user adds a member to, e.g., a struct.
337-
bool EnableTypeFingerprints = true;
338-
339333
/// When using fine-grained dependencies, emit dot files for every swiftdeps
340334
/// file.
341335
bool EmitFineGrainedDependencySourcefileDotFiles = false;

‎include/swift/Driver/Compilation.h

-7
Original file line numberDiff line numberDiff line change
@@ -265,9 +265,6 @@ class Compilation {
265265
const bool OnlyOneDependencyFile;
266266

267267
private:
268-
/// Is the parser recording token hashes for each type body?
269-
const bool EnableTypeFingerprints;
270-
271268
/// Helpful for debugging, but slows down the driver. So, only turn on when
272269
/// needed.
273270
const bool VerifyFineGrainedDependencyGraphAfterEveryImport;
@@ -319,8 +316,6 @@ class Compilation {
319316
bool ShowDriverTimeCompilation = false,
320317
std::unique_ptr<UnifiedStatsReporter> Stats = nullptr,
321318
bool OnlyOneDependencyFile = false,
322-
bool EnableTypeFingerprints =
323-
LangOptions().EnableTypeFingerprints,
324319
bool VerifyFineGrainedDependencyGraphAfterEveryImport = false,
325320
bool EmitFineGrainedDependencyDotFileAfterEveryImport = false,
326321
bool FineGrainedDependenciesIncludeIntrafileOnes = false,
@@ -386,8 +381,6 @@ class Compilation {
386381
}
387382
void disableIncrementalBuild(Twine why);
388383

389-
bool getEnableTypeFingerprints() const { return EnableTypeFingerprints; }
390-
391384
bool getVerifyFineGrainedDependencyGraphAfterEveryImport() const {
392385
return VerifyFineGrainedDependencyGraphAfterEveryImport;
393386
}

‎include/swift/Driver/FineGrainedDependencyDriverGraph.h

+2-7
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,6 @@ class ModuleDepGraph {
189189
const bool verifyFineGrainedDependencyGraphAfterEveryImport;
190190
const bool emitFineGrainedDependencyDotFileAfterEveryImport;
191191

192-
const bool EnableTypeFingerprints;
193-
194192
private:
195193
/// If tracing dependencies, holds a vector used to hold the current path
196194
/// def - use/def - use/def - ...
@@ -296,14 +294,12 @@ class ModuleDepGraph {
296294
/// \p stats may be null
297295
ModuleDepGraph(const bool verifyFineGrainedDependencyGraphAfterEveryImport,
298296
const bool emitFineGrainedDependencyDotFileAfterEveryImport,
299-
const bool EnableTypeFingerprints,
300297
const bool shouldTraceDependencies,
301298
UnifiedStatsReporter *stats)
302299
: verifyFineGrainedDependencyGraphAfterEveryImport(
303300
verifyFineGrainedDependencyGraphAfterEveryImport),
304301
emitFineGrainedDependencyDotFileAfterEveryImport(
305302
emitFineGrainedDependencyDotFileAfterEveryImport),
306-
EnableTypeFingerprints(EnableTypeFingerprints),
307303
currentPathIfTracing(
308304
shouldTraceDependencies
309305
? llvm::Optional<std::vector<const ModuleDepGraphNode *>>(
@@ -314,11 +310,10 @@ class ModuleDepGraph {
314310
}
315311

316312
/// For unit tests.
317-
ModuleDepGraph(const bool EnableTypeFingerprints,
318-
const bool EmitDotFilesForDebugging = false)
313+
ModuleDepGraph(const bool EmitDotFilesForDebugging = false)
319314
: ModuleDepGraph(
320315
true, /*emitFineGrainedDependencyDotFileAfterEveryImport=*/
321-
EmitDotFilesForDebugging, EnableTypeFingerprints, false, nullptr) {}
316+
EmitDotFilesForDebugging, false, nullptr) {}
322317

323318
//============================================================================
324319
// MARK: ModuleDepGraph - updating from a switdeps file

‎include/swift/Option/Options.td

-8
Original file line numberDiff line numberDiff line change
@@ -139,14 +139,6 @@ def driver_always_rebuild_dependents :
139139
Flag<["-"], "driver-always-rebuild-dependents">, InternalDebugOpt,
140140
HelpText<"Always rebuild dependents of files that have been modified">;
141141

142-
def enable_type_fingerprints :
143-
Flag<["-"], "enable-type-fingerprints">, Flags<[FrontendOption, HelpHidden]>,
144-
HelpText<"Enable per-nominal and extension body fingerprints">;
145-
146-
def disable_type_fingerprints :
147-
Flag<["-"], "disable-type-fingerprints">, Flags<[FrontendOption, HelpHidden]>,
148-
HelpText<"Disable per-nominal and extension body fingerprints">;
149-
150142
def enable_only_one_dependency_file :
151143
Flag<["-"], "enable-only-one-dependency-file">, Flags<[DoesNotAffectIncrementalBuild]>,
152144
HelpText<"Enables incremental build optimization that only produces one dependencies file">;

‎lib/AST/DeclContext.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -933,7 +933,7 @@ bool IterableDeclContext::areTokensHashedForThisBodyInsteadOfInterfaceHash()
933933
// corresponding to the fingerprinted nominal dependency node.
934934
if (isa<ExtensionDecl>(this))
935935
return false;
936-
return getASTContext().LangOpts.EnableTypeFingerprints;
936+
return true;
937937
}
938938

939939
/// Return the DeclContext to compare when checking private access in

‎lib/Driver/Compilation.cpp

+2-4
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,6 @@ Compilation::Compilation(DiagnosticEngine &Diags,
121121
bool ShowDriverTimeCompilation,
122122
std::unique_ptr<UnifiedStatsReporter> StatsReporter,
123123
bool OnlyOneDependencyFile,
124-
bool EnableTypeFingerprints,
125124
bool VerifyFineGrainedDependencyGraphAfterEveryImport,
126125
bool EmitFineGrainedDependencyDotFileAfterEveryImport,
127126
bool FineGrainedDependenciesIncludeIntrafileOnes,
@@ -149,7 +148,6 @@ Compilation::Compilation(DiagnosticEngine &Diags,
149148
Stats(std::move(StatsReporter)),
150149
FilelistThreshold(FilelistThreshold),
151150
OnlyOneDependencyFile(OnlyOneDependencyFile),
152-
EnableTypeFingerprints(EnableTypeFingerprints),
153151
VerifyFineGrainedDependencyGraphAfterEveryImport(
154152
VerifyFineGrainedDependencyGraphAfterEveryImport),
155153
EmitFineGrainedDependencyDotFileAfterEveryImport(
@@ -829,12 +827,12 @@ namespace driver {
829827
FineGrainedDepGraph(
830828
Comp.getVerifyFineGrainedDependencyGraphAfterEveryImport(),
831829
Comp.getEmitFineGrainedDependencyDotFileAfterEveryImport(),
832-
Comp.getEnableTypeFingerprints(), Comp.getTraceDependencies(),
830+
Comp.getTraceDependencies(),
833831
Comp.getStatsReporter()),
834832
FineGrainedDepGraphForRanges(
835833
Comp.getVerifyFineGrainedDependencyGraphAfterEveryImport(),
836834
Comp.getEmitFineGrainedDependencyDotFileAfterEveryImport(),
837-
Comp.getEnableTypeFingerprints(), Comp.getTraceDependencies(),
835+
Comp.getTraceDependencies(),
838836
Comp.getStatsReporter()),
839837
TQ(std::move(TaskQueue)) {}
840838

‎lib/Driver/Driver.cpp

-6
Original file line numberDiff line numberDiff line change
@@ -1014,11 +1014,6 @@ Driver::buildCompilation(const ToolChain &TC,
10141014
ArgList->hasFlag(options::OPT_enable_only_one_dependency_file,
10151015
options::OPT_disable_only_one_dependency_file, false);
10161016

1017-
const bool EnableTypeFingerprints =
1018-
ArgList->hasFlag(options::OPT_enable_type_fingerprints,
1019-
options::OPT_disable_type_fingerprints,
1020-
LangOptions().EnableTypeFingerprints);
1021-
10221017
const bool VerifyFineGrainedDependencyGraphAfterEveryImport = ArgList->hasArg(
10231018
options::
10241019
OPT_driver_verify_fine_grained_dependency_graph_after_every_import);
@@ -1050,7 +1045,6 @@ Driver::buildCompilation(const ToolChain &TC,
10501045
ShowDriverTimeCompilation,
10511046
std::move(StatsReporter),
10521047
OnlyOneDependencyFile,
1053-
EnableTypeFingerprints,
10541048
VerifyFineGrainedDependencyGraphAfterEveryImport,
10551049
EmitFineGrainedDependencyDotFileAfterEveryImport,
10561050
FineGrainedDependenciesIncludeIntrafileOnes,

‎lib/Driver/FineGrainedDependencyDriverGraph.cpp

-6
Original file line numberDiff line numberDiff line change
@@ -369,12 +369,6 @@ ModuleDepGraph::integrateSourceFileDepGraphNode(
369369
const SourceFileDepGraph &g, const SourceFileDepGraphNode *integrand,
370370
const PreexistingNodeIfAny preexistingMatch,
371371
const StringRef swiftDepsOfJob) {
372-
373-
if (!EnableTypeFingerprints &&
374-
integrand->getKey().getKind() != NodeKind::sourceFileProvide &&
375-
integrand->getFingerprint())
376-
return None;
377-
378372
if (!integrand->getIsProvides())
379373
return NullablePtr<ModuleDepGraphNode>(); // depends are captured by
380374
// recordWhatUseDependsUpon below

‎lib/Driver/ToolChains.cpp

-2
Original file line numberDiff line numberDiff line change
@@ -259,8 +259,6 @@ void ToolChain::addCommonFrontendArgs(const OutputInfo &OI,
259259
inputArgs.AddLastArg(arguments, options::OPT_O_Group);
260260
inputArgs.AddLastArg(arguments, options::OPT_RemoveRuntimeAsserts);
261261
inputArgs.AddLastArg(arguments, options::OPT_AssumeSingleThreaded);
262-
inputArgs.AddLastArg(arguments, options::OPT_enable_type_fingerprints);
263-
inputArgs.AddLastArg(arguments, options::OPT_disable_type_fingerprints);
264262
inputArgs.AddLastArg(arguments,
265263
options::OPT_fine_grained_dependency_include_intrafile);
266264
inputArgs.AddLastArg(arguments,

‎lib/Frontend/CompilerInvocation.cpp

-5
Original file line numberDiff line numberDiff line change
@@ -438,11 +438,6 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
438438
Opts.VerifySyntaxTree = true;
439439
}
440440

441-
Opts.EnableTypeFingerprints =
442-
Args.hasFlag(options::OPT_enable_type_fingerprints,
443-
options::OPT_disable_type_fingerprints,
444-
LangOptions().EnableTypeFingerprints);
445-
446441
if (Args.hasArg(OPT_emit_fine_grained_dependency_sourcefile_dot_files))
447442
Opts.EmitFineGrainedDependencySourcefileDotFiles = true;
448443

‎lib/IDE/CompletionInstance.cpp

-6
Original file line numberDiff line numberDiff line change
@@ -322,8 +322,6 @@ bool CompletionInstance::performCachedOperationIfPossible(
322322

323323
LangOptions langOpts = CI.getASTContext().LangOpts;
324324
langOpts.DisableParserLookup = true;
325-
// Ensure all non-function-body tokens are hashed into the interface hash
326-
langOpts.EnableTypeFingerprints = false;
327325
TypeCheckerOptions typeckOpts = CI.getASTContext().TypeCheckerOpts;
328326
SearchPathOptions searchPathOpts = CI.getASTContext().SearchPathOpts;
329327
DiagnosticEngine tmpDiags(tmpSM);
@@ -599,10 +597,6 @@ bool swift::ide::CompletionInstance::performOperation(
599597
// source text. That breaks an invariant of syntax tree building.
600598
Invocation.getLangOptions().BuildSyntaxTree = false;
601599

602-
// Since caching uses the interface hash, and since per type fingerprints
603-
// weaken that hash, disable them here:
604-
Invocation.getLangOptions().EnableTypeFingerprints = false;
605-
606600
// We don't need token list.
607601
Invocation.getLangOptions().CollectParsedToken = false;
608602

‎lib/Parse/ParseDecl.cpp

+7-2
Original file line numberDiff line numberDiff line change
@@ -4551,8 +4551,13 @@ Parser::parseDeclList(SourceLoc LBLoc, SourceLoc &RBLoc, Diag<> ErrorDiag,
45514551

45524552
// If we're hashing the type body separately, record the curly braces but
45534553
// nothing inside for the interface hash.
4554+
//
4555+
// FIXME: There's no real reason code completion cannot also use this code
4556+
// path. But it seems to cause lazy parsing in contexts that the current
4557+
// implementation does not expect.
45544558
Optional<llvm::SaveAndRestore<Optional<llvm::MD5>>> MemberHashingScope;
4555-
if (IDC->areTokensHashedForThisBodyInsteadOfInterfaceHash()) {
4559+
if (IDC->areTokensHashedForThisBodyInsteadOfInterfaceHash() &&
4560+
!L->isCodeCompletion()) {
45564561
recordTokenHash("{");
45574562
recordTokenHash("}");
45584563
MemberHashingScope.emplace(CurrentTokenHash, llvm::MD5());
@@ -4587,7 +4592,7 @@ Parser::parseDeclList(SourceLoc LBLoc, SourceLoc &RBLoc, Diag<> ErrorDiag,
45874592
if (RBLoc.isInvalid())
45884593
hadError = true;
45894594

4590-
if (!Context.LangOpts.EnableTypeFingerprints)
4595+
if (L->isCodeCompletion())
45914596
return std::make_pair(decls, None);
45924597

45934598
llvm::MD5::MD5Result result;

‎test/Frontend/PrivateFingerprints/class-fingerprint.swift

+3-42
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,6 @@
1-
21
// Test per-type-body fingerprints for classes
32
//
43

5-
// =============================================================================
6-
// Without the fingerprints
7-
// =============================================================================
8-
9-
// Establish status quo
10-
11-
12-
// RUN: %empty-directory(%t)
13-
// RUN: cp %S/Inputs/class-fingerprint/* %t
14-
// RUN: cp %t/definesAB{-before,}.swift
15-
16-
// Seeing weird failure on CI, so set the mod times
17-
// RUN: touch -t 200101010101 %t/*.swift
18-
19-
// RUN: cd %t && %target-swiftc_driver -disable-type-fingerprints -enable-batch-mode -j2 -incremental -driver-show-incremental main.swift definesAB.swift usesA.swift usesB.swift -module-name main -output-file-map ofm.json >&output1
20-
21-
// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB1.swiftdeps
22-
23-
// Change one type, but uses of all types get recompiled
24-
25-
// RUN: cp %t/definesAB{-after,}.swift
26-
27-
// Seeing weird failure on CI, so ensure that definesAB.swift is newer
28-
// RUN: touch -t 200201010101 %t/*
29-
// RUN: touch -t 200101010101 %t/*.swift
30-
// RUN: touch -t 200301010101 %t/definesAB.swift
31-
32-
// RUN: cd %t && %target-swiftc_driver -disable-type-fingerprints -enable-batch-mode -j2 -incremental -driver-show-incremental main.swift definesAB.swift usesA.swift usesB.swift -module-name main -output-file-map ofm.json >&output2
33-
34-
// Save for debugging:
35-
// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB1.swiftdeps
36-
37-
// RUN: %FileCheck -check-prefix=CHECK-MAINAB-RECOMPILED %s < %t/output2
38-
39-
// CHECK-MAINAB-RECOMPILED: Queuing (initial): {compile: definesAB.o <= definesAB.swift}
40-
// CHECK-MAINAB-RECOMPILED: Queuing because of dependencies discovered later: {compile: usesA.o <= usesA.swift}
41-
42-
// =============================================================================
43-
// With the fingerprints
44-
// =============================================================================
45-
464
// Establish status quo
475

486
// RUN: %empty-directory(%t)
@@ -71,3 +29,6 @@
7129
// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB4.swiftdeps
7230

7331
// RUN: %FileCheck -check-prefix=CHECK-MAINAB-RECOMPILED %s < %t/output4
32+
33+
// CHECK-MAINAB-RECOMPILED: Queuing (initial): {compile: definesAB.o <= definesAB.swift}
34+
// CHECK-MAINAB-RECOMPILED: Queuing because of dependencies discovered later: {compile: usesA.o <= usesA.swift}

‎test/Frontend/PrivateFingerprints/enum-fingerprint.swift

+3-42
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,6 @@
1-
21
// Test per-type-body fingerprints for enums
32
//
43

5-
// =============================================================================
6-
// Without the fingerprints
7-
// =============================================================================
8-
9-
// Establish status quo
10-
11-
12-
// RUN: %empty-directory(%t)
13-
// RUN: cp %S/Inputs/enum-fingerprint/* %t
14-
// RUN: cp %t/definesAB{-before,}.swift
15-
16-
// Seeing weird failure on CI, so set the mod times
17-
// RUN: touch -t 200101010101 %t/*.swift
18-
19-
// RUN: cd %t && %target-swiftc_driver -disable-type-fingerprints -enable-batch-mode -j2 -incremental -driver-show-incremental main.swift definesAB.swift usesA.swift usesB.swift -module-name main -output-file-map ofm.json >&output1
20-
21-
// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB1.swiftdeps
22-
23-
// Change one type, but uses of all types get recompiled
24-
25-
// RUN: cp %t/definesAB{-after,}.swift
26-
27-
// Seeing weird failure on CI, so ensure that definesAB.swift is newer
28-
// RUN: touch -t 200201010101 %t/*
29-
// RUN: touch -t 200101010101 %t/*.swift
30-
// RUN: touch -t 200301010101 %t/definesAB.swift
31-
32-
// RUN: cd %t && %target-swiftc_driver -disable-type-fingerprints -enable-batch-mode -j2 -incremental -driver-show-incremental main.swift definesAB.swift usesA.swift usesB.swift -module-name main -output-file-map ofm.json >&output2
33-
34-
// Save for debugging:
35-
// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB1.swiftdeps
36-
37-
// RUN: %FileCheck -check-prefix=CHECK-MAINAB-RECOMPILED %s < %t/output2
38-
39-
// CHECK-MAINAB-RECOMPILED: Queuing (initial): {compile: definesAB.o <= definesAB.swift}
40-
// CHECK-MAINAB-RECOMPILED: Queuing because of dependencies discovered later: {compile: usesA.o <= usesA.swift}
41-
42-
// =============================================================================
43-
// With the fingerprints
44-
// =============================================================================
45-
464
// Establish status quo
475

486
// RUN: %empty-directory(%t)
@@ -71,3 +29,6 @@
7129
// only-run-for-debugging: cp %t/usesB.swiftdeps %t/usesB4.swiftdeps
7230

7331
// RUN: %FileCheck -check-prefix=CHECK-MAINAB-RECOMPILED %s < %t/output4
32+
33+
// CHECK-MAINAB-RECOMPILED: Queuing (initial): {compile: definesAB.o <= definesAB.swift}
34+
// CHECK-MAINAB-RECOMPILED: Queuing because of dependencies discovered later: {compile: usesA.o <= usesA.swift}

0 commit comments

Comments
 (0)