Skip to content

Commit

Permalink
Centrally handle Rust toolchain attr defaults
Browse files Browse the repository at this point in the history
Summary:
This reverts the Rust parts from D41714656 (f171bf1) and implements it better a different way.

Advantages of the new approach:

1. No more "falsiness". The default value kicks in only if a value was not passed by the code that instantiated the toolchain, or if `None` was passed. (As far as I can tell there is no way to tell these cases apart, but that seems fine.) Previously we had been doing e.g. `toolchain_info.rustc_flags or []`, which will silently accept a toolchain constructed with inappropriate falsey values like `rustc_flags = False` or `rustc_flags = ""`.

2. A consistent central place for the default values. No more needing to scatter `or []` to every location that a particular attribute is referenced.

3. A central place to observe which attributes have not yet been given a default value, and discuss what those default values should be.

Other than #1 above, this diff does not intentionally change any behavior.

Reviewed By: zertosh

Differential Revision: D41752388

fbshipit-source-id: 47e8b290cc596528a7a3c5b3a68195083725f8bd
  • Loading branch information
David Tolnay authored and facebook-github-bot committed Dec 6, 2022
1 parent eb11457 commit f8afac6
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 28 deletions.
9 changes: 4 additions & 5 deletions prelude/rust/build.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def generate_rustdoc(
[cmd_args("--env=", k, "=", v, delimiter = "") for k, v in plain_env.items()],
[cmd_args("--path-env=", k, "=", v, delimiter = "") for k, v in path_env.items()],
toolchain_info.rustdoc,
toolchain_info.rustdoc_flags or [],
toolchain_info.rustdoc_flags,
ctx.attrs.rustdoc_flags,
common_args.args,
# FIXME: fbcode/common/rust/rustdoc/compress/main.rs expects
Expand Down Expand Up @@ -403,8 +403,7 @@ def _dependency_args(

return (args, crate_targets)

def _lintify(flag: str.type, clippy: bool.type, lints: [["resolved_macro"], None]) -> "cmd_args":
lints = lints or []
def _lintify(flag: str.type, clippy: bool.type, lints: ["resolved_macro"]) -> "cmd_args":
return cmd_args(
[lint for lint in lints if str(lint).startswith("\"clippy::") == clippy],
format = "-{}{{}}".format(flag),
Expand Down Expand Up @@ -487,8 +486,8 @@ def _compute_common_args(
"--json=diagnostic-rendered-ansi",
["-Cprefer-dynamic=yes"] if crate_type == CrateType("dylib") else [],
["--target={}".format(toolchain_info.rustc_target_triple)] if toolchain_info.rustc_target_triple else [],
toolchain_info.rustc_flags or [],
(toolchain_info.rustc_check_flags or []) if is_check else [],
toolchain_info.rustc_flags,
toolchain_info.rustc_check_flags if is_check else [],
ctx.attrs.rustc_flags,
cmd_args(ctx.attrs.features, format = '--cfg=feature="{}"'),
dependency_args,
Expand Down
2 changes: 1 addition & 1 deletion prelude/rust/rust_binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def _rust_binary_common(
shared_libs,
)

extra_flags = (toolchain_info.rustc_binary_flags or []) + (extra_flags or [])
extra_flags = toolchain_info.rustc_binary_flags + (extra_flags or [])

# Compile rust binary.
link, meta = rust_compile_multi(
Expand Down
54 changes: 32 additions & 22 deletions prelude/rust/rust_toolchain.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -5,60 +5,70 @@
# License, Version 2.0 found in the LICENSE-APACHE file in the root directory
# of this source tree.

RustToolchainInfo = provider(fields = [
# @unsorted-dict-items
_rust_toolchain_attrs = {
# Report unused dependencies
"report_unused_deps",
"report_unused_deps": None,
# Rustc target triple to use
# https://doc.rust-lang.org/rustc/platform-support.html
"rustc_target_triple",
"rustc_target_triple": None,
# Baseline compiler config
"rustc_flags",
"rustc_flags": [],
# Extra flags when building binaries
"rustc_binary_flags",
"rustc_binary_flags": [],
# Extra flags for doing check builds
"rustc_check_flags",
"rustc_check_flags": [],
# Extra flags for doing building tests
"rustc_test_flags",
"rustc_test_flags": [],
# Extra flags for rustdoc invocations
"rustdoc_flags",
"rustdoc_flags": [],
# Use rmeta for lib->lib dependencies, and only block
# linking on rlib crates. The hope is that rmeta builds
# are quick and this increases effective parallelism.
# Currently blocked by https://github.com/rust-lang/rust/issues/85401
"pipelined",
"pipelined": None,
# Filter out failures when we just need diagnostics. That is,
# a rule which fails with a compilation failure will report
# success as an RE action, but a "failure filter" action will
# report the failure if some downstream action needs one of the
# artifacts. If all you need is diagnostics, then it will report
# success. This doubles the number of actions, so it should only
# be explicitly enabled when needed.
"failure_filter",
"failure_filter": None,
# The Rust compiler (rustc)
"compiler",
"compiler": None,
# Rust documentation extractor (rustdoc)
"rustdoc",
"rustdoc": None,
# Clippy (linter) version of the compiler
"clippy_driver",
"clippy_driver": None,
# Wrapper for rustc in actions
"rustc_action",
"rustc_action": None,
# Failure filter action
"failure_filter_action",
"failure_filter_action": None,
# The default edition to use, if not specified.
"default_edition",
"default_edition": None,
# Lints
"allow_lints",
"deny_lints",
"warn_lints",
"allow_lints": [],
"deny_lints": [],
"warn_lints": [],
# Prefix (/intern/rustdoc in our case) where fbcode crates' docs are hosted.
# Used for linking types in signatures to their definition in another crate.
"extern_html_root_url_prefix",
])
"extern_html_root_url_prefix": None,
}

RustToolchainInfo = provider(fields = _rust_toolchain_attrs.keys())

# Stores "platform"/flavor name used to resolve *platform_* arguments
RustPlatformInfo = provider(fields = [
"name",
])

def ctx_toolchain_info(ctx: "context") -> "RustToolchainInfo":
return ctx.attrs._rust_toolchain[RustToolchainInfo]
toolchain_info = ctx.attrs._rust_toolchain[RustToolchainInfo]

attrs = dict()
for k, default in _rust_toolchain_attrs.items():
v = getattr(toolchain_info, k)
attrs[k] = default if v == None else v

return RustToolchainInfo(**attrs)

0 comments on commit f8afac6

Please sign in to comment.