diff --git a/README.md b/README.md index 0c675010..3a69ceac 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,7 @@ - [Use DWYU](#use-dwyu) - [Applying automatic fixes](#applying-automatic-fixes) - [Assumptions of use](#assumptions-of-use) +- [Known problems](#known-problems) - [Supported Platforms](#supported-platforms) - [Alternatives to DWYU](#alternatives-to-dwyu) - [Versioning](#versioning) @@ -169,6 +170,10 @@ For example, including header files which do not exist at the expected path. There shall not be multiple header files in the dependency tree of a target matching an include statement. Even if analysing the code works initially, it might break at any time if the ordering of paths in the analysis changes. +# Known problems + +TODO add info about compiler internal defines and especially `__cplusplus` + # Supported Platforms ### Aspect diff --git a/docs/dwyu_aspect.md b/docs/dwyu_aspect.md index 98255404..64ae54fa 100644 --- a/docs/dwyu_aspect.md +++ b/docs/dwyu_aspect.md @@ -9,8 +9,8 @@ 
 load("@depend_on_what_you_use//src/aspect:factory.bzl", "dwyu_aspect_factory")
 
-dwyu_aspect_factory(ignored_includes, recursive, skip_external_targets, skipped_tags,
-                    target_mapping, use_implementation_deps, verbose)
+dwyu_aspect_factory(experimental_set_cplusplus, ignored_includes, recursive, skip_external_targets,
+                    skipped_tags, target_mapping, use_implementation_deps, verbose)
Create a "Depend on What You Use" (DWYU) aspect. @@ -28,7 +28,8 @@ your_dwyu_aspect = dwyu_aspect_factory() | Name | Description | Default Value | | :------------- | :------------- | :------------- | -| ignored_includes | By default, DWYU ignores all headers from the standard library when comparing include statements to the dependencies. This list of headers can be seen in [std_header.py](/src/analyze_includes/std_header.py).
You can extend this list of ignored headers or replace it with a custom one by providing a json file with the information to this attribute.
Specification of possible files in the json file: This feature is demonstrated in the [ignoring_includes example](/examples/ignoring_includes). | `None` | +| experimental_set_cplusplus | **Experimental** feature whose behavior is not yet stable and an change at any time.
`__cplusplus` is a macro defined by the compiler specifying if C++ is used to compile the file and which C++ standard is used. It is comon to use preprocessor conditionals to change what code is used based on this.
DWYU cannot treat this like other preprocessor defines, as this is often not coming from the user or the Bazel C++ toolchain. The compiler itself defines the value for and sets it internally during preprocessing `__cplusplus`.
This option enables a heuristic to set `__cplusplus` for the preprocessor used internally by DWYU: This feature is demonstrated in the [set_cpp_standard example](/examples/set_cpp_standard). | `False` | +| ignored_includes | By default, DWYU ignores all headers from the standard library when comparing include statements to the dependencies. This list of headers can be seen in [std_header.py](/src/analyze_includes/std_header.py).
You can extend this list of ignored headers or replace it with a custom one by providing a json file with the information to this attribute.
Specification of possible files in the json file: This feature is demonstrated in the [ignoring_includes example](/examples/ignoring_includes). | `None` | | recursive | By default, the DWYU aspect analyzes only the target it is being applied to. You can change this to recursively analyzing dependencies following the `deps` and `implementation_deps` attributes by setting this to True.
This feature is demonstrated in the [recursion example](/examples/recursion). | `False` | | skip_external_targets | Sometimes external dependencies are not our under control and thus analyzing them is of little value. If this flag is True, DWYU will automatically skip all targets from external workspaces. This can be useful in combination with the recursive analysis mode.
This feature is demonstrated in the [skipping_targets example](/examples/skipping_targets). | `False` | | skipped_tags | Do not execute the DWYU analysis on targets with at least one of those tags. By default skips the analysis for targets tagged with 'no-dwyu'.
This feature is demonstrated in the [skipping_targets example](/examples/skipping_targets). | `["no-dwyu"]` | diff --git a/examples/aspect.bzl b/examples/aspect.bzl index 342cd4ab..f8ed58fe 100644 --- a/examples/aspect.bzl +++ b/examples/aspect.bzl @@ -5,6 +5,7 @@ dwyu = dwyu_aspect_factory(use_implementation_deps = True) dwyu_recursive = dwyu_aspect_factory(recursive = True) dwyu_recursive_skip_external = dwyu_aspect_factory(recursive = True, skip_external_targets = True) dwyu_custom_skipping = dwyu_aspect_factory(skipped_tags = ["my_tag"]) +dwyu_set_cplusplus = dwyu_aspect_factory(experimental_set_cplusplus = True) # We need to explicitly pass labels as passing strings does not work with a bzlmod setup. dwyu_ignoring_includes = dwyu_aspect_factory(ignored_includes = Label("@//ignoring_includes:ignore_includes.json")) diff --git a/examples/set_cpp_standard/BUILD b/examples/set_cpp_standard/BUILD new file mode 100644 index 00000000..7036e4e1 --- /dev/null +++ b/examples/set_cpp_standard/BUILD @@ -0,0 +1,11 @@ +cc_library( + name = "cpp_lib", + srcs = ["cpp_lib.cpp"], + hdrs = ["cpp_lib.h"], +) + +cc_library( + name = "use_specific_cpp_standard", + hdrs = ["use_specific_cpp_standard.h"], + copts = ["-std=c++17"], +) diff --git a/examples/set_cpp_standard/README.md b/examples/set_cpp_standard/README.md new file mode 100644 index 00000000..6a5011b9 --- /dev/null +++ b/examples/set_cpp_standard/README.md @@ -0,0 +1,14 @@ +**Beware, this is an experimental feature which has not yet a stable behavior!** + +`__cplusplus` is a macro defined by the compiler specifying if C++ is used to compile the file and which C++ standard is used. +`__cplusplus` typically is not passed on the compiler command line as defined value, but set internally by the preprocessor while processing a file. +Thus, DWYU does not know about it when performing the processing to resolve preprocessor statements. +To work around this, DWYU we can set `__cplusplus` based on a heuristic. + +Executing
+`bazel build --aspects=//:aspect.bzl%dwyu_set_cplusplus --output_groups=dwyu //set_cpp_standard:cpp_lib`
+showcases that DWYU can process `__cplusplus` based preprocessor statements with a heuristic. + +Executing
+`bazel build --aspects=//:aspect.bzl%dwyu_set_cplusplus --output_groups=dwyu //set_cpp_standard:use_specific_cpp_standard`
+showcases that checking for a specific C++ standard works as long as the compiler command specifies the desired C++ standard. diff --git a/examples/set_cpp_standard/cpp_lib.cpp b/examples/set_cpp_standard/cpp_lib.cpp new file mode 100644 index 00000000..447181df --- /dev/null +++ b/examples/set_cpp_standard/cpp_lib.cpp @@ -0,0 +1,5 @@ +#include "set_cpp_standard/cpp_lib.h" + +void someFunction() { + // Do something +} diff --git a/examples/set_cpp_standard/cpp_lib.h b/examples/set_cpp_standard/cpp_lib.h new file mode 100644 index 00000000..f05a1812 --- /dev/null +++ b/examples/set_cpp_standard/cpp_lib.h @@ -0,0 +1,7 @@ +#ifndef __cplusplus + +#include "not/existing/dep.h" + +#endif + +void someFunction(); diff --git a/examples/set_cpp_standard/use_specific_cpp_standard.h b/examples/set_cpp_standard/use_specific_cpp_standard.h new file mode 100644 index 00000000..b9fdb6e7 --- /dev/null +++ b/examples/set_cpp_standard/use_specific_cpp_standard.h @@ -0,0 +1,5 @@ +#if __cplusplus != 201703 + +#include "not/existing/dep.h" + +#endif diff --git a/examples/test.py b/examples/test.py index ee212743..5acd2fa3 100755 --- a/examples/test.py +++ b/examples/test.py @@ -47,6 +47,14 @@ class Example: build_cmd="//rule_using_dwyu:dwyu", expected_success=False, ), + Example( + build_cmd="--aspects=//:aspect.bzl%dwyu_set_cplusplus --output_groups=dwyu //set_cpp_standard:cpp_lib", + expected_success=True, + ), + Example( + build_cmd="--aspects=//:aspect.bzl%dwyu_set_cplusplus --output_groups=dwyu //set_cpp_standard:use_specific_cpp_standard", + expected_success=True, + ), Example( build_cmd="--aspects=//:aspect.bzl%dwyu --output_groups=dwyu //skipping_targets:bad_target", expected_success=False, diff --git a/src/aspect/dwyu.bzl b/src/aspect/dwyu.bzl index e5073d28..d2200c2b 100644 --- a/src/aspect/dwyu.bzl +++ b/src/aspect/dwyu.bzl @@ -3,6 +3,31 @@ load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain") load("@depend_on_what_you_use//src/cc_info_mapping:providers.bzl", "DwyuCcInfoRemappingsInfo") load("@rules_cc//cc:defs.bzl", "CcInfo", "cc_common") +# Based on those references: +# https://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html +# https://clang.llvm.org/cxx_status.html +# https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html +# https://learn.microsoft.com/en-us/cpp/build/reference/std-specify-language-standard-version?view=msvc-170 +# +# We ignore the fuzzy variants (e.g. 'c++1X' or 'c++2X') as their values seem to be not constant but depend on the +# compiler version. +_STD_FLAG_TO_STANDARD = { + "c++03": 199711, + "c++11": 201103, + "c++14": 201402, + "c++17": 201703, + "c++20": 202002, + "c++23": 202302, + "c++98": 199711, + "gnu++03": 199711, + "gnu++11": 201103, + "gnu++14": 201402, + "gnu++17": 201703, + "gnu++20": 202002, + "gnu++23": 202302, + "gnu++98": 199711, +} + def _is_external(ctx): return ctx.label.workspace_root.startswith("external") @@ -66,17 +91,6 @@ def _process_dependencies(ctx, target, deps, verbose): ) for dep in deps] def extract_defines_from_compiler_flags(compiler_flags): - """ - We extract the relevant defines from the compiler command line flags. We utilize the compiler flags since the - toolchain can set defines which are not available through CcInfo or the cpp fragments. Furthermore, defines - potentially overwrite or deactivate each other depending on the order in which they appear in the compiler - command. Thus, this is the only way to make sure DWYU analyzes what would actually happen during compilation. - - Args: - compiler_flags: List of flags making up the compilation command - Returns: - List of defines - """ defines = {} for cflag in compiler_flags: @@ -95,7 +109,45 @@ def extract_defines_from_compiler_flags(compiler_flags): return defines.values() -def _gather_defines(ctx, target_compilation_context): +def extract_cpp_standard_from_compiler_flags(compiler_flags): + cpp_standard = None + + for cflag in compiler_flags: + standard_value = None + + # gcc/clang + if cflag.startswith("-std="): + standard_value = cflag.split("=", 1)[1] + + # MSVC + if cflag.startswith("/std:"): + standard_value = cflag.split(":", 1)[1] + + if standard_value: + standard = _STD_FLAG_TO_STANDARD.get(standard_value, None) + if standard: + cpp_standard = standard + elif not cpp_standard: + # We see a standard flag, but don't understands its value. We ensure the C++ standard macro is defined + # Even if we can't define its exact value. + cpp_standard = 1 + + return cpp_standard + +def _is_c_file(file): + """ + Heuristic for finding C files by looking for the established file extensions + """ + return file.extension in ["h", "c"] + +def _gather_defines(ctx, target_compilation_context, target_files): + """ + We extract the relevant defines from the compiler command line flags. We utilize the compiler flags since the + toolchain can set defines which are not available through CcInfo or the cpp fragments. Furthermore, defines + potentially overwrite or deactivate each other depending on the order in which they appear in the compiler + command. Thus, this is the only way to make sure DWYU analyzes what would actually happen during compilation. + """ + cc_toolchain = find_cpp_toolchain(ctx) feature_configuration = cc_common.configure_features( @@ -104,6 +156,7 @@ def _gather_defines(ctx, target_compilation_context): requested_features = ctx.features, unsupported_features = ctx.disabled_features, ) + compile_variables = cc_common.create_compile_variables( feature_configuration = feature_configuration, cc_toolchain = cc_toolchain, @@ -121,7 +174,30 @@ def _gather_defines(ctx, target_compilation_context): variables = compile_variables, ) - return extract_defines_from_compiler_flags(compiler_command_line_flags) + defines = extract_defines_from_compiler_flags(compiler_command_line_flags) + + if ctx.attr._set_cplusplus: + cpp_standard = None + compiler_cpp_standard = extract_cpp_standard_from_compiler_flags(compiler_command_line_flags) + + # If we can extract a c++ standard from the compiler invocation, we use it as we consider this our most reliable + # source of information + if compiler_cpp_standard: + cpp_standard = compiler_cpp_standard + + # If we could not determine the C++ standard based on the compiler arguments, we use a heuristic based on the + # file types. If any file extension other than ['.h', '.c'] is used, we assume C++. + if not cpp_standard: + # We don't know the exact C++ standard, but at least we can enable preprocessor control statements caring + # only about '__cplusplus' being defined at all or not. + if not all([_is_c_file(file) for file in target_files]): + cpp_standard = 1 + + # If we assume this is a C++ compilation, add the corresponding constant to the defines list + if cpp_standard: + defines.append("__cplusplus={}".format(cpp_standard)) + + return defines def _exchange_cc_info(deps, mapping): transformed = [] @@ -214,10 +290,15 @@ def dwyu_aspect_impl(target, ctx): if not public_files and not private_files: return [] + defines = _gather_defines( + ctx, + target_compilation_context = target[CcInfo].compilation_context, + target_files = public_files + private_files, + ) processed_target = _process_target( ctx, target = struct(label = target.label, cc_info = target[CcInfo]), - defines = _gather_defines(ctx, target_compilation_context = target[CcInfo].compilation_context), + defines = defines, output_path = "{}_processed_target_under_inspection.json".format(target.label.name), is_target_under_inspection = True, verbose = ctx.attr._verbose, diff --git a/src/aspect/factory.bzl b/src/aspect/factory.bzl index bd3e0abb..046a0804 100644 --- a/src/aspect/factory.bzl +++ b/src/aspect/factory.bzl @@ -4,6 +4,7 @@ load(":dwyu.bzl", "dwyu_aspect_impl") _DEFAULT_SKIPPED_TAGS = ["no-dwyu"] def dwyu_aspect_factory( + experimental_set_cplusplus = False, ignored_includes = None, recursive = False, skip_external_targets = False, @@ -22,13 +23,28 @@ def dwyu_aspect_factory( ``` Args: + experimental_set_cplusplus: **Experimental** feature whose behavior is not yet stable and an change at any time.
+ `__cplusplus` is a macro defined by the compiler specifying if C++ is used to compile the file and which C++ standard is used. + It is comon to use preprocessor conditionals to change what code is used based on this.
+ DWYU cannot treat this like other preprocessor defines, as this is often not coming from the user or the Bazel C++ toolchain. + The compiler itself defines the value for and sets it internally during preprocessing `__cplusplus`.
+ This option enables a heuristic to set `__cplusplus` for the preprocessor used internally by DWYU: + + This feature is demonstrated in the [set_cpp_standard example](/examples/set_cpp_standard). + ignored_includes: By default, DWYU ignores all headers from the standard library when comparing include statements to the dependencies. This list of headers can be seen in [std_header.py](/src/analyze_includes/std_header.py).
You can extend this list of ignored headers or replace it with a custom one by providing a json file with the information to this attribute.
Specification of possible files in the json file: This feature is demonstrated in the [ignoring_includes example](/examples/ignoring_includes). + recursive: By default, the DWYU aspect analyzes only the target it is being applied to. You can change this to recursively analyzing dependencies following the `deps` and `implementation_deps` attributes by setting this to True.
This feature is demonstrated in the [recursion example](/examples/recursion). + skip_external_targets: Sometimes external dependencies are not our under control and thus analyzing them is of little value. If this flag is True, DWYU will automatically skip all targets from external workspaces. This can be useful in combination with the recursive analysis mode.
This feature is demonstrated in the [skipping_targets example](/examples/skipping_targets). + skipped_tags: Do not execute the DWYU analysis on targets with at least one of those tags. By default skips the analysis for targets tagged with 'no-dwyu'.
This feature is demonstrated in the [skipping_targets example](/examples/skipping_targets). + target_mapping: Accepts a [dwyu_make_cc_info_mapping](/docs/cc_info_mapping.md) target. Allows virtually combining targets regarding which header can be provided by which dependency. For the full details see the `dwyu_make_cc_info_mapping` documentation.
This feature is demonstrated in the [target_mapping example](/examples/target_mapping). + use_implementation_deps: `cc_library` offers the attribute [`implementation_deps`](https://bazel.build/reference/be/c-cpp#cc_library.implementation_deps) to distinguish between public (aka interface) and private (aka implementation) dependencies. Headers from the private dependencies are not made available to users of the library.
Setting this to True allows DWYU to raise an error if headers from a `deps` dependency are used only in private files. In such a cease the dependency should be moved from `deps` to `implementation_deps`.
This feature is demonstrated in the [basic_usage example](/examples/basic_usage). + verbose: If True, print debugging information about what DWYU does. Returns: @@ -102,6 +124,9 @@ def dwyu_aspect_factory( "_recursive": attr.bool( default = recursive, ), + "_set_cplusplus": attr.bool( + default = experimental_set_cplusplus, + ), "_skip_external_targets": attr.bool( default = skip_external_targets, ), diff --git a/src/aspect/test/dwyu_test.bzl b/src/aspect/test/dwyu_test.bzl index a2de10cf..150acad7 100644 --- a/src/aspect/test/dwyu_test.bzl +++ b/src/aspect/test/dwyu_test.bzl @@ -1,5 +1,5 @@ load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest") -load("//src/aspect:dwyu.bzl", "extract_defines_from_compiler_flags") +load("//src/aspect:dwyu.bzl", "extract_cpp_standard_from_compiler_flags", "extract_defines_from_compiler_flags") def _extract_defines_from_compiler_flags_test_impl(ctx): env = unittest.begin(ctx) @@ -38,10 +38,35 @@ def _extract_defines_from_compiler_flags_test_impl(ctx): return unittest.end(env) +def _extract_cpp_standard_from_compiler_flags_test_impl(ctx): + env = unittest.begin(ctx) + + # None if empty list is provided + asserts.equals(env, None, extract_cpp_standard_from_compiler_flags([])) + + # None if nothing can be found + asserts.equals(env, None, extract_cpp_standard_from_compiler_flags(["whatever"])) + + # Unknown standard value yields 1 + asserts.equals(env, 1, extract_cpp_standard_from_compiler_flags(["whatever", "-std=foo"])) + + # Basic case + asserts.equals(env, 202002, extract_cpp_standard_from_compiler_flags(["-std=c++20", "whatever"])) + + # Last definition wins + asserts.equals(env, 199711, extract_cpp_standard_from_compiler_flags(["-std=c++20", "whatever", "-std=c++98"])) + + # MSVC syntax + asserts.equals(env, 202302, extract_cpp_standard_from_compiler_flags(["/std:c++23"])) + + return unittest.end(env) + extract_defines_from_compiler_flags_test = unittest.make(_extract_defines_from_compiler_flags_test_impl) +extract_cpp_standard_from_compiler_flags_test = unittest.make(_extract_cpp_standard_from_compiler_flags_test_impl) def dwyu_aspect_test_suite(name): unittest.suite( name, extract_defines_from_compiler_flags_test, + extract_cpp_standard_from_compiler_flags_test, ) diff --git a/src/utils/utils.bzl b/src/utils/utils.bzl index 9d267f2b..855d8060 100644 --- a/src/utils/utils.bzl +++ b/src/utils/utils.bzl @@ -7,6 +7,10 @@ def label_to_name(label): def print_compilation_context(cc_info, headline = None): """ Print CcInfo's compilation_context in a structured way. + + print debugging is eased by those flags which prevent print statements being omitted on subsequent execution + --nokeep_state_after_build + --notrack_incremental_state """ cc = cc_info.compilation_context headline_str = "\n" + headline if headline else "" diff --git a/test/aspect/set_cpp_standard/BUILD b/test/aspect/set_cpp_standard/BUILD new file mode 100644 index 00000000..7036e4e1 --- /dev/null +++ b/test/aspect/set_cpp_standard/BUILD @@ -0,0 +1,11 @@ +cc_library( + name = "cpp_lib", + srcs = ["cpp_lib.cpp"], + hdrs = ["cpp_lib.h"], +) + +cc_library( + name = "use_specific_cpp_standard", + hdrs = ["use_specific_cpp_standard.h"], + copts = ["-std=c++17"], +) diff --git a/test/aspect/set_cpp_standard/aspect.bzl b/test/aspect/set_cpp_standard/aspect.bzl new file mode 100644 index 00000000..ee61f10f --- /dev/null +++ b/test/aspect/set_cpp_standard/aspect.bzl @@ -0,0 +1,3 @@ +load("@depend_on_what_you_use//:defs.bzl", "dwyu_aspect_factory") + +set_cplusplus = dwyu_aspect_factory(experimental_set_cplusplus = True) diff --git a/test/aspect/set_cpp_standard/cpp_lib.cpp b/test/aspect/set_cpp_standard/cpp_lib.cpp new file mode 100644 index 00000000..447181df --- /dev/null +++ b/test/aspect/set_cpp_standard/cpp_lib.cpp @@ -0,0 +1,5 @@ +#include "set_cpp_standard/cpp_lib.h" + +void someFunction() { + // Do something +} diff --git a/test/aspect/set_cpp_standard/cpp_lib.h b/test/aspect/set_cpp_standard/cpp_lib.h new file mode 100644 index 00000000..4bbf54d0 --- /dev/null +++ b/test/aspect/set_cpp_standard/cpp_lib.h @@ -0,0 +1,8 @@ +#ifndef __cplusplus + +// If '__cplusplus' would not be set, this invalid include would fail in the DWYU analysis +#include "not/existing/dep.h" + +#endif + +void someFunction(); diff --git a/test/aspect/set_cpp_standard/test_set_cplusplus.py b/test/aspect/set_cpp_standard/test_set_cplusplus.py new file mode 100644 index 00000000..10e19ef4 --- /dev/null +++ b/test/aspect/set_cpp_standard/test_set_cplusplus.py @@ -0,0 +1,10 @@ +from result import ExpectedResult, Result +from test_case import TestCaseBase + + +class TestCase(TestCaseBase): + def execute_test_logic(self) -> Result: + expected = ExpectedResult(success=True) + actual = self._run_dwyu(target="//set_cpp_standard:all", aspect="//set_cpp_standard:aspect.bzl%set_cplusplus") + + return self._check_result(actual=actual, expected=expected) diff --git a/test/aspect/set_cpp_standard/use_specific_cpp_standard.h b/test/aspect/set_cpp_standard/use_specific_cpp_standard.h new file mode 100644 index 00000000..e4aa1d85 --- /dev/null +++ b/test/aspect/set_cpp_standard/use_specific_cpp_standard.h @@ -0,0 +1,6 @@ +#if __cplusplus != 201703 + +// If '__cplusplus' would not be set to the expected value, this invalid include would fail in the DWYU analysis +#include "not/existing/dep.h" + +#endif