Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rust_expand and rust_expand_aspect for generating macro expanded source files. #1643

Closed
wants to merge 1 commit into from

Conversation

freeformstu
Copy link
Contributor

@freeformstu freeformstu commented Nov 9, 2022

Proposal for #1642

Summary

Rustc can be used to expand all macros so that you can inspect the generated source files easier.

This feature is enabled via -Zunpretty=expanded. The -Z flag is only available in the nightly
version of rustc.

Expanding

Build and test your targets normally.

bazel build //:ok_binary   
INFO: Analyzed target //:ok_binary (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:ok_binary up-to-date:
  bazel-bin/ok_binary
INFO: Elapsed time: 0.081s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action

Use the aspect to generate the expanded files in as a one-off build.

bazel build --config=expanded //:ok_binary
INFO: Analyzed target //:ok_binary (1 packages loaded, 2 targets configured).
INFO: Found 1 target...
Aspect @rules_rust//rust/private:expand.bzl%rust_expand_aspect of //:ok_binary up-to-date:
  bazel-bin/ok_binary.expand.rs
INFO: Elapsed time: 0.149s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action

Targeting tests is valid as well.

bazel build --config=expanded //:ok_test  
INFO: Analyzed target //:ok_test (0 packages loaded, 2 targets configured).
INFO: Found 1 target...
Aspect @rules_rust//rust/private:expand.bzl%rust_expand_aspect of //:ok_test up-to-date:
  bazel-bin/test-397521499/ok_test.expand.rs
INFO: Elapsed time: 0.113s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action

Finally, manually wire up a rust_expand target explicitly if you want a target to build.

load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_expand")

rust_binary(
    name = "ok_binary",
    srcs = ["src/main.rs"],
    edition = "2021",
)

rust_expand(
    name = "ok_binary_expand",
    deps = [":ok_binary"],
)
bazel build //:ok_binary_expand
INFO: Analyzed target //:ok_binary_expand (0 packages loaded, 1 target configured).
INFO: Found 1 target...
Target //:ok_binary_expand up-to-date:
  bazel-bin/ok_binary.expand.rs
INFO: Elapsed time: 0.090s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action

@google-cla
Copy link

google-cla bot commented Nov 9, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@freeformstu freeformstu force-pushed the dev/expand branch 2 times, most recently from ac82745 to 0706e35 Compare November 9, 2022 03:41
@UebelAndre UebelAndre self-requested a review November 11, 2022 03:00
@UebelAndre UebelAndre changed the title Add rust_expand and rust_expand_aspect for generating macro expanded source files. Add rust_expand and rust_expand_aspect for generating macro expanded source files. Dec 7, 2022
Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! I think this would be a set of very useful rules. Got some feedback for you.

Also, I'm not sure if this is something that should go in the rust package vs tools or util.
@freeformstu any strong feelings here?
@illicitonion @scentini any opinions?

@@ -411,6 +411,16 @@ tasks:
- "//..."
test_targets:
- "//..."
expand_examples_ubuntu2004:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding a new example, would you be willing to some integration tests for this in test

As of #1671 you should be able to gain access to a nightly toolchain in the main repo via a transition on @rules_rust//rust/toolchain/channel

),
default = Label("@bazel_tools//tools/cpp:current_cc_toolchain"),
),
"_extra_rustc_flag": attr.label(default = "//:extra_rustc_flag"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nit:

Suggested change
"_extra_rustc_flag": attr.label(default = "//:extra_rustc_flag"),
"_extra_rustc_flag": attr.label(
default = Label("//:extra_rustc_flag"),
),

default = Label("@bazel_tools//tools/cpp:current_cc_toolchain"),
),
"_extra_rustc_flag": attr.label(default = "//:extra_rustc_flag"),
"_extra_rustc_flags": attr.label(default = "//:extra_rustc_flags"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nit:

Suggested change
"_extra_rustc_flags": attr.label(default = "//:extra_rustc_flags"),
"_extra_rustc_flags": attr.label(
default = Label("//:extra_rustc_flags"),
),

# See the License for the specific language governing permissions and
# limitations under the License.

"""A module defining Rust expansion rules"""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you expand this module header a bit? I think it'd be nice to have some of the context currently in examples/expand/README.md here so we could render the docs for folks with the rest of rules_rust docs.


return target[rust_common.crate_info]

def _expand_aspect_impl(target, ctx):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nit:

Suggested change
def _expand_aspect_impl(target, ctx):
def _rust_expand_aspect_impl(target, ctx):

I think it's nice to have the rule name be in the implementation function name.
e.g. The implementation of rust_binary is _rust_binary_impl.

if crate_info.is_test:
args.rustc_flags.add("--test")

expand_out = ctx.actions.declare_file(ctx.label.name + ".expand.rs", sibling = crate_info.output)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is all source code expanded into one file?

)
```

Then the targets can be expanded with the following command:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you add a small snippet of how someone might inspect the generated source after running this?

@scentini
Copy link
Collaborator

What is the use case for the rust_expand rule? I would expect expanding the macros makes sense for debug purposes only, and for that the --config=expand should work well on the command line.

@UebelAndre
Copy link
Collaborator

Closing in favor of #2356

@UebelAndre UebelAndre closed this Dec 25, 2023
UebelAndre added a commit that referenced this pull request Dec 27, 2023
Proposal for #1642
Duplicates #1643 (special
thanks to @freeformstu)

### Summary

Rustc can be used to expand all macros so that you can inspect the
generated source files easier.

This feature is enabled via `-Zunpretty={mode}`. The `-Z` flag is only
available in the nightly
version of `rustc` (rust-lang/rust#43364).


### Unprettying

Build and test your targets normally.

```
bazel build //:ok_binary   
INFO: Analyzed target //:ok_binary (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:ok_binary up-to-date:
  bazel-bin/ok_binary
INFO: Elapsed time: 0.081s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
```

Use the aspect to generate the expanded files in as a one-off build.

(`.bazelrc`)
```
# Enable unpretty for all targets in the workspace
build:unpretty --aspects=@rules_rust//rust:defs.bzl%rust_unpretty_aspect
build:unpretty --output_groups=+rust_unpretty

# `unpretty` requires the nightly toolchain. See tracking issue:
# rust-lang/rust#43364
build:unpretty --@rules_rust//rust/toolchain/channel=nightly
```

```
bazel build --config=unpretty //:ok_binary
INFO: Analyzed target //:ok_binary (1 packages loaded, 2 targets configured).
INFO: Found 1 target...
Aspect @rules_rust//rust/private:unpretty.bzl%rust_unpretty_aspect of //:ok_binary up-to-date:
  bazel-bin/ok_binary.expand.rs
INFO: Elapsed time: 0.149s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
```

Targeting tests is valid as well.

```
bazel build --config=unpretty //:ok_test  
INFO: Analyzed target //:ok_test (0 packages loaded, 2 targets configured).
INFO: Found 1 target...
Aspect @rules_rust//rust/private:unpretty.bzl%rust_expand_aspect of //:ok_test up-to-date:
  bazel-bin/test-397521499/ok_test.expand.rs
INFO: Elapsed time: 0.113s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
```

Finally, manually wire up a `rust_unpretty` target explicitly if you
want a target to build. This rule is unique compared to the aspect in
that it forces a transition to a nightly toolchain so that `-Zunpretty`
can be used.

```starlark
load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_unpretty")

rust_binary(
    name = "ok_binary",
    srcs = ["src/main.rs"],
    edition = "2021",
)

rust_unpretty(
    name = "ok_binary_expand",
    deps = [":ok_binary"],
)
```

```
bazel build //:ok_binary_expand
INFO: Analyzed target //:ok_binary_expand (0 packages loaded, 1 target configured).
INFO: Found 1 target...
Target //:ok_binary_expand up-to-date:
  bazel-bin/ok_binary.expand.rs
INFO: Elapsed time: 0.090s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants