Skip to content

Commit 6674e1b

Browse files
cachemeifyoucancachemeifyoucan
committed
Address review feedback
1 parent 54be86b commit 6674e1b

25 files changed

+117
-147
lines changed

‎include/swift/AST/ASTContext.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ class ASTContext final {
285285
DiagnosticEngine &Diags;
286286

287287
/// OutputBackend for writing outputs.
288-
llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> Backend;
288+
llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> OutputBackend;
289289

290290
/// If the shared pointer is not a \c nullptr and the pointee is \c true,
291291
/// all operations working on this ASTContext should be aborted at the next
@@ -1525,13 +1525,13 @@ class ASTContext final {
15251525
/// Get the output backend. The output backend needs to be initialized via
15261526
/// constructor or `setOutputBackend`.
15271527
llvm::vfs::OutputBackend &getOutputBackend() const {
1528-
assert(Backend && "OutputBackend is not setup");
1529-
return *Backend;
1528+
assert(OutputBackend && "OutputBackend is not setup");
1529+
return *OutputBackend;
15301530
}
15311531
/// Set output backend for virtualized outputs.
15321532
void setOutputBackend(
1533-
llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> OutputBackend) {
1534-
Backend = std::move(OutputBackend);
1533+
llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> OutBackend) {
1534+
OutputBackend = std::move(OutBackend);
15351535
}
15361536

15371537
private:

‎include/swift/AST/DiagnosticsCommon.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ ERROR(not_implemented,none,
2929
ERROR(error_opening_output,none,
3030
"error opening '%0' for output: %1", (StringRef, StringRef))
3131

32+
ERROR(error_closing_output,none,
33+
"error closing '%0' for output: %1", (StringRef, StringRef))
34+
3235
ERROR(cannot_find_group_info_file,none,
3336
"cannot find group info file at path: '%0'", (StringRef))
3437

‎include/swift/AST/FileSystem.h

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ namespace swift {
2626
/// \returns true if there were any errors, either from the filesystem
2727
/// operations or from \p action returning true.
2828
inline bool
29-
withOutputFile(DiagnosticEngine &diags, llvm::vfs::OutputBackend &Backend,
29+
withOutputPath(DiagnosticEngine &diags, llvm::vfs::OutputBackend &Backend,
3030
StringRef outputPath,
3131
llvm::function_ref<bool(llvm::raw_pwrite_stream &)> action) {
3232
assert(!outputPath.empty());
@@ -37,15 +37,19 @@ withOutputFile(DiagnosticEngine &diags, llvm::vfs::OutputBackend &Backend,
3737
if (!outputFile) {
3838
diags.diagnose(SourceLoc(), diag::error_opening_output, outputPath,
3939
toString(outputFile.takeError()));
40-
return false;
40+
return true;
4141
}
4242

4343
bool failed = action(*outputFile);
4444
// If there is an error, discard output. Otherwise keep the output file.
4545
if (auto error = failed ? outputFile->discard() : outputFile->keep()) {
46-
diags.diagnose(SourceLoc(), diag::error_opening_output, outputPath,
47-
toString(std::move(error)));
48-
return false;
46+
// Don't diagnose discard error.
47+
if (failed)
48+
consumeError(std::move(error));
49+
else
50+
diags.diagnose(SourceLoc(), diag::error_closing_output, outputPath,
51+
toString(std::move(error)));
52+
return true;
4953
}
5054
return failed;
5155
}

‎include/swift/AST/IRGenOptions.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ class IRGenOptions {
318318
/// Print the LLVM inline tree at the end of the LLVM pass pipeline.
319319
unsigned PrintInlineTree : 1;
320320

321-
/// Always recompile the output even the module hash might match.
321+
/// Always recompile the output even if the module hash might match.
322322
unsigned AlwaysCompile : 1;
323323

324324
/// Whether we should embed the bitcode file.

‎include/swift/ClangImporter/ClangImporter.h

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -395,9 +395,7 @@ class ClangImporter final : public ClangModuleLoader {
395395
/// replica.
396396
///
397397
/// \sa clang::GeneratePCHAction
398-
bool
399-
emitBridgingPCH(llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> backend,
400-
StringRef headerPath, StringRef outputPCHPath);
398+
bool emitBridgingPCH(StringRef headerPath, StringRef outputPCHPath);
401399

402400
/// Returns true if a clang CompilerInstance can successfully read in a PCH,
403401
/// assuming it exists, with the current options. This can be used to find out
@@ -408,21 +406,16 @@ class ClangImporter final : public ClangModuleLoader {
408406
/// module map into the replica and emits a PCM file for one of the modules it
409407
/// declares. Delegates to clang for everything except construction of the
410408
/// replica.
411-
bool emitPrecompiledModule(
412-
llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> backend,
413-
StringRef moduleMapPath, StringRef moduleName, StringRef outputPath);
409+
bool emitPrecompiledModule(StringRef moduleMapPath, StringRef moduleName,
410+
StringRef outputPath);
414411

415412
/// Makes a temporary replica of the ClangImporter's CompilerInstance and
416413
/// dumps information about a PCM file (assumed to be generated by -emit-pcm
417414
/// or in the Swift module cache). Delegates to clang for everything except
418415
/// construction of the replica.
419-
bool dumpPrecompiledModule(
420-
llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> backend,
421-
StringRef modulePath, StringRef outputPath);
416+
bool dumpPrecompiledModule(StringRef modulePath, StringRef outputPath);
422417

423-
bool
424-
runPreprocessor(llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> backend,
425-
StringRef inputPath, StringRef outputPath);
418+
bool runPreprocessor(StringRef inputPath, StringRef outputPath);
426419
const clang::Module *getClangOwningModule(ClangNode Node) const;
427420
bool hasTypedef(const clang::Decl *typeDecl) const;
428421

@@ -510,8 +503,7 @@ class ClangImporter final : public ClangModuleLoader {
510503

511504
Optional<std::string>
512505
getOrCreatePCH(const ClangImporterOptions &ImporterOptions,
513-
StringRef SwiftPCHHash,
514-
llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> Backend);
506+
StringRef SwiftPCHHash);
515507
Optional<std::string>
516508
/// \param isExplicit true if the PCH filename was passed directly
517509
/// with -import-objc-header option.

‎include/swift/Frontend/Frontend.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ class CompilerInstance {
461461
std::unique_ptr<UnifiedStatsReporter> Stats;
462462

463463
/// Virtual OutputBackend.
464-
llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> TheOutputBackend = nullptr;
464+
llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> OutputBackend = nullptr;
465465

466466
/// The verification output backend.
467467
using HashBackendTy = llvm::vfs::HashingOutputBackend<llvm::BLAKE3>;
@@ -518,11 +518,11 @@ class CompilerInstance {
518518
}
519519

520520
llvm::vfs::OutputBackend &getOutputBackend() const {
521-
return *TheOutputBackend;
521+
return *OutputBackend;
522522
}
523523
void
524524
setOutputBackend(llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> Backend) {
525-
TheOutputBackend = std::move(Backend);
525+
OutputBackend = std::move(Backend);
526526
}
527527
using HashingBackendPtrTy = llvm::IntrusiveRefCntPtr<HashBackendTy>;
528528
HashingBackendPtrTy getHashingBackend() { return HashBackend; }

‎lib/AST/ASTContext.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -661,12 +661,12 @@ ASTContext::ASTContext(
661661
ClangImporterOptions &ClangImporterOpts,
662662
symbolgraphgen::SymbolGraphOptions &SymbolGraphOpts,
663663
SourceManager &SourceMgr, DiagnosticEngine &Diags,
664-
llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> OutputBackend,
664+
llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> OutBackend,
665665
std::function<bool(llvm::StringRef, bool)> PreModuleImportCallback)
666666
: LangOpts(langOpts), TypeCheckerOpts(typecheckOpts), SILOpts(silOpts),
667667
SearchPathOpts(SearchPathOpts), ClangImporterOpts(ClangImporterOpts),
668668
SymbolGraphOpts(SymbolGraphOpts), SourceMgr(SourceMgr), Diags(Diags),
669-
Backend(std::move(OutputBackend)), evaluator(Diags, langOpts),
669+
OutputBackend(std::move(OutBackend)), evaluator(Diags, langOpts),
670670
TheBuiltinModule(createBuiltinModule(*this)),
671671
StdlibModuleName(getIdentifier(STDLIB_NAME)),
672672
SwiftShimsModuleName(getIdentifier(SWIFT_SHIMS_NAME)),
@@ -714,8 +714,8 @@ ASTContext::ASTContext(
714714

715715
createModuleToExecutablePluginMap();
716716
// Provide a default OnDiskOutputBackend if user didn't supply one.
717-
if (!Backend)
718-
Backend = llvm::makeIntrusiveRefCnt<llvm::vfs::OnDiskOutputBackend>();
717+
if (!OutputBackend)
718+
OutputBackend = llvm::makeIntrusiveRefCnt<llvm::vfs::OnDiskOutputBackend>();
719719
}
720720

721721
ASTContext::~ASTContext() {

‎lib/AST/FineGrainedDependencies.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ void SourceFileDepGraph::emitDotFile(llvm::vfs::OutputBackend &outputBackend,
377377
StringRef outputPath,
378378
DiagnosticEngine &diags) {
379379
std::string dotFileName = outputPath.str() + ".dot";
380-
withOutputFile(
380+
withOutputPath(
381381
diags, outputBackend, dotFileName, [&](llvm::raw_pwrite_stream &out) {
382382
DotFileEmitter<SourceFileDepGraph>(out, *this, false, false).emit();
383383
return false;

‎lib/AST/FineGrainedDependencyFormat.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ bool swift::fine_grained_dependencies::writeFineGrainedDependencyGraphToPath(
502502
DiagnosticEngine &diags, llvm::vfs::OutputBackend &backend, StringRef path,
503503
const SourceFileDepGraph &g) {
504504
PrettyStackTraceStringAction stackTrace("saving fine-grained dependency graph", path);
505-
return withOutputFile(diags, backend, path, [&](llvm::raw_ostream &out) {
505+
return withOutputPath(diags, backend, path, [&](llvm::raw_ostream &out) {
506506
SmallVector<char, 0> Buffer;
507507
llvm::BitstreamWriter Writer{Buffer};
508508
writeFineGrainedDependencyGraph(Writer, g, Purpose::ForSwiftDeps);

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,7 @@ namespace {
194194
Importer.addSearchPath(path, /*isFramework*/false, /*isSystem=*/false);
195195
}
196196

197-
auto PCH = Importer.getOrCreatePCH(ImporterOpts, SwiftPCHHash,
198-
Ctx.getOutputBackend().clone());
197+
auto PCH = Importer.getOrCreatePCH(ImporterOpts, SwiftPCHHash);
199198
if (PCH.has_value()) {
200199
Impl.getClangInstance()->getPreprocessorOpts().ImplicitPCHInclude =
201200
PCH.value();
@@ -944,8 +943,7 @@ ClangImporter::getPCHFilename(const ClangImporterOptions &ImporterOptions,
944943
}
945944

946945
Optional<std::string> ClangImporter::getOrCreatePCH(
947-
const ClangImporterOptions &ImporterOptions, StringRef SwiftPCHHash,
948-
llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> Backend) {
946+
const ClangImporterOptions &ImporterOptions, StringRef SwiftPCHHash) {
949947
bool isExplicit;
950948
auto PCHFilename = getPCHFilename(ImporterOptions, SwiftPCHHash,
951949
isExplicit);
@@ -962,8 +960,7 @@ Optional<std::string> ClangImporter::getOrCreatePCH(
962960
return None;
963961
}
964962
auto FailedToEmit =
965-
emitBridgingPCH(std::move(Backend), ImporterOptions.BridgingHeader,
966-
PCHFilename.value());
963+
emitBridgingPCH(ImporterOptions.BridgingHeader, PCHFilename.value());
967964
if (FailedToEmit) {
968965
return None;
969966
}
@@ -1696,12 +1693,12 @@ ClangImporter::cloneCompilerInstanceForPrecompiling() {
16961693
clonedInstance->setFileManager(&fileManager);
16971694
clonedInstance->createSourceManager(fileManager);
16981695
clonedInstance->setTarget(&Impl.Instance->getTarget());
1696+
clonedInstance->setOutputBackend(Impl.SwiftContext.OutputBackend);
16991697

17001698
return clonedInstance;
17011699
}
17021700

17031701
bool ClangImporter::emitBridgingPCH(
1704-
llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> backend,
17051702
StringRef headerPath, StringRef outputPCHPath) {
17061703
auto emitInstance = cloneCompilerInstanceForPrecompiling();
17071704
auto &invocation = emitInstance->getInvocation();
@@ -1718,8 +1715,6 @@ bool ClangImporter::emitBridgingPCH(
17181715
FrontendOpts.OutputFile = outputPCHPath.str();
17191716
FrontendOpts.ProgramAction = clang::frontend::GeneratePCH;
17201717

1721-
emitInstance->setOutputBackend(std::move(backend));
1722-
17231718
auto action = wrapActionForIndexingIfEnabled(
17241719
FrontendOpts, std::make_unique<clang::GeneratePCHAction>());
17251720
emitInstance->ExecuteAction(*action);
@@ -1734,7 +1729,6 @@ bool ClangImporter::emitBridgingPCH(
17341729
}
17351730

17361731
bool ClangImporter::runPreprocessor(
1737-
llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> backend,
17381732
StringRef inputPath, StringRef outputPath) {
17391733
auto emitInstance = cloneCompilerInstanceForPrecompiling();
17401734
auto &invocation = emitInstance->getInvocation();
@@ -1753,16 +1747,13 @@ bool ClangImporter::runPreprocessor(
17531747
FrontendOpts.OutputFile = outputPath.str();
17541748
FrontendOpts.ProgramAction = clang::frontend::PrintPreprocessedInput;
17551749

1756-
emitInstance->setOutputBackend(std::move(backend));
1757-
17581750
auto action = wrapActionForIndexingIfEnabled(
17591751
FrontendOpts, std::make_unique<clang::PrintPreprocessedAction>());
17601752
emitInstance->ExecuteAction(*action);
17611753
return emitInstance->getDiagnostics().hasErrorOccurred();
17621754
}
17631755

17641756
bool ClangImporter::emitPrecompiledModule(
1765-
llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> backend,
17661757
StringRef moduleMapPath, StringRef moduleName, StringRef outputPath) {
17671758
auto emitInstance = cloneCompilerInstanceForPrecompiling();
17681759
auto &invocation = emitInstance->getInvocation();
@@ -1784,8 +1775,6 @@ bool ClangImporter::emitPrecompiledModule(
17841775
FrontendOpts.OutputFile = outputPath.str();
17851776
FrontendOpts.ProgramAction = clang::frontend::GenerateModule;
17861777

1787-
emitInstance->setOutputBackend(std::move(backend));
1788-
17891778
auto action = wrapActionForIndexingIfEnabled(
17901779
FrontendOpts,
17911780
std::make_unique<clang::GenerateModuleFromModuleMapAction>());
@@ -1800,7 +1789,6 @@ bool ClangImporter::emitPrecompiledModule(
18001789
}
18011790

18021791
bool ClangImporter::dumpPrecompiledModule(
1803-
llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> backend,
18041792
StringRef modulePath, StringRef outputPath) {
18051793
auto dumpInstance = cloneCompilerInstanceForPrecompiling();
18061794
auto &invocation = dumpInstance->getInvocation();
@@ -1813,8 +1801,6 @@ bool ClangImporter::dumpPrecompiledModule(
18131801
FrontendOpts.Inputs = {inputFile};
18141802
FrontendOpts.OutputFile = outputPath.str();
18151803

1816-
dumpInstance->setOutputBackend(std::move(backend));
1817-
18181804
auto action = std::make_unique<clang::DumpModuleInfoAction>();
18191805
dumpInstance->ExecuteAction(*action);
18201806

‎lib/DependencyScan/ModuleDependencyCacheSerialization.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1151,7 +1151,7 @@ bool swift::dependencies::module_dependency_cache_serialization::
11511151
StringRef path, const SwiftDependencyScanningService &cache) {
11521152
PrettyStackTraceStringAction stackTrace(
11531153
"saving inter-module dependency graph", path);
1154-
return withOutputFile(diags, backend, path, [&](llvm::raw_ostream &out) {
1154+
return withOutputPath(diags, backend, path, [&](llvm::raw_ostream &out) {
11551155
SmallVector<char, 0> Buffer;
11561156
llvm::BitstreamWriter Writer{Buffer};
11571157
writeInterModuleDependenciesCache(Writer, cache);

‎lib/DependencyScan/ScanDependencies.cpp

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
#include "llvm/Support/CommandLine.h"
4545
#include "llvm/Support/FileSystem.h"
4646
#include "llvm/Support/StringSaver.h"
47+
#include "llvm/Support/VirtualOutputBackend.h"
4748
#include "llvm/Support/YAMLParser.h"
4849
#include "llvm/Support/YAMLTraits.h"
4950
#include "llvm/Support/raw_ostream.h"
@@ -935,6 +936,25 @@ static void writeJSON(llvm::raw_ostream &out,
935936
}
936937
}
937938

939+
static bool writePrescanJSONToOutput(DiagnosticEngine &diags,
940+
llvm::vfs::OutputBackend &backend,
941+
StringRef path,
942+
const swiftscan_import_set_t importSet) {
943+
return withOutputPath(diags, backend, path, [&](llvm::raw_pwrite_stream &os) {
944+
writePrescanJSON(os, importSet);
945+
return false;
946+
});
947+
}
948+
949+
static bool writeJSONToOutput(DiagnosticEngine &diags,
950+
llvm::vfs::OutputBackend &backend, StringRef path,
951+
const swiftscan_dependency_graph_t dependencies) {
952+
return withOutputPath(diags, backend, path, [&](llvm::raw_pwrite_stream &os) {
953+
writeJSON(os, dependencies);
954+
return false;
955+
});
956+
}
957+
938958
static swiftscan_dependency_graph_t
939959
generateFullDependencyGraph(CompilerInstance &instance,
940960
ModuleDependenciesCache &cache,
@@ -1420,11 +1440,8 @@ bool swift::dependencies::scanDependencies(CompilerInstance &instance) {
14201440
return true;
14211441
auto dependencies = std::move(*dependenciesOrErr);
14221442

1423-
if (withOutputFile(Context.Diags, instance.getOutputBackend(), path,
1424-
[&](llvm::raw_pwrite_stream &os) {
1425-
writeJSON(os, dependencies);
1426-
return false;
1427-
}))
1443+
if (writeJSONToOutput(Context.Diags, instance.getOutputBackend(), path,
1444+
dependencies))
14281445
return true;
14291446

14301447
// This process succeeds regardless of whether any errors occurred.
@@ -1453,11 +1470,8 @@ bool swift::dependencies::prescanDependencies(CompilerInstance &instance) {
14531470
auto importSet = std::move(*importSetOrErr);
14541471

14551472
// Serialize and output main module dependencies only and exit.
1456-
if (withOutputFile(Context.Diags, instance.getOutputBackend(), path,
1457-
[&](llvm::raw_pwrite_stream &os) {
1458-
writePrescanJSON(os, importSet);
1459-
return false;
1460-
}))
1473+
if (writePrescanJSONToOutput(Context.Diags, instance.getOutputBackend(), path,
1474+
importSet))
14611475
return true;
14621476

14631477
// This process succeeds regardless of whether any errors occurred.
@@ -1498,12 +1512,9 @@ bool swift::dependencies::batchScanDependencies(
14981512
if ((*iresults).getError())
14991513
return true;
15001514

1501-
if (withOutputFile(instance.getASTContext().Diags,
1502-
instance.getOutputBackend(), (*ientries).outputPath,
1503-
[&](llvm::raw_pwrite_stream &os) {
1504-
writeJSON(os, **iresults);
1505-
return false;
1506-
}))
1515+
if (writeJSONToOutput(instance.getASTContext().Diags,
1516+
instance.getOutputBackend(), (*ientries).outputPath,
1517+
**iresults))
15071518
return true;
15081519
}
15091520
return false;
@@ -1537,12 +1548,9 @@ bool swift::dependencies::batchPrescanDependencies(
15371548
if ((*iresults).getError())
15381549
return true;
15391550

1540-
if (withOutputFile(instance.getASTContext().Diags,
1541-
instance.getOutputBackend(), (*ientries).outputPath,
1542-
[&](llvm::raw_pwrite_stream &os) {
1543-
writePrescanJSON(os, **iresults);
1544-
return false;
1545-
}))
1551+
if (writePrescanJSONToOutput(instance.getASTContext().Diags,
1552+
instance.getOutputBackend(),
1553+
(*ientries).outputPath, **iresults))
15461554
return true;
15471555
}
15481556
return false;

0 commit comments

Comments
 (0)