Skip to content

Commit 3e8f19e

Browse files
authored
Cleanup args construction (#2122)
Some minor optimizations to analysis time memory usage (avoid creating some list temporaries, some extra strings, etc.) Some of the usage might be simpler as `args.add("foo", foo)` instead of `args.add(foo, format="foo=%s")` but I opted to keep the usage the same as before for safety.
1 parent 582346a commit 3e8f19e

File tree

8 files changed

+45
-54
lines changed

8 files changed

+45
-54
lines changed

bindgen/bindgen.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ def _rust_bindgen_impl(ctx):
115115

116116
# Configure Bindgen Arguments
117117
args.add_all(ctx.attr.bindgen_flags)
118-
args.add(header.path)
118+
args.add(header)
119119
args.add("--output", output)
120120

121121
# Vanilla usage of bindgen produces formatted output, here we do the same if we have `rustfmt` in our toolchain.

cargo/private/cargo_build_script.bzl

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -213,18 +213,17 @@ def _cargo_build_script_impl(ctx):
213213
# See https://doc.rust-lang.org/cargo/reference/build-scripts.html#-sys-packages
214214
# for details.
215215
args = ctx.actions.args()
216-
args.add_all([
217-
script.path,
218-
links,
219-
out_dir.path,
220-
env_out.path,
221-
flags_out.path,
222-
link_flags.path,
223-
link_search_paths.path,
224-
dep_env_out.path,
225-
streams.stdout.path,
226-
streams.stderr.path,
227-
])
216+
args.add(script)
217+
args.add(links)
218+
args.add(out_dir.path)
219+
args.add(env_out)
220+
args.add(flags_out)
221+
args.add(link_flags)
222+
args.add(link_search_paths)
223+
args.add(dep_env_out)
224+
args.add(streams.stdout)
225+
args.add(streams.stderr)
226+
228227
build_script_inputs = []
229228
for dep in ctx.attr.link_deps:
230229
if rust_common.dep_info in dep and dep[rust_common.dep_info].dep_env:

proto/protobuf/toolchain.bzl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,10 @@ def rust_generate_proto(
8080
# so we create an empty grpc module in the other case.
8181
tools.append(proto_toolchain.grpc_plugin)
8282
tools.append(ctx.executable._optional_output_wrapper)
83-
args.add_all([f.path for f in grpc_files])
83+
args.add_all(grpc_files)
8484
args.add_all([
8585
"--",
86-
proto_toolchain.protoc.path,
86+
proto_toolchain.protoc,
8787
"--plugin=protoc-gen-grpc-rust=" + proto_toolchain.grpc_plugin.path,
8888
"--grpc-rust_out=" + output_directory,
8989
])

rust/private/clippy.bzl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ def _clippy_aspect_impl(target, ctx):
137137
# or rustc may fail to create intermediate output files because the directory does not exist.
138138
if ctx.attr._capture_output[CaptureClippyOutputInfo].capture_output:
139139
clippy_out = ctx.actions.declare_file(ctx.label.name + ".clippy.out", sibling = crate_info.output)
140-
args.process_wrapper_flags.add("--stderr-file", clippy_out.path)
140+
args.process_wrapper_flags.add("--stderr-file", clippy_out)
141141

142142
if clippy_flags:
143143
fail("""Combining @rules_rust//:clippy_flags with @rules_rust//:capture_clippy_output=true is currently not supported.
@@ -150,7 +150,7 @@ See https://github.com/bazelbuild/rules_rust/pull/1264#discussion_r853241339 for
150150
# A marker file indicating clippy has executed successfully.
151151
# This file is necessary because "ctx.actions.run" mandates an output.
152152
clippy_out = ctx.actions.declare_file(ctx.label.name + ".clippy.ok", sibling = crate_info.output)
153-
args.process_wrapper_flags.add("--touch-file", clippy_out.path)
153+
args.process_wrapper_flags.add("--touch-file", clippy_out)
154154

155155
if clippy_flags:
156156
args.rustc_flags.add_all(clippy_flags)

rust/private/rustc.bzl

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -899,8 +899,8 @@ def construct_arguments(
899899
rustc_flags.set_param_file_format("multiline")
900900
rustc_flags.use_param_file("@%s", use_always = False)
901901
rustc_flags.add(crate_info.root)
902-
rustc_flags.add("--crate-name=" + crate_info.name)
903-
rustc_flags.add("--crate-type=" + crate_info.type)
902+
rustc_flags.add(crate_info.name, format = "--crate-name=%s")
903+
rustc_flags.add(crate_info.type, format = "--crate-type=%s")
904904

905905
error_format = "human"
906906
if hasattr(attr, "_error_format"):
@@ -921,15 +921,15 @@ def construct_arguments(
921921
# If the os is not windows, we can get colorized output.
922922
json.append("diagnostic-rendered-ansi")
923923

924-
rustc_flags.add("--json=" + ",".join(json))
924+
rustc_flags.add_joined(json, format_joined = "--json=%s", join_with = ",")
925925

926926
error_format = "json"
927927

928928
if build_metadata:
929929
# Configure process_wrapper to terminate rustc when metadata are emitted
930930
process_wrapper_flags.add("--rustc-quit-on-rmeta", "true")
931931

932-
rustc_flags.add("--error-format=" + error_format)
932+
rustc_flags.add(error_format, format = "--error-format=%s")
933933

934934
# Mangle symbols to disambiguate crates with the same name. This could
935935
# happen only for non-final artifacts where we compute an output_hash,
@@ -939,37 +939,33 @@ def construct_arguments(
939939
# Bazel, such as rust_binary, rust_static_library and rust_shared_library,
940940
# where output_hash is None we don't need to add these flags.
941941
if output_hash:
942-
extra_filename = "-" + output_hash
943-
rustc_flags.add("--codegen=metadata=" + extra_filename)
944-
rustc_flags.add("--codegen=extra-filename=" + extra_filename)
942+
rustc_flags.add(output_hash, format = "--codegen=metadata=-%s")
943+
rustc_flags.add(output_hash, format = "--codegen=extra-filename=-%s")
945944

946945
if output_dir:
947-
rustc_flags.add("--out-dir=" + output_dir)
946+
rustc_flags.add(output_dir, format = "--out-dir=%s")
948947

949948
compilation_mode = get_compilation_mode_opts(ctx, toolchain)
950-
rustc_flags.add("--codegen=opt-level=" + compilation_mode.opt_level)
951-
rustc_flags.add("--codegen=debuginfo=" + compilation_mode.debug_info)
949+
rustc_flags.add(compilation_mode.opt_level, format = "--codegen=opt-level=%s")
950+
rustc_flags.add(compilation_mode.debug_info, format = "--codegen=debuginfo=%s")
952951

953952
# For determinism to help with build distribution and such
954953
if remap_path_prefix != None:
955954
rustc_flags.add("--remap-path-prefix=${{pwd}}={}".format(remap_path_prefix))
956955

957956
if emit:
958-
rustc_flags.add("--emit=" + ",".join(emit_with_paths))
957+
rustc_flags.add_joined(emit_with_paths, format_joined = "--emit=%s", join_with = ",")
959958
if error_format != "json":
960959
# Color is not compatible with json output.
961960
rustc_flags.add("--color=always")
962-
rustc_flags.add("--target=" + toolchain.target_flag_value)
961+
rustc_flags.add(toolchain.target_flag_value, format = "--target=%s")
963962
if hasattr(attr, "crate_features"):
964963
rustc_flags.add_all(getattr(attr, "crate_features"), before_each = "--cfg", format_each = 'feature="%s"')
965964
if linker_script:
966-
rustc_flags.add(linker_script.path, format = "--codegen=link-arg=-T%s")
965+
rustc_flags.add(linker_script, format = "--codegen=link-arg=-T%s")
967966

968-
# Gets the paths to the folders containing the standard library (or libcore)
969-
rust_std_paths = toolchain.rust_std_paths.to_list()
970-
971-
# Tell Rustc where to find the standard library
972-
rustc_flags.add_all(rust_std_paths, before_each = "-L", format_each = "%s")
967+
# Tell Rustc where to find the standard library (or libcore)
968+
rustc_flags.add_all(toolchain.rust_std_paths, before_each = "-L", format_each = "%s")
973969
rustc_flags.add_all(rust_flags)
974970

975971
# Gather data path from crate_info since it is inherited from real crate for rust_doc and rust_test
@@ -995,12 +991,12 @@ def construct_arguments(
995991
use_pic = _should_use_pic(cc_toolchain, feature_configuration, crate_info.type, compilation_mode)
996992
rpaths = _compute_rpaths(toolchain, output_dir, dep_info, use_pic)
997993
else:
998-
rpaths = depset([])
994+
rpaths = depset()
999995

1000996
ld, link_args, link_env = get_linker_and_args(ctx, attr, crate_info.type, cc_toolchain, feature_configuration, rpaths, rustdoc)
1001997

1002998
env.update(link_env)
1003-
rustc_flags.add("--codegen=linker=" + ld)
999+
rustc_flags.add(ld, format = "--codegen=linker=%s")
10041000
rustc_flags.add_joined("--codegen", link_args, join_with = " ", format_joined = "link-args=%s")
10051001

10061002
_add_native_link_flags(rustc_flags, dep_info, linkstamp_outs, ambiguous_libs, crate_info.type, toolchain, cc_toolchain, feature_configuration, compilation_mode)
@@ -1067,7 +1063,7 @@ def construct_arguments(
10671063
rustc_flags.add_all(ctx.attr._extra_exec_rustc_flag[ExtraExecRustcFlagsInfo].extra_exec_rustc_flags)
10681064

10691065
if _is_no_std(ctx, toolchain, crate_info):
1070-
rustc_flags.add_all(['--cfg=feature="no_std"'])
1066+
rustc_flags.add('--cfg=feature="no_std"')
10711067

10721068
# Create a struct which keeps the arguments separate so each may be tuned or
10731069
# replaced where necessary
@@ -1590,7 +1586,7 @@ def add_edition_flags(args, crate):
15901586
crate (CrateInfo): A CrateInfo provider
15911587
"""
15921588
if crate.edition != "2015":
1593-
args.add("--edition={}".format(crate.edition))
1589+
args.add(crate.edition, format = "--edition=%s")
15941590

15951591
def _create_extra_input_args(build_info, dep_info):
15961592
"""Gather additional input arguments from transitive dependencies
@@ -1928,12 +1924,11 @@ def _add_native_link_flags(args, dep_info, linkstamp_outs, ambiguous_libs, crate
19281924
# If there are ambiguous libs, the disambiguation symlinks to them are
19291925
# all created in the same directory. Add it to the library search path.
19301926
ambiguous_libs_dirname = ambiguous_libs.values()[0].dirname
1931-
args.add("-Lnative={}".format(ambiguous_libs_dirname))
1927+
args.add(ambiguous_libs_dirname, format = "-Lnative=%s")
19321928

19331929
args.add_all(args_and_pic_and_ambiguous_libs, map_each = make_link_flags)
19341930

1935-
for linkstamp_out in linkstamp_outs:
1936-
args.add_all(["-C", "link-arg=%s" % linkstamp_out.path])
1931+
args.add_all(linkstamp_outs, before_each = "-C", format_each = "link-args=%s")
19371932

19381933
if crate_type in ["dylib", "cdylib"]:
19391934
# For shared libraries we want to link C++ runtime library dynamically

rust/private/rustdoc.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ def rustdoc_compile_action(
138138

139139
# `rustdoc` does not support the SYSROOT environment variable. To account
140140
# for this, the flag must be explicitly passed to the `rustdoc` binary.
141-
args.rustc_flags.add("--sysroot=${{pwd}}/{}".format(toolchain.sysroot_short_path))
141+
args.rustc_flags.add(toolchain.sysroot_short_path, format = "--sysroot=${{pwd}}/%s")
142142

143143
return struct(
144144
executable = ctx.executable._process_wrapper,

rust/private/rustfmt.bzl

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,11 @@ def _perform_check(edition, srcs, ctx):
7474
marker = ctx.actions.declare_file(ctx.label.name + ".rustfmt.ok")
7575

7676
args = ctx.actions.args()
77-
args.add("--touch-file")
78-
args.add(marker)
77+
args.add("--touch-file", marker)
7978
args.add("--")
8079
args.add(rustfmt_toolchain.rustfmt)
81-
args.add("--config-path")
82-
args.add(config)
83-
args.add("--edition")
84-
args.add(edition)
80+
args.add("--config-path", config)
81+
args.add("--edition", edition)
8582
args.add("--check")
8683
args.add_all(srcs)
8784

test/process_wrapper/process_wrapper_tester.bzl

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,21 +25,21 @@ def _impl(ctx):
2525
if combined or ctx.attr.test_config == "stdout":
2626
stdout_output = ctx.actions.declare_file(ctx.label.name + ".stdout")
2727
outputs.append(stdout_output)
28-
args.add("--stdout-file", stdout_output.path)
28+
args.add("--stdout-file", stdout_output)
2929

3030
if combined or ctx.attr.test_config == "stderr":
3131
stderr_output = ctx.actions.declare_file(ctx.label.name + ".stderr")
3232
outputs.append(stderr_output)
33-
args.add("--stderr-file", stderr_output.path)
33+
args.add("--stderr-file", stderr_output)
3434

3535
if combined or (ctx.attr.test_config != "stdout" and ctx.attr.test_config != "stderr"):
3636
touch_output = ctx.actions.declare_file(ctx.label.name + ".touch")
3737
outputs.append(touch_output)
38-
args.add("--touch-file", touch_output.path)
38+
args.add("--touch-file", touch_output)
3939
if ctx.attr.test_config == "copy-output":
4040
copy_output = ctx.actions.declare_file(ctx.label.name + ".touch.copy")
4141
outputs.append(copy_output)
42-
args.add_all("--copy-output", [touch_output.path, copy_output.path])
42+
args.add_all("--copy-output", [touch_output, copy_output])
4343

4444
if combined or ctx.attr.test_config == "env-files":
4545
args.add_all(ctx.files.env_files, before_each = "--env-file")
@@ -53,7 +53,7 @@ def _impl(ctx):
5353

5454
args.add("--")
5555

56-
args.add(ctx.executable._process_wrapper_tester.path)
56+
args.add(ctx.executable._process_wrapper_tester)
5757
args.add(ctx.attr.test_config)
5858
args.add("--current-dir", "${pwd}")
5959
args.add("--test-subst", "subst key to ${key}")

0 commit comments

Comments
 (0)