Skip to content

Cross-Import Overlays #29582

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

Conversation

beccadax
Copy link
Contributor

@beccadax beccadax commented Feb 1, 2020

This PR adds cross-import overlays to Swift. Cross-import overlays allow a module to declare that, when it is imported in the same file as a second module, one or more additional modules should be imported as well. It can be used, for instance, to automatically import test helpers when your module is imported along with XCTest.

This implementation intentionally deviates from the pitch thread in certain ways:

  • We use YAML, rather than JSON, to specify the cross-import information. YAML was selected because, unlike JSON, it allows comments. Their file extension is now .swiftoverlay.
  • Rather than storing overlays in .../Foo.swiftmodule/Overlays, we store them in .../Foo.swiftcrossimport. This allows "thin" (i.e. non-platform-specific) Swift modules to declare cross-import overlays.
  • Clang modules can now declare cross-import overlays by placing a Foo.swiftcrossimport directory next to the module.modulemap file that defines them.
  • A mechanism has been added to declare platform-specific (rather, module-target-triple-specific) cross-import overlays.
  • Some edge cases are handled differently from the pitch; for instance, we don't emit a diagnostic if both modules declare the overlay.

Other differences are either unintentional, incomplete, or undecided.

Fixes rdar://problem/59444602.

@beccadax

This comment has been minimized.

@beccadax

This comment has been minimized.

@swift-ci

This comment has been minimized.

@swift-ci

This comment has been minimized.

@beccadax

This comment has been minimized.

@swift-ci

This comment has been minimized.

@swift-ci

This comment has been minimized.

@beccadax beccadax force-pushed the the-most-ambitious-crossover-event-in-history branch 2 times, most recently from b3355ae to 1c53d17 Compare February 12, 2020 03:33
@beccadax beccadax marked this pull request as ready for review February 14, 2020 05:39
@beccadax
Copy link
Contributor Author

beccadax commented Feb 14, 2020

Feature-complete. To do:

  • Rebase
  • Self-review
  • Clean up version history
  • CI testing
  • Peer review

@beccadax beccadax force-pushed the the-most-ambitious-crossover-event-in-history branch from c3c5bdd to a0edb7a Compare February 14, 2020 05:51
@beccadax beccadax changed the title [WIP] Cross-Import Overlays Cross-Import Overlays Feb 14, 2020
@beccadax

This comment has been minimized.

@beccadax

This comment has been minimized.

@swift-ci

This comment has been minimized.

@swift-ci

This comment has been minimized.

@beccadax

This comment has been minimized.

@swift-ci

This comment has been minimized.

@beccadax

This comment has been minimized.

@swift-ci

This comment has been minimized.

@beccadax beccadax force-pushed the the-most-ambitious-crossover-event-in-history branch from c9875b9 to bdd84f5 Compare February 15, 2020 21:02
@beccadax

This comment has been minimized.

@swift-ci

This comment has been minimized.

@swift-ci

This comment has been minimized.

@beccadax

This comment has been minimized.

It’s not often that a PointerUnion makes the code *more* safe and clear, so let’s take it.
@beccadax beccadax force-pushed the the-most-ambitious-crossover-event-in-history branch from 2291bff to 9a09eb7 Compare February 19, 2020 09:03
@beccadax
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 8fae0de76f17ba1525930f33a1b8b814a950a227

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 8fae0de76f17ba1525930f33a1b8b814a950a227

@beccadax
Copy link
Contributor Author

@varungandhi-apple I'm going to merge this, but feel free to post additional comments on the last round of changes.

@beccadax beccadax merged commit d842fa3 into swiftlang:master Feb 19, 2020
@varungandhi-apple
Copy link
Contributor

LGTM 😄

@drodriguez
Copy link
Contributor

Just a note: a couple of tests seem broken on Windows (started in https://ci-external.swift.org/job/oss-swift-windows-x86_64/2907/ and https://ci-external.swift.org/job/oss-swift-windows-x86_64-vs2019/103/). One of the failing tests seems to be simple error message differences in Windows. The other seems to be some path comparison that doesn't normalize paths.

/cc @compnerd

@devincoughlin
Copy link
Contributor

We'll look into this.

@devincoughlin
Copy link
Contributor

I have a PR up at #29962 to unblock windows testing.

@varungandhi-apple
Copy link
Contributor

varungandhi-apple commented Feb 20, 2020

Related PR with the diagnostic fix: [just to get the GitHub cross-linking to show up] #29965

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.

7 participants