Skip to content

Commit a6e1734

Browse files
authored
Merge pull request #9884 from huonw/symbol-list-8
Improve TBD validation logic
2 parents 20a815c + 79c29db commit a6e1734

File tree

14 files changed

+102
-31
lines changed

14 files changed

+102
-31
lines changed

include/swift/Frontend/FrontendOptions.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,15 @@ class FrontendOptions {
271271
/// variables by name when we print it out. This eases diffing of SIL files.
272272
bool EmitSortedSIL = false;
273273

274+
/// The different modes for validating TBD against the LLVM IR.
275+
enum class TBDValidationMode {
276+
None, ///< Do no validation.
277+
MissingFromTBD, ///< Only check for symbols that are in IR but not TBD.
278+
All, ///< Check for symbols that are in IR but not TBD and TBD but not IR.
279+
};
280+
274281
/// Compare the symbols in the IR against the TBD file we would generate.
275-
bool ValidateTBDAgainstIR = false;
282+
TBDValidationMode ValidateTBDAgainstIR = TBDValidationMode::None;
276283

277284
/// An enum with different modes for automatically crashing at defined times.
278285
enum class DebugCrashMode {

include/swift/Option/FrontendOptions.td

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -421,8 +421,9 @@ def group_info_path : Separate<["-"], "group-info-path">,
421421
def diagnostics_editor_mode : Flag<["-"], "diagnostics-editor-mode">,
422422
HelpText<"Diagnostics will be used in editor">;
423423

424-
def validate_tbd_against_ir: Flag<["-"], "validate-tbd-against-ir">,
425-
HelpText<"Compare the symbols in the IR against the TBD file that would be generated.">;
424+
def validate_tbd_against_ir_EQ: Joined<["-"], "validate-tbd-against-ir=">,
425+
HelpText<"Compare the symbols in the IR against the TBD file that would be generated.">,
426+
MetaVarName<"<level>">;
426427

427428
// FIXME: Remove this flag when void subscripts are implemented.
428429
// This is used to guard preemptive testing for the fix-it.

lib/Frontend/CompilerInvocation.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,20 @@ static bool ParseFrontendArgs(FrontendOptions &Opts, ArgList &Args,
191191
Opts.StatsOutputDir = A->getValue();
192192
}
193193

194-
Opts.ValidateTBDAgainstIR |= Args.hasArg(OPT_validate_tbd_against_ir);
194+
if (const Arg *A = Args.getLastArg(OPT_validate_tbd_against_ir_EQ)) {
195+
using Mode = FrontendOptions::TBDValidationMode;
196+
StringRef value = A->getValue();
197+
if (value == "none") {
198+
Opts.ValidateTBDAgainstIR = Mode::None;
199+
} else if (value == "missing") {
200+
Opts.ValidateTBDAgainstIR = Mode::MissingFromTBD;
201+
} else if (value == "all") {
202+
Opts.ValidateTBDAgainstIR = Mode::All;
203+
} else {
204+
Diags.diagnose(SourceLoc(), diag::error_unsupported_option_argument,
205+
A->getOption().getPrefixedName(), value);
206+
}
207+
}
195208

196209
if (const Arg *A = Args.getLastArg(OPT_warn_long_function_bodies)) {
197210
unsigned attempt;

lib/FrontendTool/FrontendTool.cpp

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
#include "swift/Migrator/Migrator.h"
5454
#include "swift/PrintAsObjC/PrintAsObjC.h"
5555
#include "swift/Serialization/SerializationOptions.h"
56+
#include "swift/Serialization/SerializedModuleLoader.h"
5657
#include "swift/SILOptimizer/PassManager/Passes.h"
5758

5859
// FIXME: We're just using CompilerInstance::createOutputFile.
@@ -673,19 +674,30 @@ static bool performCompile(CompilerInstance &Instance,
673674
"All actions not requiring SILGen must have been handled!");
674675

675676
std::unique_ptr<SILModule> SM = Instance.takeSILModule();
677+
// Records whether the SIL is directly computed from the AST we have, meaning
678+
// that it will exactly match the source. It might not if, for instance, some
679+
// of the inputs are SIB with extra explicit SIL.
680+
auto astGuaranteedToCorrespondToSIL = false;
676681
if (!SM) {
682+
auto fileIsSIB = [](const FileUnit *File) -> bool {
683+
auto SASTF = dyn_cast<SerializedASTFile>(File);
684+
return SASTF && SASTF->isSIB();
685+
};
677686
if (opts.PrimaryInput.hasValue() && opts.PrimaryInput.getValue().isFilename()) {
678687
FileUnit *PrimaryFile = PrimarySourceFile;
679688
if (!PrimaryFile) {
680689
auto Index = opts.PrimaryInput.getValue().Index;
681690
PrimaryFile = Instance.getMainModule()->getFiles()[Index];
682691
}
692+
astGuaranteedToCorrespondToSIL = !fileIsSIB(PrimaryFile);
683693
SM = performSILGeneration(*PrimaryFile, Invocation.getSILOptions(),
684694
None, opts.SILSerializeAll);
685695
} else {
686-
SM = performSILGeneration(Instance.getMainModule(), Invocation.getSILOptions(),
687-
opts.SILSerializeAll,
688-
true);
696+
auto mod = Instance.getMainModule();
697+
astGuaranteedToCorrespondToSIL =
698+
llvm::none_of(mod->getFiles(), fileIsSIB);
699+
SM = performSILGeneration(mod, Invocation.getSILOptions(),
700+
opts.SILSerializeAll, true);
689701
}
690702
}
691703

@@ -917,17 +929,31 @@ static bool performCompile(CompilerInstance &Instance,
917929
countStatsPostIRGen(*Stats, *IRModule);
918930
}
919931

920-
if (opts.ValidateTBDAgainstIR) {
932+
bool allSymbols = false;
933+
switch (opts.ValidateTBDAgainstIR) {
934+
case FrontendOptions::TBDValidationMode::None:
935+
break;
936+
case FrontendOptions::TBDValidationMode::All:
937+
allSymbols = true;
938+
LLVM_FALLTHROUGH;
939+
case FrontendOptions::TBDValidationMode::MissingFromTBD: {
940+
if (!inputFileKindCanHaveTBDValidated(Invocation.getInputKind()) ||
941+
!astGuaranteedToCorrespondToSIL)
942+
break;
943+
921944
auto hasMultipleIRGenThreads = Invocation.getSILOptions().NumThreads > 1;
922945
bool error;
923946
if (PrimarySourceFile)
924-
error =
925-
validateTBD(PrimarySourceFile, *IRModule, hasMultipleIRGenThreads);
947+
error = validateTBD(PrimarySourceFile, *IRModule, hasMultipleIRGenThreads,
948+
allSymbols);
926949
else
927950
error = validateTBD(Instance.getMainModule(), *IRModule,
928-
hasMultipleIRGenThreads);
951+
hasMultipleIRGenThreads, allSymbols);
929952
if (error)
930953
return true;
954+
955+
break;
956+
}
931957
}
932958

933959
std::unique_ptr<llvm::TargetMachine> TargetMachine =

lib/FrontendTool/TBD.cpp

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,24 @@ bool swift::writeTBD(ModuleDecl *M, bool hasMultipleIRGenThreads,
6060
return false;
6161
}
6262

63+
bool swift::inputFileKindCanHaveTBDValidated(InputFileKind kind) {
64+
// Only things that involve an AST can have a TBD file computed, at the
65+
// moment.
66+
switch (kind) {
67+
case InputFileKind::IFK_Swift:
68+
case InputFileKind::IFK_Swift_Library:
69+
return true;
70+
case InputFileKind::IFK_None:
71+
case InputFileKind::IFK_Swift_REPL:
72+
case InputFileKind::IFK_SIL:
73+
case InputFileKind::IFK_LLVM_IR:
74+
return false;
75+
}
76+
}
77+
6378
static bool validateSymbolSet(DiagnosticEngine &diags,
64-
llvm::StringSet<> symbols,
65-
llvm::Module &IRModule) {
79+
llvm::StringSet<> symbols, llvm::Module &IRModule,
80+
bool diagnoseExtraSymbolsInTBD) {
6681
auto error = false;
6782

6883
// Diff the two sets of symbols, flagging anything outside their intersection.
@@ -96,31 +111,37 @@ static bool validateSymbolSet(DiagnosticEngine &diags,
96111
error = true;
97112
}
98113

99-
for (auto &name : sortSymbols(symbols)) {
100-
diags.diagnose(SourceLoc(), diag::symbol_in_tbd_not_in_ir, name,
101-
Demangle::demangleSymbolAsString(name));
102-
error = true;
114+
if (diagnoseExtraSymbolsInTBD) {
115+
// Look for any extra symbols.
116+
for (auto &name : sortSymbols(symbols)) {
117+
diags.diagnose(SourceLoc(), diag::symbol_in_tbd_not_in_ir, name,
118+
Demangle::demangleSymbolAsString(name));
119+
error = true;
120+
}
103121
}
104122

105123
return error;
106124
}
107125

108126
bool swift::validateTBD(ModuleDecl *M, llvm::Module &IRModule,
109-
bool hasMultipleIRGenThreads) {
127+
bool hasMultipleIRGenThreads,
128+
bool diagnoseExtraSymbolsInTBD) {
110129
llvm::StringSet<> symbols;
111130
for (auto file : M->getFiles())
112131
enumeratePublicSymbols(file, symbols, hasMultipleIRGenThreads,
113132
/*isWholeModule=*/true);
114133

115-
return validateSymbolSet(M->getASTContext().Diags, symbols, IRModule);
134+
return validateSymbolSet(M->getASTContext().Diags, symbols, IRModule,
135+
diagnoseExtraSymbolsInTBD);
116136
}
117137

118138
bool swift::validateTBD(FileUnit *file, llvm::Module &IRModule,
119-
bool hasMultipleIRGenThreads) {
139+
bool hasMultipleIRGenThreads,
140+
bool diagnoseExtraSymbolsInTBD) {
120141
llvm::StringSet<> symbols;
121142
enumeratePublicSymbols(file, symbols, hasMultipleIRGenThreads,
122143
/*isWholeModule=*/false);
123144

124145
return validateSymbolSet(file->getParentModule()->getASTContext().Diags,
125-
symbols, IRModule);
146+
symbols, IRModule, diagnoseExtraSymbolsInTBD);
126147
}

lib/FrontendTool/TBD.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
#ifndef SWIFT_FRONTENDTOOL_TBD_H
1414
#define SWIFT_FRONTENDTOOL_TBD_H
1515

16+
#include "swift/Frontend/FrontendOptions.h"
17+
1618
namespace llvm {
1719
class StringRef;
1820
class Module;
@@ -24,10 +26,11 @@ class FrontendOptions;
2426

2527
bool writeTBD(ModuleDecl *M, bool hasMultipleIRGenThreads,
2628
llvm::StringRef OutputFilename);
29+
bool inputFileKindCanHaveTBDValidated(InputFileKind kind);
2730
bool validateTBD(ModuleDecl *M, llvm::Module &IRModule,
28-
bool hasMultipleIRGenThreads);
31+
bool hasMultipleIRGenThreads, bool diagnoseExtraSymbolsInTBD);
2932
bool validateTBD(FileUnit *M, llvm::Module &IRModule,
30-
bool hasMultipleIRGenThreads);
33+
bool hasMultipleIRGenThreads, bool diagnoseExtraSymbolsInTBD);
3134
}
3235

3336
#endif

test/TBD/class.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -validate-tbd-against-ir %s
1+
// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -validate-tbd-against-ir=all %s
22

33
open class OpenNothing {}
44

test/TBD/class_objc.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -import-objc-header %S/Inputs/objc_class_header.h -validate-tbd-against-ir %s
1+
// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -import-objc-header %S/Inputs/objc_class_header.h -validate-tbd-against-ir=all %s
22

33
// REQUIRES: objc_interop
44

test/TBD/extension.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// RUN: rm -rf %t && mkdir -p %t
22
// RUN: %target-build-swift %S/Inputs/extension_types.swift -module-name ExtensionTypes -emit-module -emit-module-path %t/ExtensionTypes.swiftmodule
3-
// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -validate-tbd-against-ir -I %t %s
3+
// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -validate-tbd-against-ir=all -I %t %s
44

55
import ExtensionTypes
66

test/TBD/function.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -validate-tbd-against-ir %s
1+
// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -validate-tbd-against-ir=all %s
22

33
public func publicNoArgs() {}
44
public func publicSomeArgs(_: Int, x: Int) {}

test/TBD/global.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -validate-tbd-against-ir %s
1+
// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -validate-tbd-against-ir=all %s
22

33
public let publicLet: Int = 0
44
internal let internalLet: Int = 0

test/TBD/main.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
// RUN: %target-swift-frontend -emit-ir -o- -module-name test -validate-tbd-against-ir %s
1+
// RUN: %target-swift-frontend -emit-ir -o- -module-name test -validate-tbd-against-ir=all %s
22

33
// Top-level code (i.e. implicit `main`) should be handled

test/TBD/protocol.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -validate-tbd-against-ir %s
1+
// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -validate-tbd-against-ir=all %s
22

33
public protocol Public {
44
func publicMethod()

test/TBD/struct.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -validate-tbd-against-ir %s
1+
// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -validate-tbd-against-ir=all %s
22

33
public struct PublicNothing {}
44

0 commit comments

Comments
 (0)