diff --git a/README.md b/README.md index 9b0f2292..055ff52b 100644 --- a/README.md +++ b/README.md @@ -166,6 +166,28 @@ your_aspect = dwyu_aspect_factory(use_implementation_deps = True) Examples for this can be seen at the [implementation_deps test cases](test/aspect/implementation_deps). +## Target mapping + +Sometimes users don't want to follow the DWYU rules for all targets or have to work with external dependencies not following the DWYU principles. +For such cases DWYU allows creating a mapping which for a chosen target makes more headers available as the target actually provides. +In other words, one can combine virtually for the DWYU analysis multiple targets. +Doing so allows using headers from transitive dependencies without DWYU raising an error for select cases. + +Such a mapping is created with the [dwyu_make_cc_info_mapping](src/cc_info_mapping/cc_info_mapping.bzl) rule. +This offers multiple wys of mapping targets: + +1. Explicitly providing a list of targets which are mapped into a single target. +1. Specifying that all direct dependencies of a given target are mapped into the target. +1. Specifying that all transitive dependencies of a given target are mapped into the target. + +Activate this behavior via: + +```starlark +your_aspect = dwyu_aspect_factory(target_mapping = "") +``` + +Examples for this can be seen at the [target_mapping test cases](test/aspect/target_mapping). + ## Applying automatic fixes DWYU offers a tool to automatically fix some detected problems. diff --git a/src/aspect/dwyu.bzl b/src/aspect/dwyu.bzl index 8ab04645..2e7ec135 100644 --- a/src/aspect/dwyu.bzl +++ b/src/aspect/dwyu.bzl @@ -1,5 +1,6 @@ load("@bazel_tools//tools/build_defs/cc:action_names.bzl", "CPP_COMPILE_ACTION_NAME") load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain") +load("@depend_on_what_you_use//src/cc_info_mapping:cc_info_mapping.bzl", "DwyuCcInfoRemappingsInfo") def _get_target_sources(rule): public_files = [] @@ -22,7 +23,7 @@ def _get_relevant_header(target_context, is_target_under_inspection): def _process_target(ctx, target, defines, output_path, is_target_under_inspection, verbose): processing_output = ctx.actions.declare_file(output_path) - cc_context = target[CcInfo].compilation_context + cc_context = target.cc_info.compilation_context header_files = _get_relevant_header( target_context = cc_context, is_target_under_inspection = is_target_under_inspection, @@ -114,6 +115,34 @@ def _gather_defines(ctx, target_compilation_context): return extract_defines_from_compiler_flags(compiler_command_line_flags) +def _exchange_cc_info(deps, mapping): + transformed = [] + mapping_info = mapping[0][DwyuCcInfoRemappingsInfo].mapping + for dep in deps: + if dep.label in mapping_info: + transformed.append(struct(label = dep.label, cc_info = mapping_info[dep.label])) + else: + transformed.append(struct(label = dep.label, cc_info = dep[CcInfo])) + return transformed + +def _preprocess_deps(ctx): + """ + Normally this function does nothing and simply stores dependencies and their CcInfo providers in a specific format. + If the user chooses to use the target mapping feature, we exchange here the CcInf provider for some targets with a + different one. + """ + target_impl_deps = [] + if ctx.attr._target_mapping: + target_deps = _exchange_cc_info(deps = ctx.rule.attr.deps, mapping = ctx.attr._target_mapping) + if hasattr(ctx.rule.attr, "implementation_deps"): + pass + else: + target_deps = [struct(label = dep.label, cc_info = dep[CcInfo]) for dep in ctx.rule.attr.deps] + if hasattr(ctx.rule.attr, "implementation_deps"): + target_impl_deps = [struct(label = dep.label, cc_info = dep[CcInfo]) for dep in ctx.rule.attr.implementation_deps] + + return target_deps, target_impl_deps + def _do_ensure_private_deps(ctx): """ The implementation_deps feature is only meaningful and available for cc_library, where in contrast to cc_binary @@ -158,22 +187,20 @@ def dwyu_aspect_impl(target, ctx): processed_target = _process_target( ctx, - target = target, + target = struct(label = target.label, cc_info = target[CcInfo]), defines = _gather_defines(ctx, target_compilation_context = target[CcInfo].compilation_context), output_path = "{}_processed_target_under_inspection.json".format(target.label.name), is_target_under_inspection = True, verbose = False, ) + # TODO consistently use impl_deps as variable name and only use long name in docs and APIs + target_deps, target_impl_deps = _preprocess_deps(ctx) + # TODO Investigate if we can prevent running this multiple times for the same dep if multiple # target_under_inspection have the same dependency - processed_deps = _process_dependencies(ctx, target = target, deps = ctx.rule.attr.deps, verbose = False) - processed_implementation_deps = _process_dependencies( - ctx, - target = target, - deps = ctx.rule.attr.implementation_deps if hasattr(ctx.rule.attr, "implementation_deps") else [], - verbose = False, - ) + processed_deps = _process_dependencies(ctx, target = target, deps = target_deps, verbose = False) + processed_impl_deps = _process_dependencies(ctx, target = target, deps = target_impl_deps, verbose = False) report_file = ctx.actions.declare_file("{}_dwyu_report.json".format(target.label.name)) args = ctx.actions.args() diff --git a/src/aspect/factory.bzl b/src/aspect/factory.bzl index 9d16ef77..8c41e11e 100644 --- a/src/aspect/factory.bzl +++ b/src/aspect/factory.bzl @@ -1,8 +1,10 @@ +load("@depend_on_what_you_use//src/cc_info_mapping:cc_info_mapping.bzl", "DwyuCcInfoRemappingsInfo") load(":dwyu.bzl", "dwyu_aspect_impl") def dwyu_aspect_factory( config = None, recursive = False, + target_mapping = None, use_implementation_deps = False): """ Create a "Depend on What You Use" (DWYU) aspect. @@ -14,14 +16,17 @@ def dwyu_aspect_factory( config: Configuration file for the tool comparing the include statements to the dependencies. recursive: If true, execute the aspect on all transitive dependencies. If false, analyze only the target the aspect is being executed on. + target_mapping: A target providing a map of target labels to alternative CcInfo provider objects for those + targets. Typically created with the dwyu_make_cc_info_mapping rule. use_implementation_deps: If true, ensure cc_library dependencies which are used only in private files are - listed in implementation_deps. Only available for Bazel >= 5.0.0 and if flag + listed in implementation_deps. Only available if flag '--experimental_cc_implementation_deps' is provided. Returns: Configured DWYU aspect """ attr_aspects = ["deps"] if recursive else [] aspect_config = [config] if config else [] + aspect_target_mapping = [target_mapping] if target_mapping else [] return aspect( implementation = dwyu_aspect_impl, attr_aspects = attr_aspects, @@ -53,6 +58,10 @@ def dwyu_aspect_factory( "_recursive": attr.bool( default = recursive, ), + "_target_mapping": attr.label_list( + providers = [DwyuCcInfoRemappingsInfo], + default = aspect_target_mapping, + ), "_use_implementation_deps": attr.bool( default = use_implementation_deps, ), diff --git a/src/cc_info_mapping/BUILD b/src/cc_info_mapping/BUILD new file mode 100644 index 00000000..e69de29b diff --git a/src/cc_info_mapping/cc_info_mapping.bzl b/src/cc_info_mapping/cc_info_mapping.bzl new file mode 100644 index 00000000..f54550d5 --- /dev/null +++ b/src/cc_info_mapping/cc_info_mapping.bzl @@ -0,0 +1,70 @@ +load("@depend_on_what_you_use//src/cc_info_mapping/private:direct_deps.bzl", "mapping_to_direct_deps") +load("@depend_on_what_you_use//src/cc_info_mapping/private:explicit.bzl", "explicit_mapping") +load("@depend_on_what_you_use//src/cc_info_mapping/private:providers.bzl", "DwyuCcInfoRemapInfo") +load("@depend_on_what_you_use//src/cc_info_mapping/private:transitive_deps.bzl", "mapping_to_transitive_deps") +load("@depend_on_what_you_use//src/utils:utils.bzl", "label_to_name") + +MAP_DIRECT_DEPS = "__DWYU_MAP_DIRECT_DEPS__" +MAP_TRANSITIVE_DEPS = "__DWYU_MAP_TRANSITIVE_DEPS__" + +DwyuCcInfoRemappingsInfo = provider( + "Dictionary of targets labels wnd which CcInfo provider DWYU should use for analysing them.", + fields = { + "mapping": "Dictionary with structure {'target label': CcInfo provider which should be used by DWYU}.", + }, +) + +def _make_remapping_info_impl(ctx): + return DwyuCcInfoRemappingsInfo(mapping = { + remap[DwyuCcInfoRemapInfo].target: remap[DwyuCcInfoRemapInfo].cc_info + for remap in ctx.attr.remappings + }) + +_make_remapping_info = rule( + implementation = _make_remapping_info_impl, + provides = [DwyuCcInfoRemappingsInfo], + attrs = { + "remappings": attr.label_list(providers = [DwyuCcInfoRemapInfo]), + }, +) + +def dwyu_make_cc_info_mapping(name, mapping): + """ + Create a mapping which allows treating targets as if they themselves would offer header files, which in fact are + coming from their dependencies. This enables the DWYU analysis to skip over some usage of headers provided by + transitive dependencies without raising an error. + + Args: + name: Unique name for this target. Will be the prefix for all private intermediate targets. + mapping: Dictionary containing various targets and how they should be mapped. Possible mappings are: + - An explicit list of targets which are mapped to the main target. Be careful only to choose targets + which are dependencies of the main target! + - The MAP_DIRECT_DEPS token which tells the rule to map all direct dependencies to the main target. + - The MAP_TRANSITIVE_DEPS token which tells the rule to map recursively all transitive dependencies to + the main target. + """ + mappings = [] + for target, map_to in mapping.items(): + mapping_action = "{}_mapping_{}".format(name, label_to_name(target)) + if map_to == MAP_DIRECT_DEPS: + mapping_to_direct_deps( + name = mapping_action, + target = target, + ) + elif map_to == MAP_TRANSITIVE_DEPS: + mapping_to_transitive_deps( + name = mapping_action, + target = target, + ) + else: + explicit_mapping( + name = mapping_action, + target = target, + map_to = map_to, + ) + mappings.append(mapping_action) + + _make_remapping_info( + name = name, + remappings = mappings, + ) diff --git a/src/cc_info_mapping/private/BUILD b/src/cc_info_mapping/private/BUILD new file mode 100644 index 00000000..e69de29b diff --git a/src/cc_info_mapping/private/direct_deps.bzl b/src/cc_info_mapping/private/direct_deps.bzl new file mode 100644 index 00000000..60bbfff6 --- /dev/null +++ b/src/cc_info_mapping/private/direct_deps.bzl @@ -0,0 +1,34 @@ +load(":providers.bzl", "DwyuCcInfoRemapInfo") + +def _aggregate_direct_deps_aspect_impl(target, ctx): + aggregated_compilation_context = cc_common.merge_compilation_contexts( + compilation_contexts = + [tgt[CcInfo].compilation_context for tgt in [target] + ctx.rule.attr.deps], + ) + + return DwyuCcInfoRemapInfo(target = target.label, cc_info = CcInfo( + compilation_context = aggregated_compilation_context, + linking_context = target[CcInfo].linking_context, + )) + +_aggregate_direct_deps_aspect = aspect( + implementation = _aggregate_direct_deps_aspect_impl, + provides = [DwyuCcInfoRemapInfo], + attr_aspects = [], +) + +def _mapping_to_direct_deps_impl(ctx): + return ctx.attr.target[DwyuCcInfoRemapInfo] + +mapping_to_direct_deps = rule( + implementation = _mapping_to_direct_deps_impl, + provides = [DwyuCcInfoRemapInfo], + attrs = { + "target": attr.label(aspects = [_aggregate_direct_deps_aspect], providers = [CcInfo]), + }, + doc = """ +Make headers from all direct dependencies available as if they where provided by the main target itself. +We do so by merging the compilation_context information from the direct dependencies into the main target's CcInfo. +We explicitly ignore implementation_deps, as allowing to map them would break their design of not being visible to users of the target. + """, +) diff --git a/src/cc_info_mapping/private/explicit.bzl b/src/cc_info_mapping/private/explicit.bzl new file mode 100644 index 00000000..64cf92c1 --- /dev/null +++ b/src/cc_info_mapping/private/explicit.bzl @@ -0,0 +1,25 @@ +load(":providers.bzl", "DwyuCcInfoRemapInfo") + +def _explicit_mapping_impl(ctx): + aggregated_compilation_context = cc_common.merge_compilation_contexts( + compilation_contexts = + [tgt[CcInfo].compilation_context for tgt in [ctx.attr.target] + ctx.attr.map_to], + ) + + return DwyuCcInfoRemapInfo(target = ctx.attr.target.label, cc_info = CcInfo( + compilation_context = aggregated_compilation_context, + linking_context = ctx.attr.target[CcInfo].linking_context, + )) + +explicit_mapping = rule( + implementation = _explicit_mapping_impl, + provides = [DwyuCcInfoRemapInfo], + attrs = { + "map_to": attr.label_list(providers = [CcInfo]), + "target": attr.label(providers = [CcInfo]), + }, + doc = """ +Make headers from all explicitly listed targets available as if they where provided by the main target itself. +We do so by merging the compilation_context information from the listed targets into the main target's CcInfo. + """, +) diff --git a/src/cc_info_mapping/private/providers.bzl b/src/cc_info_mapping/private/providers.bzl new file mode 100644 index 00000000..147f391d --- /dev/null +++ b/src/cc_info_mapping/private/providers.bzl @@ -0,0 +1,7 @@ +DwyuCcInfoRemapInfo = provider( + "An alternative CcInfo object for a target which can be used by DWYU during the analysis.", + fields = { + "cc_info": "CcInfo provider.", + "target": "Label of target which should use the cc_info object.", + }, +) diff --git a/src/cc_info_mapping/private/transitive_deps.bzl b/src/cc_info_mapping/private/transitive_deps.bzl new file mode 100644 index 00000000..d3b48b0b --- /dev/null +++ b/src/cc_info_mapping/private/transitive_deps.bzl @@ -0,0 +1,35 @@ +load(":providers.bzl", "DwyuCcInfoRemapInfo") + +def _aggregate_transitive_deps_aspect_impl(target, ctx): + all_cc_info = [target[CcInfo]] + all_cc_info.extend([dep[DwyuCcInfoRemapInfo].cc_info for dep in ctx.rule.attr.deps]) + aggregated_compilation_context = cc_common.merge_compilation_contexts( + compilation_contexts = [cci.compilation_context for cci in all_cc_info], + ) + + return DwyuCcInfoRemapInfo(target = target.label, cc_info = CcInfo( + compilation_context = aggregated_compilation_context, + linking_context = target[CcInfo].linking_context, + )) + +_aggregate_transitive_deps_aspect = aspect( + implementation = _aggregate_transitive_deps_aspect_impl, + provides = [DwyuCcInfoRemapInfo], + attr_aspects = ["deps"], +) + +def _mapping_to_transitive_deps_impl(ctx): + return ctx.attr.target[DwyuCcInfoRemapInfo] + +mapping_to_transitive_deps = rule( + implementation = _mapping_to_transitive_deps_impl, + provides = [DwyuCcInfoRemapInfo], + attrs = { + "target": attr.label(aspects = [_aggregate_transitive_deps_aspect], providers = [CcInfo]), + }, + doc = """ +Make headers from all transitive dependencies available as if they where provided by the main target itself. +We do so by recursively merging the compilation_context information from the dependencies into the main target's CcInfo. +We explicitly ignore implementation_deps, as allowing to map them would break their design of not being visible to users of the target. + """, +) diff --git a/test/aspect/execute_tests.py b/test/aspect/execute_tests.py index 2fc86270..def99a49 100755 --- a/test/aspect/execute_tests.py +++ b/test/aspect/execute_tests.py @@ -294,6 +294,94 @@ ], ), ), + TestCase( + name="target_mapping_b_fail_without_mapping", + cmd=TestCmd( + target="//test/aspect/target_mapping:use_lib_b", + aspect=DEFAULT_ASPECT, + ), + expected=ExpectedResult( + success=False, + invalid_includes=[ + "File='test/aspect/target_mapping/use_lib_b.cpp', include='test/aspect/target_mapping/libs/b.h'" + ], + unused_public_deps=["//test/aspect/target_mapping/libs:a"], + ), + ), + TestCase( + name="target_mapping_b_succeed_with_specific_mapping", + cmd=TestCmd( + target="//test/aspect/target_mapping:use_lib_b", + aspect="//test/aspect/target_mapping:aspect.bzl%map_specific_deps", + ), + expected=ExpectedResult(success=True), + ), + TestCase( + name="target_mapping_b_succeed_with_direct_deps_mapping", + cmd=TestCmd( + target="//test/aspect/target_mapping:use_lib_b", + aspect="//test/aspect/target_mapping:aspect.bzl%map_direct_deps", + ), + expected=ExpectedResult(success=True), + ), + TestCase( + name="target_mapping_b_succeed_with_transitive_deps_mapping", + cmd=TestCmd( + target="//test/aspect/target_mapping:use_lib_b", + aspect="//test/aspect/target_mapping:aspect.bzl%map_transitive_deps", + ), + expected=ExpectedResult(success=True), + ), + TestCase( + name="target_mapping_c_fail_without_mapping", + cmd=TestCmd( + target="//test/aspect/target_mapping:use_lib_c", + aspect=DEFAULT_ASPECT, + ), + expected=ExpectedResult( + success=False, + invalid_includes=[ + "File='test/aspect/target_mapping/use_lib_c.cpp', include='test/aspect/target_mapping/libs/c.h'" + ], + unused_public_deps=["//test/aspect/target_mapping/libs:a"], + ), + ), + TestCase( + name="target_mapping_c_fail_with_specific_mapping", + cmd=TestCmd( + target="//test/aspect/target_mapping:use_lib_c", + aspect="//test/aspect/target_mapping:aspect.bzl%map_specific_deps", + ), + expected=ExpectedResult( + success=False, + invalid_includes=[ + "File='test/aspect/target_mapping/use_lib_c.cpp', include='test/aspect/target_mapping/libs/c.h'" + ], + unused_public_deps=["//test/aspect/target_mapping/libs:a"], + ), + ), + TestCase( + name="target_mapping_c_fail_with_direct_mapping", + cmd=TestCmd( + target="//test/aspect/target_mapping:use_lib_c", + aspect="//test/aspect/target_mapping:aspect.bzl%map_direct_deps", + ), + expected=ExpectedResult( + success=False, + invalid_includes=[ + "File='test/aspect/target_mapping/use_lib_c.cpp', include='test/aspect/target_mapping/libs/c.h'" + ], + unused_public_deps=["//test/aspect/target_mapping/libs:a"], + ), + ), + TestCase( + name="target_mapping_c_succeed_with_transitive_mapping", + cmd=TestCmd( + target="//test/aspect/target_mapping:use_lib_c", + aspect="//test/aspect/target_mapping:aspect.bzl%map_transitive_deps", + ), + expected=ExpectedResult(success=True), + ), ] if __name__ == "__main__": diff --git a/test/aspect/target_mapping/BUILD b/test/aspect/target_mapping/BUILD new file mode 100644 index 00000000..c465c68a --- /dev/null +++ b/test/aspect/target_mapping/BUILD @@ -0,0 +1,11 @@ +cc_binary( + name = "use_lib_b", + srcs = ["use_lib_b.cpp"], + deps = ["//test/aspect/target_mapping/libs:a"], +) + +cc_binary( + name = "use_lib_c", + srcs = ["use_lib_c.cpp"], + deps = ["//test/aspect/target_mapping/libs:a"], +) diff --git a/test/aspect/target_mapping/README.md b/test/aspect/target_mapping/README.md new file mode 100644 index 00000000..e9eb2fea --- /dev/null +++ b/test/aspect/target_mapping/README.md @@ -0,0 +1,5 @@ +In some cases a user does not want to follow the DWYU design guidelines. +Maybe a public targets acts as proxy for implementation detail targets which are however providing the header files. +Or an external dependency is used which is assumed to be used in a specific way. + +To prevent DWYU from raising errors in such cases, we allow mapping the headers provided by the dependencies of a target to the target itself. diff --git a/test/aspect/target_mapping/aspect.bzl b/test/aspect/target_mapping/aspect.bzl new file mode 100644 index 00000000..66771ca6 --- /dev/null +++ b/test/aspect/target_mapping/aspect.bzl @@ -0,0 +1,5 @@ +load("//:defs.bzl", "dwyu_aspect_factory") + +map_specific_deps = dwyu_aspect_factory(target_mapping = "//test/aspect/target_mapping/mapping:map_specific_deps") +map_direct_deps = dwyu_aspect_factory(target_mapping = "//test/aspect/target_mapping/mapping:map_direct_deps") +map_transitive_deps = dwyu_aspect_factory(target_mapping = "//test/aspect/target_mapping/mapping:map_transitive_deps") diff --git a/test/aspect/target_mapping/libs/BUILD b/test/aspect/target_mapping/libs/BUILD new file mode 100644 index 00000000..7a3f48a4 --- /dev/null +++ b/test/aspect/target_mapping/libs/BUILD @@ -0,0 +1,18 @@ +cc_library( + name = "a", + hdrs = ["a.h"], + visibility = ["//test/aspect/target_mapping:__subpackages__"], + deps = [":b"], +) + +cc_library( + name = "b", + hdrs = ["b.h"], + visibility = ["//test/aspect/target_mapping/mapping:__pkg__"], + deps = [":c"], +) + +cc_library( + name = "c", + hdrs = ["c.h"], +) diff --git a/test/aspect/target_mapping/libs/a.h b/test/aspect/target_mapping/libs/a.h new file mode 100644 index 00000000..f028707a --- /dev/null +++ b/test/aspect/target_mapping/libs/a.h @@ -0,0 +1,6 @@ +#include "test/aspect/target_mapping/libs/b.h" + +int doA() +{ + return 42 + doB(); +} diff --git a/test/aspect/target_mapping/libs/b.h b/test/aspect/target_mapping/libs/b.h new file mode 100644 index 00000000..70b2ef90 --- /dev/null +++ b/test/aspect/target_mapping/libs/b.h @@ -0,0 +1,6 @@ +#include "test/aspect/target_mapping/libs/c.h" + +int doB() +{ + return 42 + doC(); +} diff --git a/test/aspect/target_mapping/libs/c.h b/test/aspect/target_mapping/libs/c.h new file mode 100644 index 00000000..7c392c19 --- /dev/null +++ b/test/aspect/target_mapping/libs/c.h @@ -0,0 +1,4 @@ +int doC() +{ + return 42; +} diff --git a/test/aspect/target_mapping/mapping/BUILD b/test/aspect/target_mapping/mapping/BUILD new file mode 100644 index 00000000..615eac14 --- /dev/null +++ b/test/aspect/target_mapping/mapping/BUILD @@ -0,0 +1,24 @@ +load("//src/cc_info_mapping:cc_info_mapping.bzl", "MAP_DIRECT_DEPS", "MAP_TRANSITIVE_DEPS", "dwyu_make_cc_info_mapping") + +package(default_visibility = ["//test/aspect/target_mapping:__pkg__"]) + +dwyu_make_cc_info_mapping( + name = "map_specific_deps", + mapping = { + "//test/aspect/target_mapping/libs:a": ["//test/aspect/target_mapping/libs:b"], + }, +) + +dwyu_make_cc_info_mapping( + name = "map_direct_deps", + mapping = { + "//test/aspect/target_mapping/libs:a": MAP_DIRECT_DEPS, + }, +) + +dwyu_make_cc_info_mapping( + name = "map_transitive_deps", + mapping = { + "//test/aspect/target_mapping/libs:a": MAP_TRANSITIVE_DEPS, + }, +) diff --git a/test/aspect/target_mapping/use_lib_b.cpp b/test/aspect/target_mapping/use_lib_b.cpp new file mode 100644 index 00000000..62e6ac76 --- /dev/null +++ b/test/aspect/target_mapping/use_lib_b.cpp @@ -0,0 +1,6 @@ +#include "test/aspect/target_mapping/libs/b.h" + +int main() +{ + return doB(); +} diff --git a/test/aspect/target_mapping/use_lib_c.cpp b/test/aspect/target_mapping/use_lib_c.cpp new file mode 100644 index 00000000..bc156616 --- /dev/null +++ b/test/aspect/target_mapping/use_lib_c.cpp @@ -0,0 +1,6 @@ +#include "test/aspect/target_mapping/libs/c.h" + +int main() +{ + return doC(); +}