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

Stabilize extended_key_value_attributes #83366

Merged
merged 1 commit into from
May 19, 2021

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Mar 22, 2021

Closes #44732. Closes #78835. Closes #82768 (by making it irrelevant).

Stabilization report

Summary

This stabilizes using macro expansion in key-value attributes, like so:

#[doc = include_str!("my_doc.md")]
struct S;

Eager macro expansion only happens in a limited number of built-in attributes, with #[doc] being the primary example that will benefit from this. Notably, the attributes path, crate_type, and recursion_limit do not support this because they need their values before or during expansion. It is also not supported with proc-macros.

See Petrochenkov's excellent blog post on internals
for alternatives that were considered and rejected ("why accept no more and no less?")

This has been available on nightly since 1.50 with no major issues.

Notes

Accepted syntax

The parser accepts arbitrary Rust expressions in this position, but any expression other than a macro invocation will ultimately lead to an error because it is not expected by the built-in expression forms (e.g., #[doc]). Note that decorators and the like may be able to observe other expression forms.

Expansion ordering

Expansion of macro expressions in "inert" attributes occurs after decorators have executed, analogously to macro expressions appearing in the function body or other parts of decorator input.

There is currently no way for decorators to accept macros in key-value position if macro expansion must be performed before the decorator executes (if the macro can simply be copied into the output for later expansion, that can work).

Test cases

The feature has also been dogfooded extensively in the compiler and
standard library:

Implementation history

Unresolved Questions

#83366 (comment) listed some concerns, but they have been resolved as of this final report.

Additional Information

There are two workarounds that have a similar effect for #[doc]
attributes on nightly. One is to emulate this behavior by using a limited version of this feature that was stabilized for historical reasons:

macro_rules! forward_inner_docs {
    ($e:expr => $i:item) => {
        #[doc = $e]
        $i
    };
}

forward_inner_docs!(include_str!("lib.rs") => struct S {});

The other is to use doc(include):

 #![feature(external_doc)]
 #[doc(include = "lib.rs")]
 struct S {}

The first works, but is non-trivial for people to discover, and
difficult to read and maintain. The second is a strange special-case for
a particular use of the macro. This generalizes it to work for any use
case, not just including files.

I plan to remove doc(include) when this is stabilized
(#82539). The forward_inner_docs
workaround will still compile without warnings, but I expect it to be
used less once it's no longer necessary.

@jyn514 jyn514 added A-attributes Area: Attributes (`#[…]`, `#![…]`) A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) T-lang Relevant to the language team, which will review and decide on the PR/issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. labels Mar 22, 2021
@rust-highfive
Copy link
Collaborator

r? @steveklabnik

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 22, 2021
@jyn514
Copy link
Member Author

jyn514 commented Mar 22, 2021

r? @petrochenkov (forgot to put it in the PR description, sorry).

I added T-lang and T-compiler since I think they may need to both sign off on the FCP? Feel free to remove one or the other if necessary.

@jyn514 jyn514 added the F-extended_key_value_attributes `#![feature(extended_key_value_attributes)] label Mar 22, 2021
@jyn514 jyn514 force-pushed the stabilize-key-value-attrs branch from a18c9bc to dc408e2 Compare March 22, 2021 05:48
@jyn514 jyn514 force-pushed the stabilize-key-value-attrs branch from dc408e2 to d9e7d25 Compare March 22, 2021 06:13
@pksunkara
Copy link
Contributor

Stabilizing this would not make #82768 irrelevant because of people trying to provide backward compatibility.

Clap's MSRV is 1.46.0. We would like to keep it the same and use the doc feature flag to conditionally enable the doc = include_str! when building docs. That way, MSRV is bumped to 1.53.0 only when building the docs.

@jonas-schievink jonas-schievink added the relnotes Marks issues that should be documented in the release notes of the next release. label Mar 22, 2021
@jyn514
Copy link
Member Author

jyn514 commented Mar 22, 2021

Clap's MSRV is 1.46.0. We would like to keep it the same and use the doc feature flag to conditionally enable the doc = include_str! when building docs. That way, MSRV is bumped to 1.53.0 only when building the docs.

@pksunkara see the workaround in #82768 (comment).

@camelid
Copy link
Member

camelid commented Mar 22, 2021

Closes #44732.

I don't think this should close that issue since this PR does not stabilize, deprecate, or remove doc(include). #82539 should be the one to close that issue.

@bors

This comment has been minimized.

@petrochenkov
Copy link
Contributor

One is to emulate this behavior by using eager
expansion

This is not eager expansion, this is a limited version of the same extended_key_value_attributes that exists for historical reasons.

@petrochenkov
Copy link
Contributor

petrochenkov commented Mar 23, 2021

There are two questions to answer/acknowledge before stabilizing this:

  • Whether to stabilize #[attr = ARBITRARY_EXPRESSION] or macro calls only #[attr = mac::call!(a b c)].

    If arbitrary expressions are stabilized, then things like

    #[attr = {
        let a = 10;
        println!("{}", a + 11);
    }]

    become legal as well, but only syntactically (under cfg(FALSE)), they will still be rejected semantically.

    Even if we only stabilize macros, right now the compiler has to know that mac::call!(a b c) is an expression macro to expand it, but it's not something fundamental, we may be able to expand macros in context independent way in the future.

  • Relative order of expanding macros in #[attr = mac!()] and in other places.

    Right now mac!() is expanded last of all the macro invocations below

    #[macro_attr1]
    #[doc = mac!()]
    #[macro_attr2]
    #[derive(MacroDerive1, MacroDerive2)]
    struct S;
    
    bar!();

    and not between macro_attr1 and macro_attr2 as some people could expect.

    However, this falls under the general rule telling that the expansion order of macros applied to different targets is unspecified, i.e. macro_attr1 and macro_attr2 are applied to the same target struct S so they are ordered, but mac!() is an entirely separate invocation similarly to e.g. bar!().
    The problem is only that the expansion order is easily discoverable by macro_attr(1,2) and MacroDerive(1,2) here, so it becomes fixed in practice.

    Expanding mac!() in a way making its result visible to macro_attr2 should be somewhat annoying from the implementation point of view (I didn't investigate details).

@petrochenkov petrochenkov added I-nominated S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 23, 2021
@camelid
Copy link
Member

camelid commented Mar 23, 2021

  • Expanding mac!() in a way making its result visible to macro_attr2 should be somewhat annoying from the implementation point of view (I didn't investigate details).

Do you mean macro_attr1 or do attribute macros evaluate from the outermost to the innermost? I would have assumed macro_attr1 would be applied on the result of doc; doc on the result of macro_attr; and macro_attr2 on the result of derive.

@petrochenkov
Copy link
Contributor

@camelid
Macros on the same target are expanded in the left-to-right order, so #[macro_attr1] is run first and cannot see results of anything after it.

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Aug 12, 2021
Pkgsrc changes:
 * Bump bootstrap requirements to 1.53.0.
 * Adjust patches, adapt to upstream changes, adjust cargo checksums
 * If using an external llvm, require >= 10.0

Upsteream changes:

Version 1.54.0 (2021-07-29)
============================

Language
-----------------------

- [You can now use macros for values in built-in attribute macros.][83366]
  While a seemingly minor addition on its own, this enables a lot of
  powerful functionality when combined correctly. Most notably you can
  now include external documentation in your crate by writing the following.
  ```rust
  #![doc = include_str!("README.md")]
  ```
  You can also use this to include auto-generated modules:
  ```rust
  #[path = concat!(env!("OUT_DIR"), "/generated.rs")]
  mod generated;
  ```

- [You can now cast between unsized slice types (and types which contain
  unsized slices) in `const fn`.][85078]
- [You can now use multiple generic lifetimes with `impl Trait` where the
   lifetimes don't explicitly outlive another.][84701] In code this means
   that you can now have `impl Trait<'a, 'b>` where as before you could
   only have `impl Trait<'a, 'b> where 'b: 'a`.

Compiler
-----------------------

- [Rustc will now search for custom JSON targets in
  `/lib/rustlib/<target-triple>/target.json` where `/` is the "sysroot"
  directory.][83800] You can find your sysroot directory by running
  `rustc --print sysroot`.
- [Added `wasm` as a `target_family` for WebAssembly platforms.][84072]
- [You can now use `#[target_feature]` on safe functions when targeting
  WebAssembly platforms.][84988]
- [Improved debugger output for enums on Windows MSVC platforms.][85292]
- [Added tier 3\* support for `bpfel-unknown-none`
   and `bpfeb-unknown-none`.][79608]

\* Refer to Rust's [platform support page][platform-support-doc] for more
   information on Rust's tiered platform support.

Libraries
-----------------------

- [`panic::panic_any` will now `#[track_caller]`.][85745]
- [Added `OutOfMemory` as a variant of `io::ErrorKind`.][84744]
- [ `proc_macro::Literal` now implements `FromStr`.][84717]
- [The implementations of vendor intrinsics in core::arch have been
   significantly refactored.][83278] The main user-visible changes are
   a 50% reduction in the size of libcore.rlib and stricter validation
   of constant operands passed to intrinsics. The latter is technically
   a breaking change, but allows Rust to more closely match the C vendor
   intrinsics API.

Stabilized APIs
---------------

- [`BTreeMap::into_keys`]
- [`BTreeMap::into_values`]
- [`HashMap::into_keys`]
- [`HashMap::into_values`]
- [`arch::wasm32`]
- [`VecDeque::binary_search`]
- [`VecDeque::binary_search_by`]
- [`VecDeque::binary_search_by_key`]
- [`VecDeque::partition_point`]

Cargo
-----

- [Added the `--prune <spec>` option to `cargo-tree` to remove a package from
  the dependency graph.][cargo/9520]
- [Added the `--depth` option to `cargo-tree` to print only to a certain depth
  in the tree ][cargo/9499]
- [Added the `no-proc-macro` value to `cargo-tree --edges` to hide procedural
  macro dependencies.][cargo/9488]
- [A new environment variable named `CARGO_TARGET_TMPDIR` is
  available.][cargo/9375]
  This variable points to a directory that integration tests and
  benches can use as a "scratchpad" for testing filesystem operations.

Compatibility Notes
-------------------
- [Mixing Option and Result via `?` is no longer permitted in
  closures for inferred types.][86831]
- [Previously unsound code is no longer permitted where different
  constructors in branches could require different lifetimes.][85574]
- As previously mentioned the [`std::arch` instrinsics now uses
  stricter const checking][83278] than before and may reject some
  previously accepted code.
- [`i128` multiplication on Cortex M0+ platforms currently
  unconditionally causes overflow when compiled with `codegen-units
  = 1`.][86063]

[85574]: rust-lang/rust#85574
[86831]: rust-lang/rust#86831
[86063]: rust-lang/rust#86063
[86831]: rust-lang/rust#86831
[79608]: rust-lang/rust#79608
[84988]: rust-lang/rust#84988
[84701]: rust-lang/rust#84701
[84072]: rust-lang/rust#84072
[85745]: rust-lang/rust#85745
[84744]: rust-lang/rust#84744
[85078]: rust-lang/rust#85078
[84717]: rust-lang/rust#84717
[83800]: rust-lang/rust#83800
[83366]: rust-lang/rust#83366
[83278]: rust-lang/rust#83278
[85292]: rust-lang/rust#85292
[cargo/9520]: rust-lang/cargo#9520
[cargo/9499]: rust-lang/cargo#9499
[cargo/9488]: rust-lang/cargo#9488
[cargo/9375]: rust-lang/cargo#9375
[`BTreeMap::into_keys`]: https://doc.rust-lang.org/std/collections/struct.BTreeMap.html#method.into_keys
[`BTreeMap::into_values`]: https://doc.rust-lang.org/std/collections/struct.BTreeMap.html#method.into_values
[`HashMap::into_keys`]: https://doc.rust-lang.org/std/collections/struct.HashMap.html#method.into_keys
[`HashMap::into_values`]: https://doc.rust-lang.org/std/collections/struct.HashMap.html#method.into_values
[`arch::wasm32`]: https://doc.rust-lang.org/core/arch/wasm32/index.html
[`VecDeque::binary_search`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.binary_search
[`VecDeque::binary_search_by`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.binary_search_by
[`VecDeque::binary_search_by_key`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.binary_search_by_key
[`VecDeque::partition_point`]: https://doc.rust-lang.org/std/collections/struct.VecDeque.html#method.partition_point
@webern
Copy link

webern commented Aug 12, 2021

I came here from the release notes for 1.54. Does this feature work for derive macros? It doesn't seem like it does. I'm trying something that looks like this:

#[some_derive_macro(some_attribute = myfoo!())]
struct Whatever {
    
}

Which does not work with Unable to parse attribute: expected literal.

What I actually want is this: #52393

And would look like this:

const MY_FOO: &str = "bloop";

#[some_derive_macro(some_attribute = MY_FOO)]
struct Whatever {
    
}

I thought I might be able to do a workaround using the new 1.54 feature:

macro_rules identity! { ($x:expr) => { $x }; }

const MY_FOO: &str = "bloop";

#[some_derive_macro(some_attribute = identity!(MY_FOO))]
struct Whatever {
    
}

Alas, I was naive.

Is there any chance that derive macros can get the same love as proc macros so that we can use macros (well, actually constants) as values in derive macro attribute values?

@jplatte
Copy link
Contributor

jplatte commented Aug 13, 2021

This is not specifically about proc-macros: #78835 (comment)

@webern
Copy link

webern commented Aug 13, 2021

This is not specifically about proc-macros: #78835 (comment)

So if I read that correctly, this is supported:

#[some_derive_macro = something!()]
struct X {}

Bot not this:

#[some_derive_macro(attribute1 = something!())]
struct X {}

Is that what #78835 (comment) is saying?

Thanks.

@jyn514
Copy link
Member Author

jyn514 commented Aug 13, 2021

@webern Yes.

@michalfita
Copy link

@jyn514 but that doesn't:

#[path = concat!(env!("OUT_DIR"), "/lib.rs")]

danielocfb pushed a commit to danielocfb/libbpf-bootstrap that referenced this pull request Jan 12, 2024
Store skeletons in $OUT_DIR as opposed to .output, which is kind of the
proper thing to do (though it certainly doesn't help
discoverability...) and it is more in line with how examples in
libbpf-rs work. Also remove the "there is hope!" comments, because even
with rust-lang/rust#83366 resolved we still
cannot use the concat! macro in #[path = ...] attributes. All hope is
lost.

Signed-off-by: Daniel Müller <deso@posteo.net>
danielocfb pushed a commit to danielocfb/libbpf-bootstrap that referenced this pull request Jan 12, 2024
Store skeletons in $OUT_DIR as opposed to .output, which is kind of the
proper thing to do (though it certainly doesn't help
discoverability...) and it is more in line with how examples in
libbpf-rs work. Also remove the "there is hope!" comments, because even
with rust-lang/rust#83366 resolved we still
cannot use the concat! macro in #[path = ...] attributes. All hope is
lost.

Signed-off-by: Daniel Müller <deso@posteo.net>
anakryiko pushed a commit to libbpf/libbpf-bootstrap that referenced this pull request Jan 22, 2024
Store skeletons in $OUT_DIR as opposed to .output, which is kind of the
proper thing to do (though it certainly doesn't help
discoverability...) and it is more in line with how examples in
libbpf-rs work. Also remove the "there is hope!" comments, because even
with rust-lang/rust#83366 resolved we still
cannot use the concat! macro in #[path = ...] attributes. All hope is
lost.

Signed-off-by: Daniel Müller <deso@posteo.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. F-extended_key_value_attributes `#![feature(extended_key_value_attributes)] finished-final-comment-period The final comment period is finished for this PR / Issue. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet