From fd5054d4153610b5513e8edb7da6ef027f7333e0 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Thu, 6 Jun 2024 17:07:28 +0200 Subject: [PATCH 1/2] Force use of Clang over C++ modules with `use_module_maps` With `-std=c++20`, Clang defaults to using C++ modules rather than Clang modules, which can result in failures when using `layering_check` and thus `use_module_maps`. --- tools/cpp/unix_cc_toolchain_config.bzl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/cpp/unix_cc_toolchain_config.bzl b/tools/cpp/unix_cc_toolchain_config.bzl index 5a0cd710575076..5d03f04069616c 100644 --- a/tools/cpp/unix_cc_toolchain_config.bzl +++ b/tools/cpp/unix_cc_toolchain_config.bzl @@ -54,11 +54,14 @@ def layering_check_features(compiler, is_macos): flag_groups = [ flag_group( # macOS requires -Xclang because of a bug in Apple Clang + # -fno-cxx-modules is necessary to avoid clang defaulting + # to C++ modules with -std=c++20. The flag requires the + # -Xclang prefix even on Linux. flags = (["-Xclang"] if is_macos else []) + [ "-fmodule-name=%{module_name}", ] + (["-Xclang"] if is_macos else []) + [ "-fmodule-map-file=%{module_map_file}", - ], + ] + ["-Xclang", "-fno-cxx-modules"], ), ], ), From fa15dbd097710068f3ef5d040757322e7d5a07b5 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 7 Jun 2024 10:55:44 +0200 Subject: [PATCH 2/2] Add feature detection --- tools/cpp/BUILD.tpl | 1 + tools/cpp/unix_cc_configure.bzl | 17 +++++++++++++++++ tools/cpp/unix_cc_toolchain_config.bzl | 12 +++++------- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/tools/cpp/BUILD.tpl b/tools/cpp/BUILD.tpl index f99995d2b923eb..40e3f7a1ef1577 100644 --- a/tools/cpp/BUILD.tpl +++ b/tools/cpp/BUILD.tpl @@ -113,6 +113,7 @@ cc_toolchain_config( coverage_compile_flags = [%{coverage_compile_flags}], coverage_link_flags = [%{coverage_link_flags}], supports_start_end_lib = %{supports_start_end_lib}, + extra_flags_per_feature = %{extra_flags_per_feature}, ) # Android tooling requires a default toolchain for the armeabi-v7a cpu. diff --git a/tools/cpp/unix_cc_configure.bzl b/tools/cpp/unix_cc_configure.bzl index ef8b8b49fb6fc8..72910ec4d8d57a 100644 --- a/tools/cpp/unix_cc_configure.bzl +++ b/tools/cpp/unix_cc_configure.bzl @@ -541,6 +541,22 @@ def configure_unix_toolchain(repository_ctx, cpu_value, overriden_tools): builtin_include_directories, paths["@bazel_tools//tools/cpp:generate_system_module_map.sh"], )) + extra_flags_per_feature = {} + if is_clang: + # Only supported by LLVM 14 and later, but required with C++20 and + # layering_check as C++ modules are the default. + # https://github.com/llvm/llvm-project/commit/0556138624edf48621dd49a463dbe12e7101f17d + result = repository_ctx.execute([ + cc, + "-Xclang", + "-fno-cxx-modules", + "-o", + "/dev/null", + "-c", + str(repository_ctx.path("tools/cpp/empty.cc")), + ]) + if "-fno-cxx-modules" not in result.stderr: + extra_flags_per_feature["use_module_maps"] = ["-Xclang", "-fno-cxx-modules"] write_builtin_include_directory_paths(repository_ctx, cc, builtin_include_directories) repository_ctx.template( @@ -696,5 +712,6 @@ def configure_unix_toolchain(repository_ctx, cpu_value, overriden_tools): "%{coverage_compile_flags}": coverage_compile_flags, "%{coverage_link_flags}": coverage_link_flags, "%{supports_start_end_lib}": "True" if gold_or_lld_linker_path else "False", + "%{extra_flags_per_feature}": repr(extra_flags_per_feature), }, ) diff --git a/tools/cpp/unix_cc_toolchain_config.bzl b/tools/cpp/unix_cc_toolchain_config.bzl index 5d03f04069616c..61f656116cbdd1 100644 --- a/tools/cpp/unix_cc_toolchain_config.bzl +++ b/tools/cpp/unix_cc_toolchain_config.bzl @@ -36,7 +36,7 @@ def _target_os_version(ctx): xcode_config = ctx.attr._xcode_config[apple_common.XcodeVersionConfig] return xcode_config.minimum_os_for_platform_type(platform_type) -def layering_check_features(compiler, is_macos): +def layering_check_features(compiler, extra_flags_per_feature, is_macos): if compiler != "clang": return [] return [ @@ -54,14 +54,11 @@ def layering_check_features(compiler, is_macos): flag_groups = [ flag_group( # macOS requires -Xclang because of a bug in Apple Clang - # -fno-cxx-modules is necessary to avoid clang defaulting - # to C++ modules with -std=c++20. The flag requires the - # -Xclang prefix even on Linux. flags = (["-Xclang"] if is_macos else []) + [ "-fmodule-name=%{module_name}", ] + (["-Xclang"] if is_macos else []) + [ "-fmodule-map-file=%{module_map_file}", - ] + ["-Xclang", "-fno-cxx-modules"], + ] + extra_flags_per_feature.get("use_module_maps", []), ), ], ), @@ -1492,7 +1489,7 @@ def _impl(ctx): unfiltered_compile_flags_feature, treat_warnings_as_errors_feature, archive_param_file_feature, - ] + layering_check_features(ctx.attr.compiler, is_macos = False) + ] + layering_check_features(ctx.attr.compiler, ctx.attr.extra_flags_per_feature, is_macos = False) else: # macOS artifact name patterns differ from the defaults only for dynamic # libraries. @@ -1533,7 +1530,7 @@ def _impl(ctx): treat_warnings_as_errors_feature, archive_param_file_feature, generate_linkmap_feature, - ] + layering_check_features(ctx.attr.compiler, is_macos = True) + ] + layering_check_features(ctx.attr.compiler, ctx.attr.extra_flags_per_feature, is_macos = True) parse_headers_action_configs, parse_headers_features = parse_headers_support( parse_headers_tool_path = ctx.attr.tool_paths.get("parse_headers"), @@ -1586,6 +1583,7 @@ cc_toolchain_config = rule( "coverage_link_flags": attr.string_list(), "supports_start_end_lib": attr.bool(), "builtin_sysroot": attr.string(), + "extra_flags_per_feature": attr.string_list_dict(), "_xcode_config": attr.label(default = configuration_field( fragment = "apple", name = "xcode_config_label",