Split libc++ and libc headers in toolchain_headers.bzl and be precise which header to pick as the public header. Right now we pick all the headers with basename in STD_HEADERS as public headers, and this is picking up too many headers in practice. For example time.h (which we will add in a followup) is present 3 times (includes/time.h, includes/sys/time.h, includes/linux/time.h) in our environment. PiperOrigin-RevId: 449462761
diff --git a/rs_bindings_from_cc/BUILD b/rs_bindings_from_cc/BUILD index 293ae6b..72006b6 100644 --- a/rs_bindings_from_cc/BUILD +++ b/rs_bindings_from_cc/BUILD
@@ -422,7 +422,7 @@ ], ) -STD_HEADERS = [ +LIBCXX_HEADERS = [ "cstddef", "cstdlib", "version", @@ -445,6 +445,8 @@ "stdlib.h", ] +LIBC_HEADERS = [] + config_setting( name = "llvm_unstable", values = { @@ -475,7 +477,8 @@ bindings_for_toolchain_headers( name = "cc_std", hdrs = ":toolchain_headers", - public_hdrs = STD_HEADERS, + public_libc_hdrs = LIBC_HEADERS, + public_libcxx_hdrs = LIBCXX_HEADERS, visibility = ["//visibility:public"], )
diff --git a/rs_bindings_from_cc/bazel_support/toolchain_headers.bzl b/rs_bindings_from_cc/bazel_support/toolchain_headers.bzl index f09c3ea..9e45e26 100644 --- a/rs_bindings_from_cc/bazel_support/toolchain_headers.bzl +++ b/rs_bindings_from_cc/bazel_support/toolchain_headers.bzl
@@ -19,57 +19,63 @@ "generate_and_compile_bindings", ) -def _is_public_std_header(input, public_hdrs): - return ( - input.basename in public_hdrs and - "experimental" not in input.short_path - ) +def _has_suffix(input, suffices): + for suffix in suffices: + if input.short_path.endswith(suffix): + return True + return False -def _collect_std_hdrs(input_list, public_hdrs): - return [hdr for hdr in input_list if _is_public_std_header(hdr, public_hdrs)] +def _filter_headers_with_suffices(input_list, suffices): + return [hdr for hdr in input_list if _has_suffix(hdr, suffices)] -def _collect_nonstd_hdrs(input_list, public_hdrs): - return [hdr for hdr in input_list if not _is_public_std_header(hdr, public_hdrs)] +def _filter_headers_without_suffices(input_list, suffices): + return [hdr for hdr in input_list if not _has_suffix(hdr, suffices)] + +def _add_prefix(strings, prefix): + return [prefix + s for s in strings] def _bindings_for_toolchain_headers_impl(ctx): - std_hdrs = ctx.attr._stl[CcInfo].compilation_context.headers.to_list() + ctx.files.hdrs - all_std_hdrs = depset(direct = ctx.files.hdrs + ctx.files._builtin_hdrs, transitive = [ctx.attr._stl[CcInfo].compilation_context.headers]) + std_files = ctx.attr._stl[CcInfo].compilation_context.headers.to_list() + ctx.files.hdrs + std_and_builtin_files = depset(direct = ctx.files.hdrs + ctx.files._builtin_hdrs, transitive = [ctx.attr._stl[CcInfo].compilation_context.headers]) - # The clang builtin headers also contain some standard headers. We'll consider those part of - # the C++ Standard library target, so we generate bindings for them. - builtin_std_hdrs = _collect_std_hdrs(ctx.files._builtin_hdrs, ctx.attr.public_hdrs) - builtin_nonstd_hdrs = _collect_nonstd_hdrs( + prefixed_libcxx_hdrs = _add_prefix(ctx.attr.public_libcxx_hdrs, "c++/v1/") + + # The clang builtin headers also contain some libc++ headers. We consider those part of + # the libc++ target, so we generate bindings for them. + builtin_libcxx_files = _filter_headers_with_suffices(ctx.files._builtin_hdrs, prefixed_libcxx_hdrs) + builtin_nonstd_files = _filter_headers_without_suffices( ctx.files._builtin_hdrs, - ctx.attr.public_hdrs, + ctx.attr.public_libcxx_hdrs, ) targets_and_headers = depset( direct = [ json.encode({ "t": str(ctx.label), - "h": [hdr.path for hdr in std_hdrs + builtin_std_hdrs], + "h": [hdr.path for hdr in std_files + builtin_libcxx_files], }), json.encode({ - "t": "//:_builtin_hdrs", - "h": [h.path for h in builtin_nonstd_hdrs], + "t": "//:_nothing_should_depend_on_private_builtin_hdrs", + "h": [h.path for h in builtin_nonstd_files], }), ], ) - public_std_hdrs = _collect_std_hdrs(std_hdrs, ctx.attr.public_hdrs) + public_libcxx_files = _filter_headers_with_suffices(std_files, prefixed_libcxx_hdrs) + public_libc_files = _filter_headers_with_suffices(std_files, _add_prefix(ctx.attr.public_libc_hdrs, "v5/include/")) header_includes = [] - for hdr in public_std_hdrs: + for hdr in ctx.attr.public_libcxx_hdrs + ctx.attr.public_libc_hdrs: header_includes.append("-include") - header_includes.append(hdr.basename) + header_includes.append(hdr) - return [RustToolchainHeadersInfo(headers = all_std_hdrs)] + generate_and_compile_bindings( + return [RustToolchainHeadersInfo(headers = std_and_builtin_files)] + generate_and_compile_bindings( ctx, ctx.attr, compilation_context = ctx.attr._stl[CcInfo].compilation_context, - public_hdrs = public_std_hdrs, + public_hdrs = public_libc_files + public_libcxx_files, header_includes = header_includes, - action_inputs = all_std_hdrs, + action_inputs = std_and_builtin_files, targets_and_headers = targets_and_headers, deps_for_cc_file = ctx.attr._deps_for_bindings[DepsForBindingsInfo].deps_for_cc_file, deps_for_rs_file = ctx.attr._deps_for_bindings[DepsForBindingsInfo].deps_for_rs_file, @@ -80,8 +86,8 @@ attrs = dict( bindings_attrs.items() + { "hdrs": attr.label(), - "public_hdrs": attr.string_list(), - "deps": attr.label_list(), + "public_libc_hdrs": attr.string_list(), + "public_libcxx_hdrs": attr.string_list(), "_stl": attr.label(default = "//third_party/stl:stl"), }.items(), ),
diff --git a/rs_bindings_from_cc/importer.cc b/rs_bindings_from_cc/importer.cc index ff988e1..255b5a6 100644 --- a/rs_bindings_from_cc/importer.cc +++ b/rs_bindings_from_cc/importer.cc
@@ -468,7 +468,7 @@ llvm::Optional<llvm::StringRef> filename = source_manager.getNonBuiltinFilenameForID(id); if (!filename) { - return BazelLabel("//:builtin"); + return BazelLabel("//:_nothing_should_depend_on_private_builtin_hdrs"); } if (filename->startswith("./")) { filename = filename->substr(2);
diff --git a/rs_bindings_from_cc/test/bazel_unit_tests/headers_and_targets/rust_bindings_from_cc_aspect_test.bzl b/rs_bindings_from_cc/test/bazel_unit_tests/headers_and_targets/rust_bindings_from_cc_aspect_test.bzl index 57833ca..7511fd4 100644 --- a/rs_bindings_from_cc/test/bazel_unit_tests/headers_and_targets/rust_bindings_from_cc_aspect_test.bzl +++ b/rs_bindings_from_cc/test/bazel_unit_tests/headers_and_targets/rust_bindings_from_cc_aspect_test.bzl
@@ -61,7 +61,7 @@ asserts.equals( env, targets_and_headers[1]["t"], - "//:_builtin_hdrs", + "//:_nothing_should_depend_on_private_builtin_hdrs", ) return analysistest.end(env) @@ -83,7 +83,8 @@ target_under_test = analysistest.target_under_test(env) targets_and_headers = _get_targets_and_headers(target_under_test) - asserts.equals(env, 0, len(targets_and_headers)) + # only headers and targets from the libc++ are there + asserts.equals(env, 1, len(targets_and_headers)) return analysistest.end(env) @@ -102,15 +103,15 @@ target_under_test = analysistest.target_under_test(env) targets_and_headers = _get_targets_and_headers(target_under_test) - asserts.equals(env, 1, len(targets_and_headers)) + asserts.equals(env, 2, len(targets_and_headers)) asserts.equals( env, - targets_and_headers[0]["t"], + targets_and_headers[1]["t"], "//rs_bindings_from_cc/test/bazel_unit_tests/headers_and_targets:mylib", ) asserts.equals( env, - targets_and_headers[0]["h"], + targets_and_headers[1]["h"], ["rs_bindings_from_cc/test/bazel_unit_tests/headers_and_targets/lib.h"], ) @@ -132,27 +133,27 @@ target_under_test = analysistest.target_under_test(env) targets_and_headers = _get_targets_and_headers(target_under_test) - asserts.equals(env, 2, len(targets_and_headers)) + asserts.equals(env, 3, len(targets_and_headers)) asserts.equals( env, - targets_and_headers[0]["t"], + targets_and_headers[1]["t"], "//rs_bindings_from_cc/test/bazel_unit_tests/headers_and_targets:bottom", ) asserts.equals( env, - targets_and_headers[0]["h"], + targets_and_headers[1]["h"], ["rs_bindings_from_cc/test/bazel_unit_tests/headers_and_targets/lib.h"], ) asserts.equals( env, - targets_and_headers[1]["t"], + targets_and_headers[2]["t"], "//rs_bindings_from_cc/test/bazel_unit_tests/headers_and_targets:top", ) asserts.equals( env, - targets_and_headers[1]["h"], + targets_and_headers[2]["h"], ["rs_bindings_from_cc/test/bazel_unit_tests/headers_and_targets/top.h"], ) @@ -179,10 +180,10 @@ targets_and_headers = _get_targets_and_headers(target_under_test) # Check that none of the textual headers made it into the targets_and_headers provider. - asserts.equals(env, 1, len(targets_and_headers)) + asserts.equals(env, 2, len(targets_and_headers)) asserts.equals( env, - targets_and_headers[0]["h"], + targets_and_headers[1]["h"], ["rs_bindings_from_cc/test/bazel_unit_tests/headers_and_targets/nontextual.h"], ) @@ -244,8 +245,8 @@ target_under_test = analysistest.target_under_test(env) targets_and_headers = _get_targets_and_headers(target_under_test) - asserts.equals(env, 1, len(targets_and_headers)) - header_path = targets_and_headers[0]["h"][0] + asserts.equals(env, 2, len(targets_and_headers)) + header_path = targets_and_headers[1]["h"][0] asserts.true( env, header_path