Skip to content

Commit

Permalink
Copy strict_includes into SwiftInfo provider's Clang module descr…
Browse files Browse the repository at this point in the history 
…iptor in `swift_clang_module_aspect`.

This makes it so that the `apple_common.Objc` provider no longer needs to be handled separately for compilation by Swift build APIs. (It is still used for linking until that is migrated entirely onto `CcInfo`.)

PiperOrigin-RevId: 423822059
  • Loading branch information
@allevato @swiple-rules-gardener
allevato authored and swiple-rules-gardener committed Jan 24, 2022
1 parent 34e7ea5 commit 8ce9595
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 140 deletions.
160 changes: 61 additions & 99 deletions swift/internal/compiling.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -818,17 +818,28 @@ def _c_layering_check_configurator(prerequisites, args):
args.add("-Xcc", "-fmodules-strict-decluse")
return None

def _clang_module_strict_includes(module_context):
"""Returns the strict Clang include paths for a module context."""
if not module_context.clang:
return None
strict_includes = module_context.clang.strict_includes
if not strict_includes:
return None
return strict_includes.to_list()

def _clang_search_paths_configurator(prerequisites, args):
"""Adds Clang search paths to the command line."""
args.add_all(
depset(transitive = [
prerequisites.cc_compilation_context.includes,
# TODO(b/146575101): Replace with `objc_info.include` once this bug
# is fixed. See `_merge_target_providers` below for more details.
prerequisites.objc_include_paths_workaround,
]),
prerequisites.cc_compilation_context.includes,
before_each = "-Xcc",
format_each = "-I%s",
)
args.add_all(
prerequisites.transitive_modules,
before_each = "-Xcc",
format_each = "-I%s",
map_each = _clang_module_strict_includes,
uniquify = True,
)

# Add Clang search paths for the workspace root and Bazel output roots. The
Expand Down Expand Up @@ -876,7 +887,6 @@ def _collect_clang_module_inputs(
cc_compilation_context,
is_swift,
modules,
objc_info,
prefer_precompiled_modules):
"""Collects Clang module-related inputs to pass to an action.
Expand All @@ -890,8 +900,6 @@ def _collect_clang_module_inputs(
`swift_common.create_module`). The precompiled Clang modules or the
textual module maps and headers of these modules (depending on the
value of `prefer_precompiled_modules`) will be collected as inputs.
objc_info: The `apple_common.Objc` provider of the target being
compiled.
prefer_precompiled_modules: If True, precompiled module artifacts should
be preferred over textual module map files and headers for modules
that have them. If False, textual module map files and headers
Expand All @@ -905,37 +913,22 @@ def _collect_clang_module_inputs(
direct_inputs = []
transitive_inputs = []

if cc_compilation_context:
# The headers stored in the compilation context differ depending on the
# kind of action we're invoking:
if (is_swift and not prefer_precompiled_modules) or not is_swift:
# If this is a `SwiftCompile` with explicit modules disabled, the
# `headers` field is an already-computed set of the transitive
# headers of all the deps. (For an explicit module build, we skip it
# and will more selectively pick subsets for any individual modules
# that need to fallback to implicit modules in the loop below).
#
# If this is a `SwiftPrecompileCModule`, then by definition we're
# only here in a build with explicit modules enabled. We should only
# need the direct headers of the module being compiled and its
# direct dependencies (the latter because Clang needs them present
# on the file system to map them to the module that contains them.)
# However, we may also need some of the transitive headers, if the
# module has dependencies that aren't recognized as modules (e.g.,
# `cc_library` targets without an aspect hint) and the module's
# headers include those. This will likely over-estimate the needed
# inputs, but we can't do better without include scanning in
# Starlark.
transitive_inputs.append(cc_compilation_context.headers)
transitive_inputs.append(depset(cc_compilation_context.direct_textual_headers))

# Some rules still use the `umbrella_header` field to propagate a header
# that they don't also include in `cc_compilation_context.headers`, so we
# also need to pull these in for the time being.
# TODO(b/142867898): This can be removed once the Swift rules start
# generating its own module map for these targets.
if objc_info:
transitive_inputs.append(objc_info.umbrella_header)
if cc_compilation_context and not is_swift:
# This is a `SwiftPrecompileCModule` action, so by definition we're
# only here in a build with explicit modules enabled. We should only
# need the direct headers of the module being compiled and its
# direct dependencies (the latter because Clang needs them present
# on the file system to map them to the module that contains them.)
# However, we may also need some of the transitive headers, if the
# module has dependencies that aren't recognized as modules (e.g.,
# `cc_library` targets without an aspect hint) and the module's
# headers include those. This will likely over-estimate the needed
# inputs, but we can't do better without include scanning in
# Starlark.
transitive_inputs.append(cc_compilation_context.headers)
transitive_inputs.append(
depset(cc_compilation_context.direct_textual_headers),
)

for module in modules:
clang_module = module.clang
Expand All @@ -946,22 +939,24 @@ def _collect_clang_module_inputs(
if not module.is_system and type(module_map) == "File":
direct_inputs.append(module_map)

if prefer_precompiled_modules:
precompiled_module = clang_module.precompiled_module
if precompiled_module:
# For builds preferring explicit modules, use it if we have it
# and don't include any headers as inputs.
direct_inputs.append(precompiled_module)
else:
# If we don't have an explicit module, we need the transitive
# headers from the compilation context associated with the
# module. This will likely overestimate the headers that will
# actually be used in the action, but until we can use include
# scanning from Starlark, we can't compute a more precise input
# set.
transitive_inputs.append(
clang_module.compilation_context.headers,
)
precompiled_module = clang_module.precompiled_module

if prefer_precompiled_modules and precompiled_module:
# For builds preferring explicit modules, use it if we have it
# and don't include any headers as inputs.
direct_inputs.append(precompiled_module)
else:
# If we don't have an explicit module (or we're not using it), we
# need the transitive headers from the compilation context
# associated with the module. This will likely overestimate the
# headers that will actually be used in the action, but until we can
# use include scanning from Starlark, we can't compute a more
# precise input set.
compilation_context = clang_module.compilation_context
transitive_inputs.append(compilation_context.headers)
transitive_inputs.append(
depset(compilation_context.direct_textual_headers),
)

return swift_toolchain_config.config_result(
inputs = direct_inputs,
Expand Down Expand Up @@ -1046,7 +1041,6 @@ def _dependencies_clang_modulemaps_configurator(prerequisites, args):
cc_compilation_context = prerequisites.cc_compilation_context,
is_swift = prerequisites.is_swift,
modules = modules,
objc_info = prerequisites.objc_info,
prefer_precompiled_modules = False,
)

Expand All @@ -1072,7 +1066,6 @@ def _dependencies_clang_modules_configurator(prerequisites, args):
cc_compilation_context = prerequisites.cc_compilation_context,
is_swift = prerequisites.is_swift,
modules = modules,
objc_info = prerequisites.objc_info,
prefer_precompiled_modules = True,
)

Expand Down Expand Up @@ -1335,8 +1328,7 @@ def compile(
deps: Non-private dependencies of the target being compiled. These
targets are used as dependencies of both the Swift module being
compiled and the Clang module for the generated header. These
targets must propagate one of the following providers: `CcInfo`,
`SwiftInfo`, or `apple_common.Objc`.
targets must propagate `CcInfo` or `SwiftInfo`.
feature_configuration: A feature configuration obtained from
`swift_common.configure_features`.
generated_header_name: The name of the Objective-C generated header that
Expand All @@ -1348,8 +1340,7 @@ def compile(
private_deps: Private (implementation-only) dependencies of the target
being compiled. These are only used as dependencies of the Swift
module, not of the Clang module for the generated header. These
targets must propagate one of the following providers: `CcInfo`,
`SwiftInfo`, or `apple_common.Objc`.
targets must propagate `CcInfo` or `SwiftInfo`.
srcs: The Swift source files to compile.
swift_toolchain: The `SwiftToolchainInfo` provider of the toolchain.
target_name: The name of the target for which the code is being
Expand Down Expand Up @@ -1403,9 +1394,9 @@ def compile(
]) + compile_outputs.object_files + other_outputs

# Merge the providers from our dependencies so that we have one each for
# `SwiftInfo`, `CcInfo`, and `apple_common.Objc`. Then we can pass these
# into the action prerequisites so that configurators have easy access to
# the full set of values and inputs through a single accessor.
# `SwiftInfo` and `CcInfo`. Then we can pass these into the action
# prerequisites so that configurators have easy access to the full set of
# values and inputs through a single accessor.
merged_providers = _merge_targets_providers(
implicit_deps_providers = swift_toolchain.implicit_deps_providers,
targets = deps + private_deps,
Expand Down Expand Up @@ -1462,10 +1453,6 @@ def compile(
genfiles_dir = feature_configuration._genfiles_dir,
is_swift = True,
module_name = module_name,
objc_include_paths_workaround = (
merged_providers.objc_include_paths_workaround
),
objc_info = merged_providers.objc_info,
source_files = srcs,
transitive_modules = transitive_modules,
transitive_swiftmodules = transitive_swiftmodules,
Expand Down Expand Up @@ -1691,8 +1678,6 @@ def _precompile_clang_module(
is_swift = False,
is_swift_generated_header = is_swift_generated_header,
module_name = module_name,
objc_include_paths_workaround = depset(),
objc_info = apple_common.new_objc_provider(),
pcm_file = precompiled_module,
source_files = [module_map_file],
transitive_modules = transitive_modules,
Expand Down Expand Up @@ -1731,8 +1716,7 @@ def _create_cc_compilation_context(
deps: Non-private dependencies of the target being compiled. These
targets are used as dependencies of both the Swift module being
compiled and the Clang module for the generated header. These
targets must propagate one of the following providers: `CcInfo`,
`SwiftInfo`, or `apple_common.Objc`.
targets must propagate `CcInfo` or `SwiftInfo`.
feature_configuration: A feature configuration obtained from
`swift_common.configure_features`.
public_hdrs: Public headers that should be propagated by the new
Expand Down Expand Up @@ -2059,11 +2043,10 @@ def _declare_validated_generated_header(actions, generated_header_name):
def _merge_targets_providers(implicit_deps_providers, targets):
"""Merges the compilation-related providers for the given targets.
This function merges the `CcInfo`, `SwiftInfo`, and `apple_common.Objc`
providers from the given targets into a single provider for each. These
providers are then meant to be passed as prerequisites to compilation
actions so that configurators can populate command lines and inputs based on
their data.
This function merges the `CcInfo` and `SwiftInfo` providers from the given
targets into a single provider for each. These providers are then meant to
be passed as prerequisites to compilation actions so that configurators can
populate command lines and inputs based on their data.
Args:
implicit_deps_providers: The implicit deps providers `struct` from the
Expand All @@ -2074,40 +2057,19 @@ def _merge_targets_providers(implicit_deps_providers, targets):
A `struct` containing the following fields:
* `cc_info`: The merged `CcInfo` provider of the targets.
* `objc_include_paths_workaround`: A `depset` containing the include
paths from the given targets that should be passed to ClangImporter.
This is a workaround for some currently incorrect propagation
behavior that is being removed in the future.
* `objc_info`: The merged `apple_common.Objc` provider of the targets.
* `swift_info`: The merged `SwiftInfo` provider of the targets.
"""
cc_infos = list(implicit_deps_providers.cc_infos)
objc_infos = list(implicit_deps_providers.objc_infos)
swift_infos = list(implicit_deps_providers.swift_infos)

# TODO(b/146575101): This is only being used to preserve the current
# behavior of strict Objective-C include paths being propagated one more
# level than they should be. Once the remaining targets that depend on this
# behavior have been fixed, remove it.
objc_include_paths_workaround_depsets = []

for target in targets:
if CcInfo in target:
cc_infos.append(target[CcInfo])
if SwiftInfo in target:
swift_infos.append(target[SwiftInfo])
if apple_common.Objc in target:
objc_infos.append(target[apple_common.Objc])
objc_include_paths_workaround_depsets.append(
target[apple_common.Objc].strict_include,
)

return struct(
cc_info = cc_common.merge_cc_infos(cc_infos = cc_infos),
objc_include_paths_workaround = depset(
transitive = objc_include_paths_workaround_depsets,
),
objc_info = apple_common.new_objc_provider(providers = objc_infos),
swift_info = create_swift_info(swift_infos = swift_infos),
)

Expand Down
34 changes: 12 additions & 22 deletions swift/internal/providers.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,28 +291,10 @@ def create_clang_module(
*,
compilation_context,
module_map,
precompiled_module = None):
precompiled_module = None,
strict_includes = None):
"""Creates a value representing a Clang module used as a Swift dependency.
Note: The `compilation_context` argument of this function is primarily
intended to communicate information *to* the Swift build rules, not to
retrieve information *back out.* In most cases, it is better to depend on
the `CcInfo` provider propagated by a Swift target to collect transitive
C/Objective-C compilation information about that target. This is because the
context used when compiling the module itself may not be the same as the
context desired when depending on it. (For example, `apple_common.Objc`
supports "strict include paths" which are only propagated to direct
dependents.)
One valid exception to the guidance above is retrieving the generated header
associated with a specific Swift module. Since the `CcInfo` provider
propagated by the library will have already merged them transitively (or,
in the case of a hypothetical custom rule that propagates multiple direct
modules, the `direct_public_headers` of the `CcInfo` would also have them
merged), it is acceptable to read the headers from the compilation context
of the module struct itself in order to associate them with the module that
generated them.
Args:
compilation_context: A `CcCompilationContext` that contains the header
files and other context (such as include paths, preprocessor
Expand All @@ -328,15 +310,23 @@ def create_clang_module(
explicit module was built for the module; in that case, targets that
depend on the module will fall back to the text module map and
headers.
strict_includes: A `depset` of strings representing additional Clang
include paths that should be passed to the compiler when this module
is a _direct_ dependency of the module being compiled. May be
`None`. **This field only exists to support a specific legacy use
case and should otherwise not be used, as it is fundamentally
incompatible with Swift's import model.**
Returns:
A `struct` containing the `compilation_context`, `module_map`, and
`precompiled_module` fields provided as arguments.
A `struct` containing the `compilation_context`, `module_map`,
`precompiled_module`, and `strict_includes` fields provided as
arguments.
"""
return struct(
compilation_context = compilation_context,
module_map = module_map,
precompiled_module = precompiled_module,
strict_includes = strict_includes,
)

def create_swift_module(
Expand Down
Loading

2 comments on commit 8ce9595

@keith
Copy link
Member

@keith keith commented on 8ce9595 Apr 25, 2022

Choose a reason for hiding this comment

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

#691

@brentleyjones
Copy link
Collaborator

Choose a reason for hiding this comment

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

#1191

Please sign in to comment.