From 4a13d9aaa35b2e372da9c6b4913817a97715a178 Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Mon, 21 Mar 2022 11:24:58 +0100 Subject: [PATCH 1/4] Don't leak deps from rust_proc_macro --- rust/private/rustc.bzl | 10 ++- test/unit/proc_macro/BUILD.bazel | 8 +++ .../leaks_deps/proc_macro_definition.rs | 6 ++ .../leaks_deps/proc_macro_dep/BUILD.bazel | 7 +++ .../proc_macro_dep/proc_macro_dep.rs | 3 + .../proc_macro_does_not_leak_deps.bzl | 62 +++++++++++++++++++ .../proc_macro/leaks_deps/proc_macro_user.rs | 10 +++ 7 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 test/unit/proc_macro/leaks_deps/proc_macro_definition.rs create mode 100644 test/unit/proc_macro/leaks_deps/proc_macro_dep/BUILD.bazel create mode 100644 test/unit/proc_macro/leaks_deps/proc_macro_dep/proc_macro_dep.rs create mode 100644 test/unit/proc_macro/leaks_deps/proc_macro_does_not_leak_deps.bzl create mode 100644 test/unit/proc_macro/leaks_deps/proc_macro_user.rs diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index 982dbc273e..81c6982ece 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -173,7 +173,15 @@ def collect_deps( dep = crate_info, )) - transitive_crates.append(depset([crate_info], transitive = [dep_info.transitive_crates])) + transitive_crates.append( + depset( + [crate_info], + transitive = [] if "proc-macro" in [ + crate_info.type, + crate_info.wrapped_crate_type, + ] else [dep_info.transitive_crates], + ), + ) transitive_crate_outputs.append( depset( [crate_info.output], diff --git a/test/unit/proc_macro/BUILD.bazel b/test/unit/proc_macro/BUILD.bazel index e2d56c177a..82aa8d8ae4 100644 --- a/test/unit/proc_macro/BUILD.bazel +++ b/test/unit/proc_macro/BUILD.bazel @@ -1,5 +1,13 @@ load(":proc_macro_test.bzl", "proc_macro_test_suite") +load( + "//test/unit/proc_macro:leaks_deps/proc_macro_does_not_leak_deps.bzl", + "proc_macro_does_not_leak_deps_test_suite", +) proc_macro_test_suite( name = "proc_macro_test_suite", ) + +proc_macro_does_not_leak_deps_test_suite( + name = "proc_macro_does_not_leak_deps_test_suite", +) diff --git a/test/unit/proc_macro/leaks_deps/proc_macro_definition.rs b/test/unit/proc_macro/leaks_deps/proc_macro_definition.rs new file mode 100644 index 0000000000..080dcdc1a5 --- /dev/null +++ b/test/unit/proc_macro/leaks_deps/proc_macro_definition.rs @@ -0,0 +1,6 @@ +use proc_macro::TokenStream; + +#[proc_macro] +pub fn make_double_forty_two(_item: TokenStream) -> TokenStream { + ("fn double_forty_two() -> i32 { 2 * proc_macro_dep::forty_two() }").parse().unwrap() +} diff --git a/test/unit/proc_macro/leaks_deps/proc_macro_dep/BUILD.bazel b/test/unit/proc_macro/leaks_deps/proc_macro_dep/BUILD.bazel new file mode 100644 index 0000000000..b76d0cdca7 --- /dev/null +++ b/test/unit/proc_macro/leaks_deps/proc_macro_dep/BUILD.bazel @@ -0,0 +1,7 @@ +load("//rust:defs.bzl", "rust_library") + +rust_library( + name = "proc_macro_dep", + srcs = ["proc_macro_dep.rs"], + visibility = ["//test:__subpackages__"], +) diff --git a/test/unit/proc_macro/leaks_deps/proc_macro_dep/proc_macro_dep.rs b/test/unit/proc_macro/leaks_deps/proc_macro_dep/proc_macro_dep.rs new file mode 100644 index 0000000000..6d2a0376eb --- /dev/null +++ b/test/unit/proc_macro/leaks_deps/proc_macro_dep/proc_macro_dep.rs @@ -0,0 +1,3 @@ +pub fn forty_two() -> i32 { + 42 +} diff --git a/test/unit/proc_macro/leaks_deps/proc_macro_does_not_leak_deps.bzl b/test/unit/proc_macro/leaks_deps/proc_macro_does_not_leak_deps.bzl new file mode 100644 index 0000000000..d36fdd8c64 --- /dev/null +++ b/test/unit/proc_macro/leaks_deps/proc_macro_does_not_leak_deps.bzl @@ -0,0 +1,62 @@ +"""Unittest to verify proc-macro targets""" + +load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") +load("//rust:defs.bzl", "rust_proc_macro", "rust_test") +load( + "//test/unit:common.bzl", + "assert_action_mnemonic", +) + +def _proc_macro_does_not_leak_deps_impl(ctx): + env = analysistest.begin(ctx) + actions = analysistest.target_under_test(env).actions + action = actions[0] + assert_action_mnemonic(env, action, "Rustc") + + # Our test depends on the proc_macro_dep crate both directly and indirectly via a + # proc_macro dependency. As proc_macro depdendencies are built in exec configuration mode, + # we check that there isn't an `-exec-` path to `proc_macro_dep` in the command line arguments. + proc_macro_dep_args = [arg for arg in action.argv if "proc_macro_dep" in arg] + proc_macro_dep_in_target_mode = [arg for arg in proc_macro_dep_args if "-exec-" not in arg] + proc_macro_dep_in_exec_mode = [arg for arg in proc_macro_dep_args if "-exec-" in arg] + + asserts.equals(env, 0, len(proc_macro_dep_in_exec_mode)) + asserts.true(env, len(proc_macro_dep_in_target_mode) > 0) + + return analysistest.end(env) + +proc_macro_does_not_leak_deps_test = analysistest.make(_proc_macro_does_not_leak_deps_impl) + +def _proc_macro_does_not_leak_deps_test(): + rust_proc_macro( + name = "proc_macro_definition", + srcs = ["leaks_deps/proc_macro_definition.rs"], + deps = ["//test/unit/proc_macro/leaks_deps/proc_macro_dep"], + ) + + rust_test( + name = "deps_not_leaked", + srcs = ["leaks_deps/proc_macro_user.rs"], + deps = ["//test/unit/proc_macro/leaks_deps/proc_macro_dep"], + proc_macro_deps = [":proc_macro_definition"], + ) + + proc_macro_does_not_leak_deps_test( + name = "proc_macro_does_not_leak_deps_test", + target_under_test = ":deps_not_leaked", + ) + +def proc_macro_does_not_leak_deps_test_suite(name): + """Entry-point macro called from the BUILD file. + + Args: + name: Name of the macro. + """ + _proc_macro_does_not_leak_deps_test() + + native.test_suite( + name = name, + tests = [ + ":proc_macro_does_not_leak_deps_test", + ], + ) diff --git a/test/unit/proc_macro/leaks_deps/proc_macro_user.rs b/test/unit/proc_macro/leaks_deps/proc_macro_user.rs new file mode 100644 index 0000000000..0a7cb9dcc7 --- /dev/null +++ b/test/unit/proc_macro/leaks_deps/proc_macro_user.rs @@ -0,0 +1,10 @@ +use proc_macro_definition::make_double_forty_two; +use proc_macro_dep::forty_two; + +make_double_forty_two!(); + +#[test] +fn test_answer_macro() { + assert_eq!(double_forty_two(), forty_two() + forty_two()); +} + From a50216ab69521faa8218a22bb9563f63eb303c34 Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Mon, 21 Mar 2022 11:43:38 +0100 Subject: [PATCH 2/4] Run buildifier and rustfmt --- test/unit/proc_macro/BUILD.bazel | 2 +- test/unit/proc_macro/leaks_deps/proc_macro_definition.rs | 4 +++- test/unit/proc_macro/leaks_deps/proc_macro_user.rs | 1 - 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/test/unit/proc_macro/BUILD.bazel b/test/unit/proc_macro/BUILD.bazel index 82aa8d8ae4..1927309a17 100644 --- a/test/unit/proc_macro/BUILD.bazel +++ b/test/unit/proc_macro/BUILD.bazel @@ -1,8 +1,8 @@ -load(":proc_macro_test.bzl", "proc_macro_test_suite") load( "//test/unit/proc_macro:leaks_deps/proc_macro_does_not_leak_deps.bzl", "proc_macro_does_not_leak_deps_test_suite", ) +load(":proc_macro_test.bzl", "proc_macro_test_suite") proc_macro_test_suite( name = "proc_macro_test_suite", diff --git a/test/unit/proc_macro/leaks_deps/proc_macro_definition.rs b/test/unit/proc_macro/leaks_deps/proc_macro_definition.rs index 080dcdc1a5..03e6019c07 100644 --- a/test/unit/proc_macro/leaks_deps/proc_macro_definition.rs +++ b/test/unit/proc_macro/leaks_deps/proc_macro_definition.rs @@ -2,5 +2,7 @@ use proc_macro::TokenStream; #[proc_macro] pub fn make_double_forty_two(_item: TokenStream) -> TokenStream { - ("fn double_forty_two() -> i32 { 2 * proc_macro_dep::forty_two() }").parse().unwrap() + ("fn double_forty_two() -> i32 { 2 * proc_macro_dep::forty_two() }") + .parse() + .unwrap() } diff --git a/test/unit/proc_macro/leaks_deps/proc_macro_user.rs b/test/unit/proc_macro/leaks_deps/proc_macro_user.rs index 0a7cb9dcc7..7c897d942f 100644 --- a/test/unit/proc_macro/leaks_deps/proc_macro_user.rs +++ b/test/unit/proc_macro/leaks_deps/proc_macro_user.rs @@ -7,4 +7,3 @@ make_double_forty_two!(); fn test_answer_macro() { assert_eq!(double_forty_two(), forty_two() + forty_two()); } - From a8c7702a4b4e1261c704599da37295616419ebb8 Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Mon, 21 Mar 2022 14:17:27 +0100 Subject: [PATCH 3/4] Simplify test --- test/unit/proc_macro/leaks_deps/proc_macro_definition.rs | 4 +++- .../proc_macro/leaks_deps/proc_macro_does_not_leak_deps.bzl | 3 --- test/unit/proc_macro/leaks_deps/proc_macro_user.rs | 3 +-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/test/unit/proc_macro/leaks_deps/proc_macro_definition.rs b/test/unit/proc_macro/leaks_deps/proc_macro_definition.rs index 03e6019c07..0fdbe4b9ab 100644 --- a/test/unit/proc_macro/leaks_deps/proc_macro_definition.rs +++ b/test/unit/proc_macro/leaks_deps/proc_macro_definition.rs @@ -1,8 +1,10 @@ use proc_macro::TokenStream; +use proc_macro_dep::forty_two; #[proc_macro] pub fn make_double_forty_two(_item: TokenStream) -> TokenStream { - ("fn double_forty_two() -> i32 { 2 * proc_macro_dep::forty_two() }") + let return_value = forty_two().to_string(); + ("fn double_forty_two() -> i32 { 2 * ".to_string() + &return_value + " }") .parse() .unwrap() } diff --git a/test/unit/proc_macro/leaks_deps/proc_macro_does_not_leak_deps.bzl b/test/unit/proc_macro/leaks_deps/proc_macro_does_not_leak_deps.bzl index d36fdd8c64..20adf51232 100644 --- a/test/unit/proc_macro/leaks_deps/proc_macro_does_not_leak_deps.bzl +++ b/test/unit/proc_macro/leaks_deps/proc_macro_does_not_leak_deps.bzl @@ -17,11 +17,9 @@ def _proc_macro_does_not_leak_deps_impl(ctx): # proc_macro dependency. As proc_macro depdendencies are built in exec configuration mode, # we check that there isn't an `-exec-` path to `proc_macro_dep` in the command line arguments. proc_macro_dep_args = [arg for arg in action.argv if "proc_macro_dep" in arg] - proc_macro_dep_in_target_mode = [arg for arg in proc_macro_dep_args if "-exec-" not in arg] proc_macro_dep_in_exec_mode = [arg for arg in proc_macro_dep_args if "-exec-" in arg] asserts.equals(env, 0, len(proc_macro_dep_in_exec_mode)) - asserts.true(env, len(proc_macro_dep_in_target_mode) > 0) return analysistest.end(env) @@ -37,7 +35,6 @@ def _proc_macro_does_not_leak_deps_test(): rust_test( name = "deps_not_leaked", srcs = ["leaks_deps/proc_macro_user.rs"], - deps = ["//test/unit/proc_macro/leaks_deps/proc_macro_dep"], proc_macro_deps = [":proc_macro_definition"], ) diff --git a/test/unit/proc_macro/leaks_deps/proc_macro_user.rs b/test/unit/proc_macro/leaks_deps/proc_macro_user.rs index 7c897d942f..739e381254 100644 --- a/test/unit/proc_macro/leaks_deps/proc_macro_user.rs +++ b/test/unit/proc_macro/leaks_deps/proc_macro_user.rs @@ -1,9 +1,8 @@ use proc_macro_definition::make_double_forty_two; -use proc_macro_dep::forty_two; make_double_forty_two!(); #[test] fn test_answer_macro() { - assert_eq!(double_forty_two(), forty_two() + forty_two()); + assert_eq!(double_forty_two(), 84); } From c366ec52f01a020ac8f9792caa5b0d02f3077e8d Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Mon, 21 Mar 2022 14:54:27 +0100 Subject: [PATCH 4/4] Don't put proc_macro deps into transitive inputs --- rust/private/rustc.bzl | 5 ++++- .../proc_macro/leaks_deps/proc_macro_definition.rs | 7 ++----- .../leaks_deps/proc_macro_does_not_leak_deps.bzl | 10 +++++----- test/unit/proc_macro/leaks_deps/proc_macro_user.rs | 6 ++---- 4 files changed, 13 insertions(+), 15 deletions(-) diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index 81c6982ece..dc8ed83141 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -185,7 +185,10 @@ def collect_deps( transitive_crate_outputs.append( depset( [crate_info.output], - transitive = [dep_info.transitive_crate_outputs], + transitive = [] if "proc-macro" in [ + crate_info.type, + crate_info.wrapped_crate_type, + ] else [dep_info.transitive_crate_outputs], ), ) transitive_noncrates.append(dep_info.transitive_noncrates) diff --git a/test/unit/proc_macro/leaks_deps/proc_macro_definition.rs b/test/unit/proc_macro/leaks_deps/proc_macro_definition.rs index 0fdbe4b9ab..9fcad75d27 100644 --- a/test/unit/proc_macro/leaks_deps/proc_macro_definition.rs +++ b/test/unit/proc_macro/leaks_deps/proc_macro_definition.rs @@ -2,9 +2,6 @@ use proc_macro::TokenStream; use proc_macro_dep::forty_two; #[proc_macro] -pub fn make_double_forty_two(_item: TokenStream) -> TokenStream { - let return_value = forty_two().to_string(); - ("fn double_forty_two() -> i32 { 2 * ".to_string() + &return_value + " }") - .parse() - .unwrap() +pub fn make_forty_two(_item: TokenStream) -> TokenStream { + forty_two().to_string().parse().unwrap() } diff --git a/test/unit/proc_macro/leaks_deps/proc_macro_does_not_leak_deps.bzl b/test/unit/proc_macro/leaks_deps/proc_macro_does_not_leak_deps.bzl index 20adf51232..c2b716281c 100644 --- a/test/unit/proc_macro/leaks_deps/proc_macro_does_not_leak_deps.bzl +++ b/test/unit/proc_macro/leaks_deps/proc_macro_does_not_leak_deps.bzl @@ -13,13 +13,13 @@ def _proc_macro_does_not_leak_deps_impl(ctx): action = actions[0] assert_action_mnemonic(env, action, "Rustc") - # Our test depends on the proc_macro_dep crate both directly and indirectly via a - # proc_macro dependency. As proc_macro depdendencies are built in exec configuration mode, - # we check that there isn't an `-exec-` path to `proc_macro_dep` in the command line arguments. + # Our test target has a dependency on "proc_macro_dep" via a rust_proc_macro target, + # so the proc_macro_dep rlib should not appear as an input to the Rustc action, nor as -Ldependency on the command line. + proc_macro_dep_inputs = [i for i in action.inputs.to_list() if "proc_macro_dep" in i.path] proc_macro_dep_args = [arg for arg in action.argv if "proc_macro_dep" in arg] - proc_macro_dep_in_exec_mode = [arg for arg in proc_macro_dep_args if "-exec-" in arg] - asserts.equals(env, 0, len(proc_macro_dep_in_exec_mode)) + asserts.equals(env, 0, len(proc_macro_dep_inputs)) + asserts.equals(env, 0, len(proc_macro_dep_args)) return analysistest.end(env) diff --git a/test/unit/proc_macro/leaks_deps/proc_macro_user.rs b/test/unit/proc_macro/leaks_deps/proc_macro_user.rs index 739e381254..0be3419dc8 100644 --- a/test/unit/proc_macro/leaks_deps/proc_macro_user.rs +++ b/test/unit/proc_macro/leaks_deps/proc_macro_user.rs @@ -1,8 +1,6 @@ -use proc_macro_definition::make_double_forty_two; - -make_double_forty_two!(); +use proc_macro_definition::make_forty_two; #[test] fn test_answer_macro() { - assert_eq!(double_forty_two(), 84); + assert_eq!(make_forty_two!(), 42); }