Skip to content

Verify that just-emitted module interfaces parse and typecheck #33114

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 6 commits into from
Aug 25, 2020
Merged

Verify that just-emitted module interfaces parse and typecheck #33114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9bc787e
Add a “typecheck module interface” mode
beccadax Jul 24, 2020
d48e397
Support --driver-mode with swift-frontend
beccadax Jul 25, 2020
3c69837
Add a debug flag to emit invalid swiftinterface files
beccadax Jul 25, 2020
5f523fe
Add flag to verify just-emitted module interfaces
beccadax Aug 8, 2020
22e3cff
Tweak diagnostic for failed module interface verification
beccadax Aug 3, 2020
f824204
Verify interfaces by default in assert compilers
beccadax Aug 8, 2020
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: 6 additions & 6 deletions include/swift/AST/DiagnosticsSema.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -701,12 +701,12 @@ ERROR(sema_opening_import,Fatal,
ERROR(serialization_load_failed,Fatal,
"failed to load module '%0'", (StringRef))
ERROR(module_interface_build_failed,Fatal,
"failed to build module '%0' from its module interface; "
"%select{the compiler that produced it, '%2', may have used features "
"that aren't supported by this compiler, '%3'"
"|it may have been damaged or it may have triggered a bug in the Swift "
"compiler when it was produced}1",
(StringRef, bool, StringRef, StringRef))
"failed to %select{build module '%1' from its module interface|verify "
"module interface of '%1'}0; %select{the compiler that produced it, "
"'%3', may have used features that aren't supported by this compiler, "
"'%4'|it may have been damaged or it may have triggered a bug in the "
"Swift compiler when it was produced}2",
(bool, StringRef, bool, StringRef, StringRef))
ERROR(serialization_malformed_module,Fatal,
"malformed compiled module: %0", (StringRef))
ERROR(serialization_module_too_new,Fatal,
Expand Down
23 changes: 22 additions & 1 deletion include/swift/Driver/Action.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,10 @@ class Action {
GenerateDSYMJob,
VerifyDebugInfoJob,
GeneratePCHJob,
VerifyModuleInterfaceJob,

JobFirst = CompileJob,
JobLast = GeneratePCHJob
JobLast = VerifyModuleInterfaceJob
};

static const char *getClassName(Kind AC);
Expand Down Expand Up @@ -357,6 +358,26 @@ class StaticLinkJobAction : public JobAction {
}
};

class VerifyModuleInterfaceJobAction : public JobAction {
virtual void anchor();
file_types::ID inputType;

public:
VerifyModuleInterfaceJobAction(const Action * ModuleEmitter,
file_types::ID inputType)
: JobAction(Action::Kind::VerifyModuleInterfaceJob, { ModuleEmitter },
file_types::TY_Nothing), inputType(inputType) {
assert(inputType == file_types::TY_SwiftModuleInterfaceFile ||
inputType == file_types::TY_PrivateSwiftModuleInterfaceFile);
}

file_types::ID getInputType() const { return inputType; }

static bool classof(const Action *A) {
return A->getKind() == Action::Kind::VerifyModuleInterfaceJob;
}
};

} // end namespace driver
} // end namespace swift

Expand Down
3 changes: 3 additions & 0 deletions include/swift/Driver/ToolChain.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ class ToolChain {
virtual InvocationInfo
constructInvocation(const VerifyDebugInfoJobAction &job,
const JobContext &context) const;
virtual InvocationInfo
constructInvocation(const VerifyModuleInterfaceJobAction &job,
const JobContext &context) const;
virtual InvocationInfo constructInvocation(const GeneratePCHJobAction &job,
const JobContext &context) const;
virtual InvocationInfo
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 @@ -116,6 +116,8 @@ class FrontendOptions {

/// Build from a swiftinterface, as close to `import` as possible
CompileModuleFromInterface,
/// Same as CompileModuleFromInterface, but stopping after typechecking
TypecheckModuleFromInterface,
Copy link
Contributor

Choose a reason for hiding this comment

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

Great addition overall! Why not we just reuse CompileModuleFromInterface action instead of defining a new one? We could potentially add another flag to indicate stopping after typechecking for -compile-module-from-interface. Maintaining an additional frontend action doesn't seem appealing to me.

Copy link
Contributor Author

@beccadax beccadax Aug 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

-typecheck and -compile-module-from-interface are both compiler modes (ModeOpts), so they're mutually exclusive. Also note that their behavior is pretty different—if you take a look at the FrontendOptions.cpp diff, ActionType::TypecheckModuleFromInterface's behavior tends to match ActionType::Typecheck, not ActionType::CompileModuleFromInterface.

There's something to be said for the idea of removing -compile-module-from-interface and instead having people use -emit-module to do that; either there would be a flag to restart the compilation in the module interface builder, or this would just happen automatically when you passed a module interface. This would have a couple of nice benefits, like allowing other flags such as -dump-ast or -emit-sil. But that would be a much bigger change and I'm not sure it can be fully justified.

(Ultimately, I think the new driver probably ought to treat .swiftinterface inputs as implicitly being requests to either compile or typecheck the interface—not sure which. But that's a whole different story, and I think it's better handled in the new driver instead of the old one.)


EmitSIBGen, ///< Emit serialized AST + raw SIL
EmitSIB, ///< Emit serialized AST + canonical SIL
Expand Down
14 changes: 13 additions & 1 deletion include/swift/Frontend/ModuleInterfaceLoader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,8 @@ class ExplicitModuleMapParser {
};

struct ModuleInterfaceLoaderOptions {
FrontendOptions::ActionType requestedAction =
FrontendOptions::ActionType::EmitModuleOnly;
bool remarkOnRebuildFromInterface = false;
bool disableInterfaceLock = false;
bool disableImplicitSwiftModule = false;
Expand All @@ -271,7 +273,17 @@ struct ModuleInterfaceLoaderOptions {
remarkOnRebuildFromInterface(Opts.RemarkOnRebuildFromModuleInterface),
disableInterfaceLock(Opts.DisableInterfaceFileLock),
disableImplicitSwiftModule(Opts.DisableImplicitModules),
mainExecutablePath(Opts.MainExecutablePath) {}
mainExecutablePath(Opts.MainExecutablePath)
{
switch (Opts.RequestedAction) {
case FrontendOptions::ActionType::TypecheckModuleFromInterface:
requestedAction = FrontendOptions::ActionType::Typecheck;
break;
default:
requestedAction = FrontendOptions::ActionType::EmitModuleOnly;
break;
}
}
ModuleInterfaceLoaderOptions() = default;
};

Expand Down
5 changes: 4 additions & 1 deletion include/swift/Frontend/ModuleInterfaceSupport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,15 @@ struct ModuleInterfaceOptions {
/// back .swiftinterface and reconstructing .swiftmodule.
std::string Flags;

// Print SPI decls and attributes.
/// Print SPI decls and attributes.
bool PrintSPIs = false;

/// Print imports with both @_implementationOnly and @_spi, only applies
/// when PrintSPIs is true.
bool ExperimentalSPIImports = false;

/// Intentionally print invalid syntax into the file.
bool DebugPrintInvalidSyntax = false;
};

extern version::Version InterfaceFormatVersion;
Expand Down
8 changes: 8 additions & 0 deletions include/swift/Option/FrontendOptions.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,9 @@ def debug_forbid_typecheck_prefix : Separate<["-"], "debug-forbid-typecheck-pref
HelpText<"Triggers llvm fatal_error if typechecker tries to typecheck a decl "
"with the provided prefix name">;

def debug_emit_invalid_swiftinterface_syntax : Flag<["-"], "debug-emit-invalid-swiftinterface-syntax">,
HelpText<"Write an invalid declaration into swiftinterface files">;

def debug_cycles : Flag<["-"], "debug-cycles">,
HelpText<"Print out debug dumps when cycles are detected in evaluation">;
def build_request_dependency_graph : Flag<["-"], "build-request-dependency-graph">,
Expand Down Expand Up @@ -641,6 +644,11 @@ def build_module_from_parseable_interface :
Alias<compile_module_from_interface>,
ModeOpt;

def typecheck_module_from_interface :
Flag<["-"], "typecheck-module-from-interface">,
HelpText<"Treat the (single) input as a swiftinterface and typecheck it">,
ModeOpt;

def module_interface_preserve_types_as_written :
Flag<["-"], "module-interface-preserve-types-as-written">,
HelpText<"When emitting a module interface, preserve types as they were "
Expand Down
9 changes: 9 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 @@ -460,6 +460,15 @@ def emit_private_module_interface_path :
DoesNotAffectIncrementalBuild, ArgumentIsPath, SupplementaryOutput]>,
MetaVarName<"<path>">, HelpText<"Output private module interface file to <path>">;

def verify_emitted_module_interface :
Flag<["-"], "verify-emitted-module-interface">,
Flags<[NoInteractiveOption, DoesNotAffectIncrementalBuild]>,
HelpText<"Check that module interfaces emitted during compilation typecheck">;
def no_verify_emitted_module_interface :
Flag<["-"], "no-verify-emitted-module-interface">,
Flags<[NoInteractiveOption, DoesNotAffectIncrementalBuild]>,
HelpText<"Don't check that module interfaces emitted during compilation typecheck">;

def avoid_emit_module_source_info :
Flag<["-"], "avoid-emit-module-source-info">,
Flags<[NoInteractiveOption, DoesNotAffectIncrementalBuild]>,
Expand Down
3 changes: 3 additions & 0 deletions lib/Driver/Action.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const char *Action::getClassName(Kind AC) {
case Kind::GenerateDSYMJob: return "generate-dSYM";
case Kind::VerifyDebugInfoJob: return "verify-debug-info";
case Kind::GeneratePCHJob: return "generate-pch";
case Kind::VerifyModuleInterfaceJob: return "verify-module-interface";
}

llvm_unreachable("invalid class");
Expand Down Expand Up @@ -65,3 +66,5 @@ void GenerateDSYMJobAction::anchor() {}
void VerifyDebugInfoJobAction::anchor() {}

void GeneratePCHJobAction::anchor() {}

void VerifyModuleInterfaceJobAction::anchor() {}
24 changes: 24 additions & 0 deletions lib/Driver/Driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2208,6 +2208,30 @@ void Driver::buildActions(SmallVectorImpl<const Action *> &TopLevelActions,
TopLevelActions.push_back(MergeModuleAction);
TopLevelActions.append(AllLinkerInputs.begin(), AllLinkerInputs.end());
}

#ifdef NDEBUG
bool verifyInterfacesByDefault = false;
#else
bool verifyInterfacesByDefault = true;
#endif

if (MergeModuleAction
&& Args.hasFlag(options::OPT_verify_emitted_module_interface,
options::OPT_no_verify_emitted_module_interface,
verifyInterfacesByDefault)) {
if (Args.hasArgNoClaim(options::OPT_emit_module_interface,
options::OPT_emit_module_interface_path)) {
TopLevelActions.push_back(
C.createAction<VerifyModuleInterfaceJobAction>(MergeModuleAction,
file_types::TY_SwiftModuleInterfaceFile));
}

if (Args.hasArgNoClaim(options::OPT_emit_private_module_interface_path)) {
TopLevelActions.push_back(
C.createAction<VerifyModuleInterfaceJobAction>(MergeModuleAction,
file_types::TY_PrivateSwiftModuleInterfaceFile));
}
}
}

bool Driver::handleImmediateArgs(const ArgList &Args, const ToolChain &TC) {
Expand Down
1 change: 1 addition & 0 deletions lib/Driver/ToolChain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ std::unique_ptr<Job> ToolChain::constructJob(
CASE(GeneratePCHJob)
CASE(AutolinkExtractJob)
CASE(REPLJob)
CASE(VerifyModuleInterfaceJob)
#undef CASE
case Action::Kind::Input:
llvm_unreachable("not a JobAction");
Expand Down
35 changes: 35 additions & 0 deletions lib/Driver/ToolChains.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1047,6 +1047,41 @@ ToolChain::constructInvocation(const MergeModuleJobAction &job,
return II;
}

ToolChain::InvocationInfo
ToolChain::constructInvocation(const VerifyModuleInterfaceJobAction &job,
const JobContext &context) const {
InvocationInfo II{SWIFT_EXECUTABLE_NAME};
ArgStringList &Arguments = II.Arguments;
II.allowsResponseFiles = true;

for (auto &s : getDriver().getSwiftProgramArgs())
Arguments.push_back(s.c_str());
Arguments.push_back("-frontend");

Arguments.push_back("-typecheck-module-from-interface");

size_t sizeBefore = Arguments.size();
addInputsOfType(Arguments, context.Inputs, context.Args, job.getInputType());

(void)sizeBefore;
assert(Arguments.size() - sizeBefore == 1 &&
"should verify exactly one module interface per job");

addCommonFrontendArgs(context.OI, context.Output, context.Args, Arguments);
addRuntimeLibraryFlags(context.OI, Arguments);

addOutputsOfType(Arguments, context.Output, context.Args,
file_types::TY_SerializedDiagnostics,
"-serialize-diagnostics-path");

context.Args.AddLastArg(Arguments, options::OPT_import_objc_header);

Arguments.push_back("-module-name");
Arguments.push_back(context.Args.MakeArgString(context.OI.ModuleName));

return II;
}

ToolChain::InvocationInfo
ToolChain::constructInvocation(const ModuleWrapJobAction &job,
const JobContext &context) const {
Expand Down
5 changes: 4 additions & 1 deletion lib/Frontend/ArgsToFrontendOptionsConverter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@ bool ArgsToFrontendOptionsConverter::convert(
Opts.RequestedAction = determineRequestedAction(Args);
}

if (Opts.RequestedAction == FrontendOptions::ActionType::CompileModuleFromInterface) {
if (Opts.RequestedAction == FrontendOptions::ActionType::CompileModuleFromInterface ||
Opts.RequestedAction == FrontendOptions::ActionType::TypecheckModuleFromInterface) {
// The situations where we use this action, e.g. explicit module building and
// generating prebuilt module cache, don't need synchronization. We should avoid
// using lock files for them.
Expand Down Expand Up @@ -389,6 +390,8 @@ ArgsToFrontendOptionsConverter::determineRequestedAction(const ArgList &args) {
return FrontendOptions::ActionType::Immediate;
if (Opt.matches(OPT_compile_module_from_interface))
return FrontendOptions::ActionType::CompileModuleFromInterface;
if (Opt.matches(OPT_typecheck_module_from_interface))
return FrontendOptions::ActionType::TypecheckModuleFromInterface;

llvm_unreachable("Unhandled mode option");
}
Expand Down
2 changes: 2 additions & 0 deletions lib/Frontend/CompilerInvocation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,8 @@ static void ParseModuleInterfaceArgs(ModuleInterfaceOptions &Opts,
Args.hasArg(OPT_experimental_print_full_convention);
Opts.ExperimentalSPIImports |=
Args.hasArg(OPT_experimental_spi_imports);
Opts.DebugPrintInvalidSyntax |=
Args.hasArg(OPT_debug_emit_invalid_swiftinterface_syntax);
}

/// Save a copy of any flags marked as ModuleInterfaceOption, if running
Expand Down
4 changes: 3 additions & 1 deletion lib/Frontend/Frontend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,9 @@ void CompilerInstance::recordPrimaryInputBuffer(unsigned BufID) {

bool CompilerInstance::setUpASTContextIfNeeded() {
if (Invocation.getFrontendOptions().RequestedAction ==
FrontendOptions::ActionType::CompileModuleFromInterface) {
FrontendOptions::ActionType::CompileModuleFromInterface ||
Invocation.getFrontendOptions().RequestedAction ==
FrontendOptions::ActionType::TypecheckModuleFromInterface) {
// Compiling a module interface from source uses its own CompilerInstance
// with options read from the input file. Don't bother setting up an
// ASTContext at this level.
Expand Down
12 changes: 12 additions & 0 deletions lib/Frontend/FrontendOptions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ bool FrontendOptions::needsProperModuleName(ActionType action) {
case ActionType::EmitModuleOnly:
case ActionType::MergeModules:
case ActionType::CompileModuleFromInterface:
case ActionType::TypecheckModuleFromInterface:
return true;
case ActionType::Immediate:
case ActionType::REPL:
Expand Down Expand Up @@ -118,6 +119,7 @@ FrontendOptions::formatForPrincipalOutputFileForAction(ActionType action) {
case ActionType::Parse:
case ActionType::ResolveImports:
case ActionType::Typecheck:
case ActionType::TypecheckModuleFromInterface:
case ActionType::DumpParse:
case ActionType::DumpInterfaceHash:
case ActionType::DumpAST:
Expand Down Expand Up @@ -192,6 +194,7 @@ bool FrontendOptions::canActionEmitDependencies(ActionType action) {
case ActionType::DumpTypeRefinementContexts:
case ActionType::DumpTypeInfo:
case ActionType::CompileModuleFromInterface:
case ActionType::TypecheckModuleFromInterface:
case ActionType::Immediate:
case ActionType::REPL:
case ActionType::DumpPCM:
Expand Down Expand Up @@ -232,6 +235,7 @@ bool FrontendOptions::canActionEmitReferenceDependencies(ActionType action) {
case ActionType::DumpTypeRefinementContexts:
case ActionType::DumpTypeInfo:
case ActionType::CompileModuleFromInterface:
case ActionType::TypecheckModuleFromInterface:
case ActionType::Immediate:
case ActionType::REPL:
case ActionType::EmitPCM:
Expand Down Expand Up @@ -280,6 +284,7 @@ bool FrontendOptions::canActionEmitObjCHeader(ActionType action) {
case ActionType::DumpTypeRefinementContexts:
case ActionType::DumpTypeInfo:
case ActionType::CompileModuleFromInterface:
case ActionType::TypecheckModuleFromInterface:
case ActionType::Immediate:
case ActionType::REPL:
case ActionType::EmitPCM:
Expand Down Expand Up @@ -317,6 +322,7 @@ bool FrontendOptions::canActionEmitLoadedModuleTrace(ActionType action) {
case ActionType::DumpTypeRefinementContexts:
case ActionType::DumpTypeInfo:
case ActionType::CompileModuleFromInterface:
case ActionType::TypecheckModuleFromInterface:
case ActionType::Immediate:
case ActionType::REPL:
case ActionType::EmitPCM:
Expand Down Expand Up @@ -360,6 +366,7 @@ bool FrontendOptions::canActionEmitModule(ActionType action) {
case ActionType::DumpTypeInfo:
case ActionType::EmitSILGen:
case ActionType::CompileModuleFromInterface:
case ActionType::TypecheckModuleFromInterface:
case ActionType::Immediate:
case ActionType::REPL:
case ActionType::EmitPCM:
Expand Down Expand Up @@ -404,6 +411,7 @@ bool FrontendOptions::canActionEmitInterface(ActionType action) {
case ActionType::EmitSILGen:
case ActionType::EmitSIBGen:
case ActionType::CompileModuleFromInterface:
case ActionType::TypecheckModuleFromInterface:
case ActionType::Immediate:
case ActionType::REPL:
case ActionType::EmitPCM:
Expand Down Expand Up @@ -450,6 +458,7 @@ bool FrontendOptions::doesActionProduceOutput(ActionType action) {
case ActionType::EmitImportedModules:
case ActionType::MergeModules:
case ActionType::CompileModuleFromInterface:
case ActionType::TypecheckModuleFromInterface:
case ActionType::DumpTypeInfo:
case ActionType::EmitPCM:
case ActionType::DumpPCM:
Expand Down Expand Up @@ -484,6 +493,7 @@ bool FrontendOptions::doesActionProduceTextualOutput(ActionType action) {
case ActionType::Parse:
case ActionType::ResolveImports:
case ActionType::Typecheck:
case ActionType::TypecheckModuleFromInterface:
case ActionType::DumpParse:
case ActionType::DumpInterfaceHash:
case ActionType::DumpAST:
Expand Down Expand Up @@ -521,6 +531,7 @@ bool FrontendOptions::doesActionGenerateSIL(ActionType action) {
case ActionType::EmitImportedModules:
case ActionType::EmitPCH:
case ActionType::CompileModuleFromInterface:
case ActionType::TypecheckModuleFromInterface:
case ActionType::EmitPCM:
case ActionType::DumpPCM:
case ActionType::ScanDependencies:
Expand Down Expand Up @@ -557,6 +568,7 @@ bool FrontendOptions::doesActionGenerateIR(ActionType action) {
case ActionType::DumpTypeRefinementContexts:
case ActionType::DumpTypeInfo:
case ActionType::CompileModuleFromInterface:
case ActionType::TypecheckModuleFromInterface:
case ActionType::Typecheck:
case ActionType::ResolveImports:
case ActionType::MergeModules:
Expand Down
Loading