Skip to content

Introduce Import Resolution as a Frontend Action #17645

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.

Sign up for GitHub

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 3 commits into from
Jul 13, 2018
Merged

Introduce Import Resolution as a Frontend Action #17645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c440b0b
Diagnose parse-only invocations trying to emit dependency information
CodaFi Jun 29, 2018
cb1e9dd
Introduce Name Binding as a Frontend Action
CodaFi Jun 29, 2018
2282065
Rename "Name Binding" action to "Resolve Imports"
CodaFi Jul 13, 2018
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
12 changes: 7 additions & 5 deletions include/swift/AST/DiagnosticsFrontend.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,17 @@ NOTE(note_valid_swift_versions, none,
"valid arguments to '-swift-version' are %0", (StringRef))

ERROR(error_mode_cannot_emit_dependencies,none,
"this mode does not support emitting dependency files", ())
"this mode does not support emitting dependency files", ())
ERROR(error_mode_cannot_emit_reference_dependencies,none,
"this mode does not support emitting reference dependency files", ())
ERROR(error_mode_cannot_emit_header,none,
"this mode does not support emitting Objective-C headers", ())
"this mode does not support emitting Objective-C headers", ())
ERROR(error_mode_cannot_emit_loaded_module_trace,none,
"this mode does not support emitting the loaded module trace", ())
"this mode does not support emitting the loaded module trace", ())
ERROR(error_mode_cannot_emit_module,none,
"this mode does not support emitting modules", ())
"this mode does not support emitting modules", ())
ERROR(error_mode_cannot_emit_module_doc,none,
"this mode does not support emitting module documentation files", ())
"this mode does not support emitting module documentation files", ())
Copy link
Contributor

Choose a reason for hiding this comment

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

If you feel like it, it would be nice to actually include the referred-to mode in the error message. I understand if it's outside the scope of this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Most of these errors are already caught at the driver level, so checking for them in the frontend just helps test authors. That means there's not really a problem with talking about specific frontend arguments, which I'd normally be worried about.


WARNING(emit_reference_dependencies_without_primary_file,none,
"ignoring -emit-reference-dependencies (requires -primary-file)", ())
Expand Down
19 changes: 13 additions & 6 deletions include/swift/Frontend/Frontend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,10 +539,14 @@ class CompilerInstance {

/// Parses the input file but does no type-checking or module imports.
/// Note that this only supports parsing an invocation with a single file.
///
///
void performParseOnly(bool EvaluateConditionals = false);

/// Parses and performs name binding on all input files.
///
/// Like a parse-only invocation, a single file is required. Unlike a
/// parse-only invocation, module imports will be processed.
void performParseAndResolveImportsOnly();

private:
SourceFile *
createSourceFileForMainModule(SourceFileKind FileKind,
Expand Down Expand Up @@ -581,7 +585,9 @@ class CompilerInstance {

void addMainFileToModule(const ImplicitImports &implicitImports);

void parseAndCheckTypes(const ImplicitImports &implicitImports);
void performSemaUpTo(SourceFile::ASTStage_t LimitStage);
void parseAndCheckTypesUpTo(const ImplicitImports &implicitImports,
SourceFile::ASTStage_t LimitStage);

void parseLibraryFile(unsigned BufferID,
const ImplicitImports &implicitImports,
Expand All @@ -600,9 +606,10 @@ class CompilerInstance {

void forEachFileToTypeCheck(llvm::function_ref<void(SourceFile &)> fn);

void parseAndTypeCheckMainFile(PersistentParserState &PersistentState,
DelayedParsingCallbacks *DelayedParseCB,
OptionSet<TypeCheckingFlags> TypeCheckOptions);
void parseAndTypeCheckMainFileUpTo(SourceFile::ASTStage_t LimitStage,
PersistentParserState &PersistentState,
DelayedParsingCallbacks *DelayedParseCB,
OptionSet<TypeCheckingFlags> TypeCheckOptions);

void finishTypeChecking(OptionSet<TypeCheckingFlags> TypeCheckOptions);

Expand Down
2 changes: 2 additions & 0 deletions include/swift/Frontend/FrontendOptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ class FrontendOptions {
enum class ActionType {
NoneAction, ///< No specific action
Parse, ///< Parse only
ResolveImports, ///< Parse and resolve imports only
Typecheck, ///< Parse and type-check only
DumpParse, ///< Parse only and dump AST
DumpInterfaceHash, ///< Parse and dump the interface token hash.
Expand Down Expand Up @@ -296,6 +297,7 @@ class FrontendOptions {

private:
static bool canActionEmitDependencies(ActionType);
static bool canActionEmitReferenceDependencies(ActionType);
static bool canActionEmitObjCHeader(ActionType);
static bool canActionEmitLoadedModuleTrace(ActionType);
static bool canActionEmitModule(ActionType);
Expand Down
3 changes: 3 additions & 0 deletions include/swift/Option/Options.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,9 @@ def fixit_all : Flag<["-"], "fixit-all">,
def parse: Flag<["-"], "parse">,
HelpText<"Parse input file(s)">, ModeOpt,
Flags<[FrontendOption, NoInteractiveOption, DoesNotAffectIncrementalBuild]>;
def resolve_imports : Flag<["-"], "resolve-imports">,
HelpText<"Parse and resolve imports in input file(s)">, ModeOpt,
Flags<[FrontendOption, NoInteractiveOption, DoesNotAffectIncrementalBuild]>;
def typecheck : Flag<["-"], "typecheck">,
HelpText<"Parse and type-check input file(s)">, ModeOpt,
Flags<[FrontendOption, NoInteractiveOption, DoesNotAffectIncrementalBuild]>;
Expand Down
1 change: 1 addition & 0 deletions lib/Driver/Driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1370,6 +1370,7 @@ void Driver::buildOutputInfo(const ToolChain &TC, const DerivedArgList &Args,
OI.LinkAction = LinkKind::None;
break;
case options::OPT_parse:
case options::OPT_resolve_imports:
case options::OPT_typecheck:
case options::OPT_dump_parse:
case options::OPT_dump_ast:
Expand Down
8 changes: 8 additions & 0 deletions lib/Frontend/ArgsToFrontendOptionsConverter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,8 @@ ArgsToFrontendOptionsConverter::determineRequestedAction(const ArgList &args) {
return FrontendOptions::ActionType::EmitImportedModules;
if (Opt.matches(OPT_parse))
return FrontendOptions::ActionType::Parse;
if (Opt.matches(OPT_resolve_imports))
return FrontendOptions::ActionType::ResolveImports;
if (Opt.matches(OPT_typecheck))
return FrontendOptions::ActionType::Typecheck;
if (Opt.matches(OPT_dump_parse))
Expand Down Expand Up @@ -467,6 +469,12 @@ bool ArgsToFrontendOptionsConverter::checkUnusedSupplementaryOutputPaths()
Diags.diagnose(SourceLoc(), diag::error_mode_cannot_emit_dependencies);
return true;
}
if (!FrontendOptions::canActionEmitReferenceDependencies(Opts.RequestedAction)
&& Opts.InputsAndOutputs.hasReferenceDependenciesPath()) {
Diags.diagnose(SourceLoc(),
diag::error_mode_cannot_emit_reference_dependencies);
return true;
}
if (!FrontendOptions::canActionEmitObjCHeader(Opts.RequestedAction) &&
Opts.InputsAndOutputs.hasObjCHeaderOutputPath()) {
Diags.diagnose(SourceLoc(), diag::error_mode_cannot_emit_header);
Expand Down
58 changes: 45 additions & 13 deletions lib/Frontend/Frontend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,21 @@ shouldImplicityImportSwiftOnoneSupportModule(CompilerInvocation &Invocation) {
return Invocation.getFrontendOptions().isCreatingSIL();
}

void CompilerInstance::performParseAndResolveImportsOnly() {
performSemaUpTo(SourceFile::NameBound);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm personally not sure we need these helper functions, and think we could just add an argument to performSema (and deprecate performParseOnly later). But either way is okay with me.

}

void CompilerInstance::performSema() {
performSemaUpTo(SourceFile::TypeChecked);
}

void CompilerInstance::performSemaUpTo(SourceFile::ASTStage_t LimitStage) {
// FIXME: A lot of the logic in `performParseOnly` is a stripped-down version
// of the logic in `performSemaUpTo`. We should try to unify them over time.
if (LimitStage <= SourceFile::Parsed) {
return performParseOnly();
}

FrontendStatsTracer tracer(Context->Stats, "perform-sema");
Context->LoadedModules[MainModule->getName()] = getMainModule();

Expand Down Expand Up @@ -457,7 +471,7 @@ void CompilerInstance::performSema() {
if (MainBufferID != NO_SUCH_BUFFER)
addMainFileToModule(implicitImports);

parseAndCheckTypes(implicitImports);
parseAndCheckTypesUpTo(implicitImports, LimitStage);
}

CompilerInstance::ImplicitImports::ImplicitImports(CompilerInstance &compiler) {
Expand Down Expand Up @@ -575,8 +589,8 @@ void CompilerInstance::addMainFileToModule(
addAdditionalInitialImportsTo(MainFile, implicitImports);
}

void CompilerInstance::parseAndCheckTypes(
const ImplicitImports &implicitImports) {
void CompilerInstance::parseAndCheckTypesUpTo(
const ImplicitImports &implicitImports, SourceFile::ASTStage_t limitStage) {
FrontendStatsTracer tracer(Context->Stats, "parse-and-check-types");
// Delayed parsing callback for the primary file, or all files
// in non-WMO mode.
Expand Down Expand Up @@ -611,8 +625,8 @@ void CompilerInstance::parseAndCheckTypes(
// In addition, the main file has parsing and type-checking
// interwined.
if (MainBufferID != NO_SUCH_BUFFER) {
parseAndTypeCheckMainFile(PersistentState, PrimaryDelayedCB.get(),
TypeCheckOptions);
parseAndTypeCheckMainFileUpTo(limitStage, PersistentState,
PrimaryDelayedCB.get(), TypeCheckOptions);
}

assert(llvm::all_of(MainModule->getFiles(), [](const FileUnit *File) -> bool {
Expand All @@ -623,6 +637,11 @@ void CompilerInstance::parseAndCheckTypes(
}) && "some files have not yet had their imports resolved");
MainModule->setHasResolvedImports();

// If the limiting AST stage is name binding, we're done.
if (limitStage <= SourceFile::NameBound) {
return;
}

const auto &options = Invocation.getFrontendOptions();
forEachFileToTypeCheck([&](SourceFile &SF) {
performTypeChecking(SF, PersistentState.getTopLevelContext(),
Expand Down Expand Up @@ -720,7 +739,8 @@ bool CompilerInstance::parsePartialModulesAndLibraryFiles(
return hadLoadError;
}

void CompilerInstance::parseAndTypeCheckMainFile(
void CompilerInstance::parseAndTypeCheckMainFileUpTo(
SourceFile::ASTStage_t LimitStage,
PersistentParserState &PersistentState,
DelayedParsingCallbacks *DelayedParseCB,
OptionSet<TypeCheckingFlags> TypeCheckOptions) {
Expand All @@ -747,15 +767,27 @@ void CompilerInstance::parseAndTypeCheckMainFile(
parseIntoSourceFile(MainFile, MainFile.getBufferID().getValue(), &Done,
TheSILModule ? &SILContext : nullptr, &PersistentState,
DelayedParseCB);

if (mainIsPrimary) {
const auto &options = Invocation.getFrontendOptions();
performTypeChecking(MainFile, PersistentState.getTopLevelContext(),
TypeCheckOptions, CurTUElem,
options.WarnLongFunctionBodies,
options.WarnLongExpressionTypeChecking,
options.SolverExpressionTimeThreshold,
options.SwitchCheckingInvocationThreshold);
switch (LimitStage) {
case SourceFile::Parsing:
case SourceFile::Parsed:
llvm_unreachable("invalid limit stage");
case SourceFile::NameBound:
performNameBinding(MainFile, CurTUElem);
break;
case SourceFile::TypeChecked:
const auto &options = Invocation.getFrontendOptions();
performTypeChecking(MainFile, PersistentState.getTopLevelContext(),
TypeCheckOptions, CurTUElem,
options.WarnLongFunctionBodies,
options.WarnLongExpressionTypeChecking,
options.SolverExpressionTimeThreshold,
options.SwitchCheckingInvocationThreshold);
break;
}
}

CurTUElem = MainFile.Decls.size();
} while (!Done);

Expand Down
41 changes: 41 additions & 0 deletions lib/Frontend/FrontendOptions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ bool FrontendOptions::needsProperModuleName(ActionType action) {
switch (action) {
case ActionType::NoneAction:
case ActionType::Parse:
case ActionType::ResolveImports:
case ActionType::Typecheck:
case ActionType::DumpParse:
case ActionType::DumpAST:
Expand Down Expand Up @@ -65,6 +66,7 @@ bool FrontendOptions::isActionImmediate(ActionType action) {
switch (action) {
case ActionType::NoneAction:
case ActionType::Parse:
case ActionType::ResolveImports:
case ActionType::Typecheck:
case ActionType::DumpParse:
case ActionType::DumpAST:
Expand Down Expand Up @@ -134,6 +136,7 @@ FrontendOptions::suffixForPrincipalOutputFileForAction(ActionType action) {
return StringRef();

case ActionType::Parse:
case ActionType::ResolveImports:
case ActionType::Typecheck:
case ActionType::DumpParse:
case ActionType::DumpInterfaceHash:
Expand Down Expand Up @@ -184,6 +187,7 @@ FrontendOptions::suffixForPrincipalOutputFileForAction(ActionType action) {
bool FrontendOptions::canActionEmitDependencies(ActionType action) {
switch (action) {
case ActionType::NoneAction:
case ActionType::Parse:
case ActionType::DumpParse:
case ActionType::DumpInterfaceHash:
case ActionType::DumpAST:
Expand All @@ -194,7 +198,39 @@ bool FrontendOptions::canActionEmitDependencies(ActionType action) {
case ActionType::Immediate:
case ActionType::REPL:
return false;
case ActionType::ResolveImports:
case ActionType::Typecheck:
case ActionType::MergeModules:
case ActionType::EmitModuleOnly:
case ActionType::EmitPCH:
case ActionType::EmitSILGen:
case ActionType::EmitSIL:
case ActionType::EmitSIBGen:
case ActionType::EmitSIB:
case ActionType::EmitIR:
case ActionType::EmitBC:
case ActionType::EmitAssembly:
case ActionType::EmitObject:
case ActionType::EmitImportedModules:
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if -emit-imported-modules should be demoted from an action later…

return true;
}
}

bool FrontendOptions::canActionEmitReferenceDependencies(ActionType action) {
switch (action) {
case ActionType::NoneAction:
case ActionType::Parse:
case ActionType::ResolveImports:
case ActionType::DumpParse:
case ActionType::DumpInterfaceHash:
case ActionType::DumpAST:
case ActionType::EmitSyntax:
case ActionType::PrintAST:
case ActionType::DumpScopeMaps:
case ActionType::DumpTypeRefinementContexts:
case ActionType::Immediate:
case ActionType::REPL:
return false;
case ActionType::Typecheck:
case ActionType::MergeModules:
case ActionType::EmitModuleOnly:
Expand Down Expand Up @@ -227,6 +263,7 @@ bool FrontendOptions::canActionEmitObjCHeader(ActionType action) {
case ActionType::REPL:
return false;
case ActionType::Parse:
case ActionType::ResolveImports:
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I think Parse and ResolveImports belong in the "false" section for this one. It definitely needs a type-checked AST.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll submit another patch for that one.

case ActionType::Typecheck:
case ActionType::MergeModules:
case ActionType::EmitModuleOnly:
Expand Down Expand Up @@ -257,6 +294,7 @@ bool FrontendOptions::canActionEmitLoadedModuleTrace(ActionType action) {
case ActionType::Immediate:
case ActionType::REPL:
return false;
case ActionType::ResolveImports:
case ActionType::Typecheck:
case ActionType::MergeModules:
case ActionType::EmitModuleOnly:
Expand All @@ -278,6 +316,7 @@ bool FrontendOptions::canActionEmitModule(ActionType action) {
switch (action) {
case ActionType::NoneAction:
case ActionType::Parse:
case ActionType::ResolveImports:
case ActionType::Typecheck:
case ActionType::DumpParse:
case ActionType::DumpInterfaceHash:
Expand Down Expand Up @@ -312,6 +351,7 @@ bool FrontendOptions::canActionEmitModuleDoc(ActionType action) {
bool FrontendOptions::doesActionProduceOutput(ActionType action) {
switch (action) {
case ActionType::Parse:
case ActionType::ResolveImports:
case ActionType::Typecheck:
case ActionType::DumpParse:
case ActionType::DumpAST:
Expand Down Expand Up @@ -357,6 +397,7 @@ bool FrontendOptions::doesActionProduceTextualOutput(ActionType action) {
return false;

case ActionType::Parse:
case ActionType::ResolveImports:
case ActionType::Typecheck:
case ActionType::DumpParse:
case ActionType::DumpInterfaceHash:
Expand Down
21 changes: 13 additions & 8 deletions lib/FrontendTool/FrontendTool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -895,11 +895,14 @@ static bool performCompile(CompilerInstance &Instance,
if (Invocation.getInputKind() == InputFileKind::IFK_LLVM_IR)
return compileLLVMIR(Invocation, Instance, Stats);

if (FrontendOptions::shouldActionOnlyParse(Action))
if (FrontendOptions::shouldActionOnlyParse(Action)) {
Instance.performParseOnly(/*EvaluateConditionals*/
Action == FrontendOptions::ActionType::EmitImportedModules);
else
} else if (Action == FrontendOptions::ActionType::ResolveImports) {
Instance.performParseAndResolveImportsOnly();
} else {
Instance.performSema();
}

SWIFT_DEFER {
// Emit request-evaluator graph via GraphViz, if requested.
Expand All @@ -912,8 +915,15 @@ static bool performCompile(CompilerInstance &Instance,
}
};

ASTContext &Context = Instance.getASTContext();
if (Action == FrontendOptions::ActionType::Parse)
return Instance.getASTContext().hadError();
return Context.hadError();

(void)emitMakeDependenciesIfNeeded(Context.Diags,
Instance.getDependencyTracker(), opts);

if (Action == FrontendOptions::ActionType::ResolveImports)
return Context.hadError();

if (observer)
observer->performedSemanticAnalysis(Instance);
Expand All @@ -929,8 +939,6 @@ static bool performCompile(CompilerInstance &Instance,
debugFailWithCrash();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Arguably these crashes should happen even in -parse or -resolve-imports.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For future reference: Had an offline chat about this. Filed SR-8252 about it.


ASTContext &Context = Instance.getASTContext();

verifyGenericSignaturesIfNeeded(Invocation, Context);

(void)migrator::updateCodeAndEmitRemapIfNeeded(&Instance, Invocation);
Expand All @@ -948,9 +956,6 @@ static bool performCompile(CompilerInstance &Instance,
if (opts.PrintClangStats && Context.getClangModuleLoader())
Context.getClangModuleLoader()->printStatistics();

(void)emitMakeDependenciesIfNeeded(Context.Diags,
Instance.getDependencyTracker(), opts);

emitReferenceDependenciesForAllPrimaryInputsIfNeeded(Invocation, Instance);

(void)emitLoadedModuleTraceForAllPrimariesIfNeeded(
Expand Down
Loading