Avoid quadratic deduping in location expansion (#3636)

Does what it says on the tin. Usually the deduping was not necessary at
all because the argument were already coming from a depset or an
attribute (though kept for safety), but doing the same work for every
arg was truly unnecessary

Before:
<img width="898" height="222" alt="image"
src="https://github.com/user-attachments/assets/1ffce739-20eb-4c3b-8cc6-fed0ba702aa0"
/>


After:
<img width="898" height="150" alt="image"
src="https://github.com/user-attachments/assets/2497e70f-3434-4e9d-9dc8-4c9535cd2feb"
/>

Author: dzbarsky

cargo/private/cargo_build_script.bzl

@@ -19,6 +19,7 @@ load(
 load(
     "//rust/private:utils.bzl",
     "dedent",
+    "deduplicate",
     "expand_dict_value_locations",
     "find_cc_toolchain",
     "find_toolchain",
@@ -485,16 +486,19 @@ def _cargo_build_script_impl(ctx):
             variables = getattr(target[platform_common.TemplateVariableInfo], "variables", depset([]))
             known_variables.update(variables)
 
-    _merge_env_dict(env, expand_dict_value_locations(
-        ctx,
-        ctx.attr.build_script_env,
-        getattr(ctx.attr, "data", []) +
-        getattr(ctx.attr, "compile_data", []) +
-        getattr(ctx.attr, "tools", []) +
-        script_info.data +
-        script_info.tools,
-        known_variables,
-    ))
+    if ctx.attr.build_script_env:
+        _merge_env_dict(env, expand_dict_value_locations(
+            ctx,
+            ctx.attr.build_script_env,
+            deduplicate(
+                getattr(ctx.attr, "data", []) +
+                getattr(ctx.attr, "compile_data", []) +
+                getattr(ctx.attr, "tools", []) +
+                script_info.data +
+                script_info.tools,
+            ),
+            known_variables,
+        ))
 
     tools = depset(
         direct = [

rust/private/rust.bzl

@@ -35,6 +35,7 @@ load(
     "compute_crate_name",
     "crate_root_src",
     "dedent",
+    "deduplicate",
     "determine_lib_name",
     "determine_output_hash",
     "expand_dict_value_locations",
@@ -401,13 +402,13 @@ def _rust_test_impl(ctx):
 
         # crate.rustc_env is already expanded upstream in rust_library rule implementation
         rustc_env = dict(crate.rustc_env)
-        data_paths = depset(direct = getattr(ctx.attr, "data", [])).to_list()
-        rustc_env.update(expand_dict_value_locations(
-            ctx,
-            ctx.attr.rustc_env,
-            data_paths,
-            {},
-        ))
+        if ctx.attr.rustc_env:
+            rustc_env.update(expand_dict_value_locations(
+                ctx,
+                ctx.attr.rustc_env,
+                deduplicate(getattr(ctx.attr, "data", [])),
+                {},
+            ))
         aliases = dict(crate.aliases)
         aliases.update(ctx.attr.aliases)
 
@@ -467,13 +468,15 @@ def _rust_test_impl(ctx):
             )
             rustc_rmeta_output = generate_output_diagnostics(ctx, rust_metadata)
 
-        data_paths = depset(direct = getattr(ctx.attr, "data", [])).to_list()
-        rustc_env = expand_dict_value_locations(
-            ctx,
-            ctx.attr.rustc_env,
-            data_paths,
-            {},
-        )
+        if ctx.attr.rustc_env:
+            rustc_env = expand_dict_value_locations(
+                ctx,
+                ctx.attr.rustc_env,
+                deduplicate(getattr(ctx.attr, "data", [])),
+                {},
+            )
+        else:
+            rustc_env = {}
 
         # Target is a standalone crate. Build the test binary as its own crate.
         crate_info_dict = dict(

rust/private/rust_analyzer.bzl

@@ -28,7 +28,7 @@ load(
     "//rust/private:utils.bzl",
     "concat",
     "dedent",
-    "dedup_expand_location",
+    "deduplicate",
     "find_toolchain",
 )
 
@@ -270,9 +270,9 @@ def _create_single_crate(ctx, attrs, info):
 
     # TODO: The only imagined use case is an env var holding a filename in the workspace passed to a
     # macro like include_bytes!. Other use cases might exist that require more complex logic.
-    expand_targets = concat([getattr(attrs, attr, []) for attr in ["data", "compile_data"]])
+    expand_targets = deduplicate(concat([getattr(attrs, attr, []) for attr in ["data", "compile_data"]]))
 
-    crate["env"].update({k: dedup_expand_location(ctx, v, expand_targets) for k, v in info.env.items()})
+    crate["env"].update({k: ctx.expand_location(v, expand_targets) for k, v in info.env.items()})
 
     # Omit when a crate appears to depend on itself (e.g. foo_test crates).
     # It can happen a single source file is present in multiple crates - there can

rust/private/utils.bzl

@@ -250,20 +250,20 @@ def get_preferred_artifact(library_to_link, use_pic):
 # We do this to work around a potential crash, see
 # https://github.com/bazelbuild/bazel/issues/16664
 def dedup_expand_location(ctx, input, targets = []):
-    return ctx.expand_location(input, _deduplicate(targets))
+    return ctx.expand_location(input, deduplicate(targets))
 
-def _deduplicate(xs):
+def deduplicate(xs):
     return {x: True for x in xs}.keys()
 
 def concat(xss):
     return [x for xs in xss for x in xs]
 
-def _expand_location_for_build_script_runner(ctx, env, data, known_variables):
+def _expand_location_for_build_script_runner(ctx, v, data, known_variables):
     """A trivial helper for `expand_dict_value_locations` and `expand_list_element_locations`
 
     Args:
         ctx (ctx): The rule's context object
-        env (str): The value possibly containing location macros to expand.
+        v (str): The value possibly containing location macros to expand.
         data (sequence of Targets): See one of the parent functions.
         known_variables (dict): Make variables (probably from toolchains) to substitute in when doing make variable expansion.
 
@@ -272,16 +272,16 @@ def _expand_location_for_build_script_runner(ctx, env, data, known_variables):
     """
 
     # Fast-path - both location expansions and make vars have a `$` so we can short-circuit everything.
-    if "$" not in env:
-        return env
+    if "$" not in v:
+        return v
 
     for directive in ("$(execpath ", "$(location "):
-        if directive in env:
+        if directive in v:
             # build script runner will expand pwd to execroot for us
-            env = env.replace(directive, "$${pwd}/" + directive)
+            v = v.replace(directive, "$${pwd}/" + directive)
     return ctx.expand_make_variables(
-        env,
-        dedup_expand_location(ctx, env, data),
+        v,
+        ctx.expand_location(v, data),
         known_variables,
     )
 
@@ -315,7 +315,7 @@ def expand_dict_value_locations(ctx, env, data, known_variables):
     Returns:
         dict: A dict of environment variables with expanded location macros
     """
-    return dict([(k, _expand_location_for_build_script_runner(ctx, v, data, known_variables)) for (k, v) in env.items()])
+    return {k: _expand_location_for_build_script_runner(ctx, v, data, known_variables) for (k, v) in env.items()}
 
 def expand_list_element_locations(ctx, args, data, known_variables):
     """Performs location-macro expansion on a list of string values.

rust/toolchain.bzl

@@ -18,7 +18,7 @@ load(
 load(
     "//rust/private:utils.bzl",
     "dedent",
-    "dedup_expand_location",
+    "deduplicate",
     "find_cc_toolchain",
     "is_exec_configuration",
     "is_std_dylib",
@@ -534,9 +534,10 @@ def _experimental_use_cc_common_link(ctx):
     return ctx.attr.experimental_use_cc_common_link[BuildSettingInfo].value
 
 def _expand_flags(ctx, flags, targets):
+    targets = deduplicate(targets)
     expanded_flags = []
     for flag in flags:
-        expanded_flags.append(dedup_expand_location(ctx, flag, targets))
+        expanded_flags.append(ctx.expand_location(flag, targets))
     return expanded_flags
 
 def _rust_toolchain_impl(ctx):