Skip to content

[Frontend] Allow -emit-interface with -typecheck #19676

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 1 commit into from
Oct 3, 2018

Conversation

jrose-apple
Copy link
Contributor

Textual module interfaces don't actually depend on SILGen, so we shouldn't need to run SILGen (or serialize an entire binary module) if we're just trying to emit a textual interface. On the other hand, if we are going to run SILGen and then SIL diagnostics, we shouldn't delay those diagnostics by spending time emitting a textual interface, or for that matter a TBD file.

Using this, update all the ModuleInterface tests that use -emit-module -o /dev/null to use -typecheck instead, except for those using -merge-modules.

@jrose-apple
Copy link
Contributor Author

@swift-ci Please smoke test

@jrose-apple
Copy link
Contributor Author

@swift-ci Please test source compatibility

/// These are extracted out so that they can be invoked early when using
/// `-typecheck`, but skipped for any mode that runs SIL diagnostics if there's
/// an error found there (to get those diagnostics back to the user faster).
static bool emitAnyWholeModulePostTypeCheckSupplementaryOutputs(
Copy link
Contributor

Choose a reason for hiding this comment

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

This is much nicer, thank you!

@@ -1,3 +1,4 @@
// RUN: %target-swift-frontend -typecheck -emit-interface-path - -parse-stdlib %s | %FileCheck %s
// RUN: %target-swift-frontend -emit-interface-path - -emit-module -o /dev/null -parse-stdlib %s | %FileCheck %s
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this line can be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I wanted to have at least one test that did things both ways; a -parse-stdlib test seemed like a good example.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good, then!

@jrose-apple
Copy link
Contributor Author

Aha, yes, the very thing I changed was, in fact, being tested.

Textual module interfaces don't actually depend on SILGen, so we
shouldn't need to run SILGen (or serialize an entire binary module) if
we're just trying to emit a textual interface. On the other hand, if
we /are/ going to run SILGen and then SIL diagnostics, we shouldn't
delay those diagnostics by spending time emitting a textual interface,
or for that matter a TBD file.

Using this, update all the ModuleInterface tests that use
`-emit-module -o /dev/null` to use `-typecheck` instead, except for
those using `-merge-modules`.
@jrose-apple
Copy link
Contributor Author

Debug source compat, Release source compat.

@swift-ci Please smoke test

@jrose-apple jrose-apple merged commit 3ea7544 into swiftlang:master Oct 3, 2018
@jrose-apple jrose-apple deleted the checkmate branch October 3, 2018 22:07
modelorganism pushed a commit to modelorganism/swift that referenced this pull request Oct 11, 2018
Textual module interfaces don't actually depend on SILGen, so we
shouldn't need to run SILGen (or serialize an entire binary module) if
we're just trying to emit a textual interface. On the other hand, if
we /are/ going to run SILGen and then SIL diagnostics, we shouldn't
delay those diagnostics by spending time emitting a textual interface,
or for that matter a TBD file.

Using this, update all the ModuleInterface tests that use
`-emit-module -o /dev/null` to use `-typecheck` instead, except for
those using `-merge-modules`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants