Simplify swift_common.compile so that it returns a SwiftInfo that can be propagated by callers
#1370
+43
−27
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… can be propagated by callers The impetus for this change was the fact that inferred cross-import overlay providers weren't being merged into the propagated `SwiftInfo`, so compiling a target that directly depended on one would succeed but compiling something depending on that other target would fail. Since provider merging was being done by the rules, not by the `compile` call that collected the providers, we would have had to offer a separate API for rule implementations to collect those same cross-import providers or have `compile` return them in some separate fashion. However, all current callers of `swift_common.compile` are doing exactly the same thing: creating a new `SwiftInfo` that merges the returned module context with the deps `SwiftInfo`s that it already collected and passed to `compile`. So, it's straightforward to just return that `SwiftInfo`, into which we can merge the cross-import providers. This ensures we don't drop anything on the floor *and* it's a major API improvement. PiperOrigin-RevId: 500710468 (cherry picked from commit ac4375d) Signed-off-by: Brentley Jones <github@brentleyjones.com>
brentleyjones
requested review from
BalestraPatrick,
keith,
luispadron,
mattrobmattrob and
thii
as code owners
brentleyjones
referenced
this pull request
Oct 13, 2024
… can be propagated by callers. The impetus for this change was the fact that inferred cross-import overlay providers weren't being merged into the propagated `SwiftInfo`, so compiling a target that directly depended on one would succeed but compiling something depending on that other target would fail. Since provider merging was being done by the rules, not by the `compile` call that collected the providers, we would have had to offer a separate API for rule implementations to collect those same cross-import providers or have `compile` return them in some separate fashion. However, all current callers of `swift_common.compile` are doing exactly the same thing: creating a new `SwiftInfo` that merges the returned module context with the deps `SwiftInfo`s that it already collected and passed to `compile`. So, it's straightforward to just return that `SwiftInfo`, into which we can merge the cross-import providers. This ensures we don't drop anything on the floor *and* it's a major API improvement. PiperOrigin-RevId: 500710468
thii
approved these changes
Oct 13, 2024
brentleyjones
deleted the
bj/simplify-swift_common.compile-so-that-it-returns-a-swiftinfo-that-can-be-propagated-by-callers
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The impetus for this change was the fact that inferred cross-import overlay providers weren't being merged into the propagated
SwiftInfo, so compiling a target that directly depended on one would succeed but compiling something depending on that other target would fail.Since provider merging was being done by the rules, not by the
compilecall that collected the providers, we would have had to offer a separate API for rule implementations to collect those same cross-import providers or havecompilereturn them in some separate fashion.However, all current callers of
swift_common.compileare doing exactly the same thing: creating a newSwiftInfothat merges the returned module context with the depsSwiftInfos that it already collected and passed tocompile. So, it's straightforward to just return thatSwiftInfo, into which we can merge the cross-import providers. This ensures we don't drop anything on the floor and it's a major API improvement.PiperOrigin-RevId: 500710468
(cherry picked from commit ac4375d)