Design: Symbolic Macros

[brandjon]

Status update, Oct 2024: This is implemented and will be released with Bazel 8.0.

Symbolic Macros are a proposed new feature to make macros easier to write and less brittle. I wrote a 2-page* doc summarizing their main properties. A formal design review will come in the future, after we are further along in the prototype.

* (If you don't count the appendix)

[alexeagle]

Feature request: I often use a macro to permit users to supply an "attribute" that accepts multiple types. The polymorphism makes a nicer developer experience: for example you can provide a label of a file, or give a list of strings or a dictionary that the macro automatically serializes with a write_file so the underlying rule only accepts a label of a file.
Would be nice if macro#attrs could express more than one attr.(type)

[brandjon]

It sounds like a nice convenience but I worry that it's scope creep, and that the feature can be justified for both macros and rules and therefore considered independently of this proposal.

Absent this feature, you should be able to achieve the same thing less conveniently by using optional attributes (passing default=...) and having the macro logic decide which attr to use based on sentinel values.

[comius]

May I propose that we consider such a feature in the design, but not in the initial implementation. That is, a possible future extension, which is not prevented by the first design/implementation we roll out. If this is possible. This way it wouldn't be a such a big scope creep.

[katre]

Side comment: can we get selectable = True/False in attr for regular rules? I've had several requests for this from rule authors recently (although I keep asking them to file feature requests and none do, so maybe it's not that necessary).

[brandjon]

I'd actually like that for consistency between attr descriptors used in rules and those used in symbolic macros. It's a little tricky because I can't imagine we'd make it a mandatory param for rule attrs (think of all the rule definitions that would need updating), but it'd be nice to make it mandatory for macros.

For rules, I can also imagine it affecting whether the attr is visible to certain types of Starlark-defined transitions that care about whether an attr is selectable.

[comius]

One thing that Jon told me once is that we should point the users in the right direction through APIs. Ideally in my opinion the right direction is that all label/dependency attributes are selectable. That would mean the API should be attr.label(legacy_unselectable=True).

I feel a bit different about for example bool types. It's more extraordinary they are selectable, so thus attr.bool(selectable = True) would signify the direction

[dgp1130]

This might be a scope increase / different problem, but one challenge I've seen is that macros as they exist today always "leak" their internal state. For example:

def my_binary(name):
    native.genrule(
        name = "%s_genrule" % name,
        outs = ["%s.sh" % name],
        # ...
    )

    native.sh_binary(
        name = name,
        srcs = ["%s.sh" % name],
        deps = ["//some:deps"],
    )

my_binary(name = "bin")

sh_binary(
    name = "other_bin",
    srcs = ["bin.sh"],
    deps = ["//other:deps"],
)

In this example, a user is calling my_binary, but then generating a completely different sh_binary using the generated .sh file, something which was likely intended to be a private implementation detail in the macro.

Personally I have a habit of accepting visibility into all my macros and only setting it on the "public" output targets. However that doesn't stop targets from within the same package depending on internals of a macro. I wonder if there's appetite for making symbolic macros actually restrict visibility for internal targets? This would require some mechanism with which to declare exactly which targets are the public outputs and I'm not 100% on what that looks like. Maybe you can specify visibility manually, but when not given any target defaults to "macro private" visibility, which is even more restrictive than the typical //visibility:private.

Related concern, what about passing through testonly, compatible_with, and restricted_to? Macros often fail to pass through these particular attributes to submacros and targets, which frequently lead to bugs. I wonder if we can do something similar there? In this case it might be an even simpler approach of just setting the default value of these attributes in targets to the given value of the symbolic macro invocation. Based on your example in the doc:

# //pkg/BUILD
load(“//lib:my_macros.bzl”, “my_validated_binary”)

my_validated_binary(
    name = "foo",
    testonly = True, # Can this implicitly mark `:main` and `:check` targets `testonly`?
)

If a submacro target is explicitly given a testonly, compatible_with, or restricted_to value, then that should override this default. For example:

def _my_validated_binary_impl(name, srcs, deps, create_test, test_runner):
    my_binary(
        name = "main",
        # No `testonly` set, defaults to `my_validated_binary`'s `testonly` value.
    )
    my_validation_test(
        name = "check",
        testonly = True, # Always `testonly = True`.
    )

Feel free to call this out of scope or not the problem at hand, but just a few thoughts of how we can solve some related macro problems with this approach.

[brandjon]

FYI, during the course of designing and implementing Symbolic Macros, we designed and implemented Macro-Aware Visibility, a system that makes the visibility check apply based on what (symbolic) macro declares the target rather than what package the target lives in. So your use case of private visibility is what the behavior will be, and in order to export a target for use outside the macro you would write visibility = visibility (giving the target the same visibility as was given to the macro).

Your other idea about common attributes like testonly is the subject of an upcoming feature we're planning, for symbolic macros to be able to inherit the attributes of the primary rule that they're wrapping. Stay tuned for that (but it won't be launched with Symbolic Macros in 8.0).

[dgp1130]

This might be too specific at this stage of feedback but I'll point out that symbolic macro implementation function in the example _impl puts arguments in a specific order as defined in the macro() dictionary which seems very brittle.

my_validated_binary = macro(
    # Syntax aligns with `rule`, makes sense.
    attrs = {
        "srcs": attr.label_list(selectable=True),
        "deps": attr.label_list(selectable=True),
        "create_test": attr.bool(selectable=False),
        "test_runner": attr.label(
            selectable=False, default="//lib:default_runner"),
    },
    # ...
)

# How is this order defined? Is it the source order of the `attrs` dictionary? What about when using tricks like
# `dict.update` or `**` expansion?
def _impl(name, srcs, deps, create_test, test_runner):
    # ...

Similar, what happens when I define a keyword value with a default?

def _impl(name, srcs, deps, create_test, test_runner = Label("//lib:default_runner")):
    # ...

That could be an error, but I think the fact that there's two reasonable-looking approaches to setting a default is inherently a design smell.

Since we're trying to follow existing rule conventions, I feel like we'll want some kind of loading_ctx value to represent all the inputs, rather than a list of arguments in the implementation function.

def _impl(loading_ctx):
    my_binary(name = “main”, srcs = loading_ctx.attr.srcs, deps = loading_ctx.attr.deps)
    if loading_ctx.attr.create_test:
        if loading_ctx.attr.test_runner in BANNED_RUNNERS:
            fail(“Please avoid banned test runners”)
        my_validation_test(name = “check”, srcs = loading_ctx.attr.srcs, deps = loading_ctx.attr.deps,
                           test_runner = loading_ctx.attr.test_runner)

It doesn't look great, and I do wish we could use arguments in _impl functions, but given the existing state of the world and its relationship to macro() attributes, I feel like this is closer to what the end result will actually look like?

[brandjon]

Thanks for the suggestions. We used attribute names for the params rather than a ctx object because it was more similar to how legacy macros are written today, and would therefore be more familiar to users.

The order of these parameters is unimportant because, despite being written conventionally as normal Starlark function parameters, they are only ever called by the internal machinery using keyword arguments, not positional arguments. So you could equally well write def _impl(*, name, src, [...]).

You can write a default for the param, and the effect is more or less identical to initializing it explicitly, just like any other Starlark function. However it's better practice to give the macro an implicit attribute (an attribute whose name is prefixed by an underscore) in its attrs schema. That'll enforce that the value seen by the implementation function is consistent with its attribute type, and make it clearer what the default value is for docgen purposes.

[comius]

Macros and queries (and maybe visibility):
For the first implementation/design it's probably best to just display all the targets in queries like before.

If the visibility separates public from private targets within a macro, possible future extension is a query operator, that would only display public targets of a macro - to reduce the complexity of the results. (proposal by @murali42)

[brandjon]

Sounds like bazel query 'visible(//pkg:BUILD, //pkg:*) (get all targets in //pkg visible to //pkg).

[murali42]

@dgp1130 FWIW I completely agree with macros and leaking their internal rules. There is an internal proposal around that (e.g. supporting things like //visibility:macro_internal). There are several nuances though

• Should it be visible to a specific macro instance?
• Should it be visible to all instances of that macro?
• What about friend macros (e.g. my_binary should be able to get access to all my_library generated rules)?
• If macro foo calls macro bar, should foo get access to internal rules of bar? Ideally no, but implementation could get tricky (especially if we want to keep memory usage down).

All that said... coming up with a solution for visibility could be done later?

[brandjon]

Closing the loop now that this is designed and implemented:

Granularity is at the package level, just like before, it's just that we check the package of the macro's code rather than the BUILD file that originated the call to the macro. That means no ability to discriminate all the way down to a single macro or a single instance of the macro.

Friends are declared on a per-target basis by writing visibility = visibility + ["//some_friend_package:__pkg__"] in the target declaration. Macros themselves do not declare friends that can see all their targets by default; if they want that, they have to manually ensure they pass the friend to the visibility of each thing they declare. However, targets are automatically visible to the place that instantiated them, so macros in the same package are automatically always friends.

bar's internals absolutely are protected from foo in the general case where they live in separate packages.

Credit to you, @murali42 for proposing a related visibility solution for legacy macros in the pre-symbolic-macro era.

[matts1]

FWIW, I think we should add macro_ctx, similar to how we have ctx, repository_ctx, and module_ctx. If we don't do it in the initial design, it's gonna cause problems down the line if we want to add it, and I can see a few use cases for it.

• macro_ctx.local_label("foo") to create labels to macro-local targets
• macro_ctx.prefix to learn the macro prefix
• macro_ctx.expose(label, friend_macro) to create "friend macros"
• Rule finalizers #23160 proposes native.existing_rules(). I'd personally prefer macro_ctx.existing_rules(), since I'm not a huge fan of adding functions to global scope if it can be avoided.

[brandjon]

native.existing_rules() isn't new to finalizers, it's ported over to it from legacy macros.

In general, the idea was to keep the API similar to legacy macros where possible for familiarity, and similar to rules where it helps with schema and regularity. Reusing the native methods was part of the former.

[kirkobyte]

Hello! Would this still be the best place to discuss symbolic macro design? I've been talking with @jbedard about what would be required to enable symbolic macros for lazy evaluation of node_modules within rules_js.

In the Bazel docs about Symbolic macros this limitation for names of targets/submacros is given:

The names of any targets or submacros created by a symbolic macro must either match the macro's name parameter or must be prefixed by name followed by _ (preferred), . or -. For example, my_macro(name = "foo") may only create files or targets named foo, or prefixed by foo_, foo- or foo., for example, foo_bar.

Would it be possible to also allow / as a valid separator here?

If I'm understanding the stated constraints as they are, these node_module references would need to become something like:

• :node_modules/@angular/core -> :node_modules.@angular/core
• :node_modules/react ->:node_modules.react
• or if using the preferred: :node_modules_react/:node_modules_@angular/core

If there's a specific reason we need to avoid allowing / between the symbolic macro and the underlying submacros/targets that's fair enough. But it would be fantastic to avoid needing to change these labels if possible

[fmeum]

@tetromino @brandjon