Skip to content

Commit

Permalink
Simplify swift_common.compile so that it returns a SwiftInfo that…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
allevato authored and swiple-rules-gardener committed Jan 9, 2023
1 parent 8fe09a6 commit ac4375d
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 28 deletions.
33 changes: 27 additions & 6 deletions swift/internal/compiling.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,14 @@ def compile(
Returns:
A `struct` with the following fields:
* `swift_info`: A `SwiftInfo` provider whose list of direct modules
contains the single Swift module context produced by this function
(identical to the `module_context` field below) and whose transitive
modules represent the transitive non-private dependencies. Rule
implementations that call this function can typically return this
provider directly, except in rare cases like making multiple calls
to `swift_common.compile` that need to be merged.
* `module_context`: A Swift module context (as returned by
`create_swift_module_context`) that contains the Swift (and
potentially C/Objective-C) compilation prerequisites of the compiled
Expand Down Expand Up @@ -326,14 +334,23 @@ def compile(
],
)

# These are the `SwiftInfo` providers that will be merged with the compiled
# module context and returned as the `swift_info` field of this function's
# result. Note that private deps are explicitly not included here, as they
# are not supposed to be propagated.
#
# TODO(allevato): It would potentially clean things up if we included the
# toolchain's implicit dependencies here as well. Do this and make sure it
# doesn't break anything unexpected.
swift_infos_to_propagate = swift_infos + _cross_imported_swift_infos(
swift_toolchain = swift_toolchain,
user_swift_infos = swift_infos + private_swift_infos,
)

all_swift_infos = (
swift_infos +
swift_infos_to_propagate +
private_swift_infos +
swift_toolchain.implicit_deps_providers.swift_infos +
_cross_imported_swift_infos(
swift_toolchain = swift_toolchain,
user_swift_infos = swift_infos + private_swift_infos,
)
swift_toolchain.implicit_deps_providers.swift_infos
)
merged_swift_info = SwiftInfo(swift_infos = all_swift_infos)

Expand Down Expand Up @@ -509,6 +526,10 @@ def compile(
supplemental_outputs = struct(
indexstore_directory = compile_outputs.indexstore_directory,
),
swift_info = SwiftInfo(
modules = [module_context],
swift_infos = swift_infos_to_propagate,
),
)

def precompile_clang_module(
Expand Down
5 changes: 1 addition & 4 deletions swift/internal/swift_protoc_gen_aspect.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -487,10 +487,7 @@ def _swift_protoc_gen_aspect_impl(target, aspect_ctx):
SwiftProtoCompilationInfo(
cc_info = cc_info,
objc_info = objc_info,
swift_info = SwiftInfo(
modules = [module_context],
swift_infos = transitive_swift_infos,
),
swift_info = compile_result.swift_info,
),
]
else:
Expand Down
5 changes: 1 addition & 4 deletions swift/swift_grpc_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -351,10 +351,7 @@ def _swift_grpc_library_impl(ctx):
linking_context = linking_context,
),
deps[0][SwiftProtoInfo],
SwiftInfo(
modules = [module_context],
swift_infos = compile_deps_swift_infos,
),
compile_result.swift_info,
OutputGroupInfo(**output_groups),
]

Expand Down
7 changes: 1 addition & 6 deletions swift/swift_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -258,12 +258,7 @@ def _swift_library_impl(ctx):
extensions = ["swift"],
source_attributes = ["srcs"],
),
SwiftInfo(
modules = [module_context],
# Note that private_deps are explicitly omitted here; they should
# not propagate.
swift_infos = swift_infos,
),
compile_result.swift_info,
OutputGroupInfo(**output_groups),
]

Expand Down
5 changes: 1 addition & 4 deletions swift/swift_module_alias.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,7 @@ def _swift_module_alias_impl(ctx):
compilation_context = module_context.clang.compilation_context,
linking_context = linking_context,
),
SwiftInfo(
modules = [module_context],
swift_infos = swift_infos,
),
compile_result.swift_info,
OutputGroupInfo(**output_groups),
]

Expand Down
5 changes: 1 addition & 4 deletions swift/swift_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -342,10 +342,7 @@ def _swift_test_impl(ctx):
supplemental_outputs.indexstore_directory,
])

swift_infos_including_owner = [SwiftInfo(
modules = [compile_result.module_context],
swift_infos = swift_infos,
)]
swift_infos_including_owner = [compile_result.swift_info]

# If we're going to do test discovery below, extract the symbol graph of
# the module that we just compiled so that we can discover any tests in
Expand Down

1 comment on commit ac4375d

@brentleyjones
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please sign in to comment.