attr.string_list_dict doesn't resolve select() calls to values before type checking.

[jmillikin-stripe]

Given a rule with an attribute defined by attr.string_list_dict(), I'd expect the following two stanzas to be equivalent:

foo(my_attr = {"hello": ["world"]})

foo(
  my_attr = {"hello": select({
    "//conditions:default": ["world"],
  })},
)

However, Bazel rejects the second with a type error:

expected value of type 'list(string)' for dict value element, but got select({"//conditions:default": []}) (select)

[buchgr]

@laurentlb can you take a look please?

[gregestren]

Hi,

Unfortunately select doesn't support this kind of composition.

At core, select is designed to let you custom-choose the value an attribute takes. This means the select's type has to match the attribute's type: in this case a dictionary of string lists.

In your example, the select is bound to a string list, which is not the same type. Hence the error.

While your request makes sense as a user, it requires decomposing Bazel's type system more deeply than that system supports. So we'd basically have to refactor the type system to make this work.

We could theoretically add ad hoc support for dictionary values to make this work (similar to how + works), or apply a filter that propagates selects to the top-level, e.g. auto-translates your example to:

foo(
  my_attr = select({
    "//conditions:default": {"hello": ["world"]}
  }),
)

In this modified example, the select is now bound to a string list dict, so it now matches my_attr's type.

So these are potential paths forward but I suspect it'd be messy.

[gregestren]

Most importantly, do you have workarounds with the status quo?

[jmillikin-stripe]

The workaround is to copy-paste the non-selected part of the dict into multiple select cases. It's not pretty, but is tolerable at our config sizes.

Given that the attr module has a fixed set of allowed container types, is it practical to special-case select() when used as a key or value of those containers? For example, string_list_dict attrs must be of type Dict<String, List String>> so there's only so many ways a select could get in there.

[gregestren]

I think the challenge is that the part of the type system in the List String subtype would have to be aware of selects and know how to "pass them up" to the final attribute assignment. So code paths crossing multiple classes would have to synchronize more tightly.

Another concern is nesting selects across multiple types, i.e.:

foo(
  my_attr = select({
    "//conditions:default": {
      "hello": select({
        "//conditions:default": ["world"]}}
  }),
)

So resolving this attribute would require evaluating two selects over different types. This would effectively defer type resolution to a far later phase in Blaze's build process - a phase that's not equipped for that kind of thing now. But I'm not sure how strong the call is for mixing selects this way, so maybe we can ignore this concern.

[kastiglione]

We ran into this, wanted to have a string_dict attr whose values could be configurable. For example:

my_attr = {
    "key": "A",
    "other_key": select({
        ":config": "B",
        "//conditions:default": "C",
    }),
}

I ended up on a solution that does provide this interface, but complicates the underlying implementation. The approach is to add a macro layer above the rule (which generally wouldn't be used directly). The macro destructures the dictionary into a list of keys, and a list of values. It takes advantage of + working across list values and select values (as mentioned above).

For example, this dict:

my_attr = {
    "A": {
        ":X", "aX",
        ":Y", "aY,
    },
    "B": "b",
}

is transformed in into:

keys = ["A", "B"]
values = select({":X": ["aX"], ":Y": ["aY"]}) + ["b"]

The macro then calls the rule, which has an attr for the keys, and an attr for the values. The rules's implementation can then easily recompose the dictionary from list of keys and the (now resolved) list of values, using dict(zip(ctx.attr.keys, ctx.attr.values)).

[gregestren]

Also FYI I prototyped nested selects in #1623 earlier this year. I can't remember if it handled different types though, as requested here.

[gregestren]

I think the most practical solution remains https://github.com/bazelbuild/bazel-skylib/blob/master/lib/selects.bzl, since it doesn't deeply mess with Starlark's type system.

Also see https://docs.bazel.build/versions/master/configurable-attributes.html#or-chaining and the following section.

Going forward, I suggest criteria for opening new bugs includes demonstrating how the skylib approach is insufficient and really evaluating what kind of organization with existing mechanisms is possible before considering enhancements. We really want to lean away from direct support for syntax like described here, even though I get the intuition from the user side (enough that I even prototyped it!).