Skip to content

[Frontend] Load standard libarary in CompilerInstance::setup #40107

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion include/swift/Frontend/Frontend.h
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,9 @@ class CompilerInvocation {
if (FrontendOpts.InputMode == FrontendOptions::ParseInputMode::SIL) {
return ImplicitStdlibKind::None;
}
if (FrontendOpts.InputsAndOutputs.shouldTreatAsLLVM()) {
return ImplicitStdlibKind::None;
}
if (getParseStdlib()) {
return ImplicitStdlibKind::Builtin;
}
Expand Down Expand Up @@ -562,7 +565,7 @@ class CompilerInstance {
}

/// Returns true if there was an error during setup.
bool setup(const CompilerInvocation &Invocation);
bool setup(const CompilerInvocation &Invocation, std::string &Error);

const CompilerInvocation &getInvocation() const { return Invocation; }

Expand Down
3 changes: 3 additions & 0 deletions include/swift/Frontend/ModuleInterfaceLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,9 @@ struct ModuleInterfaceLoaderOptions {
case FrontendOptions::ActionType::TypecheckModuleFromInterface:
requestedAction = FrontendOptions::ActionType::Typecheck;
break;
case FrontendOptions::ActionType::ScanDependencies:
requestedAction = Opts.RequestedAction;
break;
Comment on lines +316 to +318
Copy link
Contributor

@hamishknight hamishknight Nov 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nkcsgexi @artemcm Does this look good to you?

Would you mind also double checking that there aren't other callers of CompilerInstance::setup where we may need to change the requested action to avoid loading the stdlib? In particular I'm not too sure what frontend arguments are being fed for the other dependency scanning call-sites, are they guaranteed to use the dependency scanning action?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine to me.
I also fully expect all dependency scanning call-sites to also have FrontendOptions::ActionType::ScanDependencies.

default:
requestedAction = FrontendOptions::ActionType::EmitModuleOnly;
break;
Expand Down
4 changes: 3 additions & 1 deletion include/swift/IDE/Utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "swift/AST/Effects.h"
#include "swift/AST/Module.h"
#include "swift/AST/ASTPrinter.h"
#include "swift/Frontend/FrontendOptions.h"
#include "swift/IDE/SourceEntityWalker.h"
#include "swift/Parse/Token.h"
#include "llvm/ADT/StringRef.h"
Expand Down Expand Up @@ -85,7 +86,8 @@ SourceCompleteResult isSourceInputComplete(StringRef Text, SourceFileKind SFKind

bool initCompilerInvocation(
CompilerInvocation &Invocation, ArrayRef<const char *> OrigArgs,
DiagnosticEngine &Diags, StringRef UnresolvedPrimaryFile,
FrontendOptions::ActionType Action, DiagnosticEngine &Diags,
StringRef UnresolvedPrimaryFile,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FileSystem,
const std::string &runtimeResourcePath,
const std::string &diagnosticDocumentationPath, time_t sessionTimestamp,
Expand Down
5 changes: 3 additions & 2 deletions lib/APIDigester/ModuleAnalyzerNodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2247,8 +2247,9 @@ swift::ide::api::getSDKNodeRoot(SDKContext &SDKCtx,
// The PrintDiags is only responsible compiler errors, we should remove the
// consumer immediately after importing is done.
SWIFT_DEFER { CI.getDiags().removeConsumer(PrintDiags); };
if (CI.setup(Invocation)) {
llvm::errs() << "Failed to setup the compiler instance\n";
std::string InstanceSetupError;
if (CI.setup(Invocation, InstanceSetupError)) {
llvm::errs() << InstanceSetupError << '\n';
return nullptr;
}

Expand Down
5 changes: 4 additions & 1 deletion lib/DependencyScan/DependencyScanningTool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,10 @@ DependencyScanningTool::initCompilerInstanceForScan(
}

// Setup the instance
Instance->setup(Invocation);
std::string InstanceSetupError;
if (Instance->setup(Invocation, InstanceSetupError)) {
return std::make_error_code(std::errc::not_supported);
}
(void)Instance->getMainModule();

return Instance;
Expand Down
3 changes: 2 additions & 1 deletion lib/DependencyScan/ScanDependencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1093,7 +1093,8 @@ forEachBatchEntry(CompilerInstance &invocationInstance,
SourceLoc(), diag::scanner_arguments_invalid, entry.arguments);
return true;
}
if (pInstance->setup(subInvok)) {
std::string InstanceSetupError;
if (pInstance->setup(subInvok, InstanceSetupError)) {
invocationInstance.getDiags().diagnose(
SourceLoc(), diag::scanner_arguments_invalid, entry.arguments);
return true;
Expand Down
4 changes: 3 additions & 1 deletion lib/DriverTool/swift_api_digester_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1849,8 +1849,10 @@ static bool readBreakageAllowlist(SDKContext &Ctx, llvm::StringSet<> &lines,
CompilerInstance instance;
CompilerInvocation invok;
invok.setModuleName("ForClangImporter");
if (instance.setup(invok))
std::string InstanceSetupError;
if (instance.setup(invok, InstanceSetupError)) {
return 1;
}
auto importer = ClangImporter::create(instance.getASTContext());
SmallString<128> preprocessedFilePath;
if (auto error = llvm::sys::fs::createTemporaryFile(
Expand Down
5 changes: 3 additions & 2 deletions lib/DriverTool/swift_api_extract_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,9 @@ class SwiftAPIExtractInvocation {
}

int extractAPI() {
if (Instance.setup(Invocation)) {
llvm::outs() << "Failed to setup compiler instance\n";
std::string InstanceSetupError;
if (Instance.setup(Invocation, InstanceSetupError)) {
llvm::outs() << InstanceSetupError << '\n';
return 1;
}

Expand Down
5 changes: 3 additions & 2 deletions lib/DriverTool/swift_symbolgraph_extract_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,9 @@ int swift_symbolgraph_extract_main(ArrayRef<const char *> Args,
.Default(AccessLevel::Public);
}

if (CI.setup(Invocation)) {
llvm::outs() << "Failed to setup compiler instance\n";
std::string InstanceSetupError;
if (CI.setup(Invocation, InstanceSetupError)) {
llvm::outs() << InstanceSetupError << '\n';
return EXIT_FAILURE;
}

Expand Down
33 changes: 28 additions & 5 deletions lib/Frontend/Frontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -357,30 +357,49 @@ void CompilerInstance::setupDependencyTrackerIfNeeded() {
DepTracker = std::make_unique<DependencyTracker>(*collectionMode);
}

bool CompilerInstance::setup(const CompilerInvocation &Invok) {
bool CompilerInstance::setup(const CompilerInvocation &Invok,
std::string &Error) {
Invocation = Invok;

setupDependencyTrackerIfNeeded();

// If initializing the overlay file system fails there's no sense in
// continuing because the compiler will read the wrong files.
if (setUpVirtualFileSystemOverlays())
if (setUpVirtualFileSystemOverlays()) {
Error = "Setting up virtual file system overlays failed";
return true;
}
setUpLLVMArguments();
setUpDiagnosticOptions();

assert(Lexer::isIdentifier(Invocation.getModuleName()));

if (setUpInputs())
if (setUpInputs()) {
Error = "Setting up inputs failed";
return true;
}

if (setUpASTContextIfNeeded())
if (setUpASTContextIfNeeded()) {
Error = "Setting up ASTContext failed";
return true;
}

setupStatsReporter();

if (setupDiagnosticVerifierIfNeeded())
if (setupDiagnosticVerifierIfNeeded()) {
Error = "Setting up diagnostics verified failed";
return true;
}

// If we expect an implicit stdlib import, load in the standard library. If we
// either fail to find it or encounter an error while loading it, bail early. Continuing will at best
// trigger a bunch of other errors due to the stdlib being missing, or at
// worst crash downstream as many call sites don't currently handle a missing
// stdlib.
if (loadStdlibIfNeeded()) {
Error = "Loading the standard library failed";
return true;
}

return false;
}
Expand Down Expand Up @@ -1101,6 +1120,10 @@ void CompilerInstance::performSema() {
}

bool CompilerInstance::loadStdlibIfNeeded() {
if (!FrontendOptions::doesActionRequireSwiftStandardLibrary(
Invocation.getFrontendOptions().RequestedAction)) {
return false;
}
// If we aren't expecting an implicit stdlib import, there's nothing to do.
if (getImplicitImportInfo().StdlibKind != ImplicitStdlibKind::Stdlib)
return false;
Expand Down
7 changes: 0 additions & 7 deletions lib/Frontend/ModuleInterfaceBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,13 +218,6 @@ bool ModuleInterfaceBuilder::buildSwiftModuleInternal(
InputInfo.getPrimarySpecificPaths().SupplementaryOutputs;
StringRef OutPath = OutputInfo.ModuleOutputPath;

// Bail out if we're going to use the standard library but can't load it. If
// we don't do this before we try to build the interface, we could end up
// trying to rebuild a broken standard library dozens of times due to
// multiple calls to `ASTContext::getStdlibModule()`.
if (SubInstance.loadStdlibIfNeeded())
return std::make_error_code(std::errc::not_supported);

// Build the .swiftmodule; this is a _very_ abridged version of the logic
// in performCompile in libFrontendTool, specialized, to just the one
// module-serialization task we're trying to do here.
Expand Down
3 changes: 2 additions & 1 deletion lib/Frontend/ModuleInterfaceLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1682,7 +1682,8 @@ InterfaceSubContextDelegateImpl::runInSubCompilerInstance(StringRef moduleName,

ForwardingDiagnosticConsumer FDC(*Diags);
subInstance.addDiagnosticConsumer(&FDC);
if (subInstance.setup(subInvocation)) {
std::string InstanceSetupError;
if (subInstance.setup(subInvocation, InstanceSetupError)) {
return std::make_error_code(std::errc::not_supported);
}

Expand Down
14 changes: 2 additions & 12 deletions lib/FrontendTool/FrontendTool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1254,17 +1254,6 @@ static bool performCompile(CompilerInstance &Instance,
if (opts.InputsAndOutputs.shouldTreatAsLLVM())
return compileLLVMIR(Instance);

// If we aren't in a parse-only context and expect an implicit stdlib import,
// load in the standard library. If we either fail to find it or encounter an
// error while loading it, bail early. Continuing the compilation will at best
// trigger a bunch of other errors due to the stdlib being missing, or at
// worst crash downstream as many call sites don't currently handle a missing
// stdlib.
if (FrontendOptions::doesActionRequireSwiftStandardLibrary(Action)) {
if (Instance.loadStdlibIfNeeded())
return true;
}

assert([&]() -> bool {
if (FrontendOptions::shouldActionOnlyParse(Action)) {
// Parsing gets triggered lazily, but let's make sure we have the right
Expand Down Expand Up @@ -2044,7 +2033,8 @@ int swift::performFrontend(ArrayRef<const char *> Args,
const DiagnosticOptions &diagOpts = Invocation.getDiagnosticOptions();
bool verifierEnabled = diagOpts.VerifyMode != DiagnosticOptions::NoVerify;

if (Instance->setup(Invocation)) {
std::string InstanceSetupError;
if (Instance->setup(Invocation, InstanceSetupError)) {
return finishDiagProcessing(1, /*verifierEnabled*/ false);
}

Expand Down
14 changes: 3 additions & 11 deletions lib/IDE/CompletionInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -561,23 +561,15 @@ void CompletionInstance::performNewOperation(

Invocation.setCodeCompletionPoint(completionBuffer, Offset);

if (CI.setup(Invocation)) {
std::string InstanceSetupError;
if (CI.setup(Invocation, InstanceSetupError)) {
Callback(CancellableResult<CompletionInstanceResult>::failure(
"failed to setup compiler instance"));
InstanceSetupError));
return;
}
CI.getASTContext().CancellationFlag = CancellationFlag;
registerIDERequestFunctions(CI.getASTContext().evaluator);

// If we're expecting a standard library, but there either isn't one, or it
// failed to load, let's bail early and hand back an empty completion
// result to avoid any downstream crashes.
if (CI.loadStdlibIfNeeded()) {
Callback(CancellableResult<CompletionInstanceResult>::failure(
"failed to load the standard library"));
return;
}

CI.performParseAndResolveImportsOnly();

bool DidFindCodeCompletionToken = CI.getCodeCompletionFile()
Expand Down
5 changes: 3 additions & 2 deletions lib/IDE/Refactoring.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1200,8 +1200,9 @@ getNotableRegions(StringRef SourceText, unsigned NameOffset, StringRef Name,
Invocation.getLangOptions().DisablePoundIfEvaluation = true;

auto Instance = std::make_unique<swift::CompilerInstance>();
if (Instance->setup(Invocation))
llvm_unreachable("Failed setup");
std::string InstanceSetupError;
if (Instance->setup(Invocation, InstanceSetupError))
llvm_unreachable(InstanceSetupError.c_str());

unsigned BufferId = Instance->getPrimarySourceFile()->getBufferID().getValue();
SourceManager &SM = Instance->getSourceMgr();
Expand Down
3 changes: 2 additions & 1 deletion lib/IDE/Utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,8 @@ class StreamDiagConsumer : public DiagnosticConsumer {

bool ide::initCompilerInvocation(
CompilerInvocation &Invocation, ArrayRef<const char *> OrigArgs,
DiagnosticEngine &Diags, StringRef UnresolvedPrimaryFile,
FrontendOptions::ActionType Action, DiagnosticEngine &Diags,
StringRef UnresolvedPrimaryFile,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FileSystem,
const std::string &runtimeResourcePath,
const std::string &diagnosticDocumentationPath, time_t sessionTimestamp,
Expand Down
3 changes: 2 additions & 1 deletion lib/Migrator/Migrator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ Migrator::performAFixItMigration(version::Version SwiftLanguageVersion) {
// rdar://78576743 - Reset LLVM global state for command-line arguments set
// by prior calls to setup.
llvm::cl::ResetAllOptionOccurrences();
if (Instance->setup(Invocation)) {
std::string InstanceSetupError;
if (Instance->setup(Invocation, InstanceSetupError)) {
return nullptr;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ class Str {
// RUN: -req=complete -pos=4:1 %s -- %s -resource-dir %t/rsrc -sdk %t/sdk == \
// RUN: -req=complete -pos=4:1 %s -- %s -resource-dir %t/rsrc -sdk %t/sdk 2>&1 | %FileCheck %s

// CHECK: error response (Request Failed): failed to load the standard library
// CHECK: error response (Request Failed): Loading the standard library failed
Loading