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

How to treat inert attributes on macro invocations? #63221

Open
petrochenkov opened this issue Aug 2, 2019 · 8 comments
Open

How to treat inert attributes on macro invocations? #63221

petrochenkov opened this issue Aug 2, 2019 · 8 comments
Assignees
Labels
A-frontend Area: frontend (errors, parsing and HIR) 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.

Comments

@petrochenkov
Copy link
Contributor

petrochenkov commented Aug 2, 2019

Examples of inert attributes on macro invocations:

#[inert]
bang_macro!();

#[inert]
#[attr_macro]
struct S;

#[inert]
#[derive(DeriveMacro)]
struct S;

// Doc comments are also attributes (attribute literals if you wish).

/// Doc.
bang_macro!();

/// Doc.
#[attr_macro]
struct S;

/// Doc.
#[derive(DeriveMacro)]
struct S;

How these attributes are treated currently (ad hoc, there's no RFC or anything):

  • For bang macros the attributes are thrown away (sometimes with a warning).
  • For attribute macros and derive macros the attributes become a part of the macro input, the attribute tokens are prepended to the tokens of the "primary" input.
    Effectively, #[inert] #[macro_attr] struct S; -> macro_attr! { #[inert] struct S; }.

Related issues: #61733 (comment).

@petrochenkov petrochenkov added A-frontend Area: frontend (errors, parsing and HIR) 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. labels Aug 2, 2019
@petrochenkov
Copy link
Contributor Author

Ideally I'd want a common scheme for all macro kinds + a scheme that's more intuitive and/or resistant to mistakes.
As recent improvements to the unused_doc_comments (#57882) showed, people somewhat often expect

/// Doc.
mac! {
    struct S;
}

to translate into

/// Doc.
struct S;

, so the current approach of throwing the attributes away may be not the best one.

@petrochenkov
Copy link
Contributor Author

Alternative 1: Throwing the attributes away.

  • #[inert] bang_macro!() -> bang_macro!()
  • #[inert] #[attr_macro] struct S; -> #[attr_macro] struct S;
  • #[inert] #[derive(DeriveMacro)] struct S; -> #[derive(DeriveMacro)] struct S;

This alternative seems non-viable for attributes/derives.
It's very idiomatic to write doc comments (which are inert attributes) before any other attributes

/// Doc.
#[some_attrs]
#[derive(SomeDerives)]
struct S;

and the doc comments must not be lost during expansion in this case.

For bang macros this is probably undesirable as well (#63221 (comment)), this leads to users writing code that most likely doesn't do what they expect.

@petrochenkov
Copy link
Contributor Author

Alternative 2: Prepending the attributes to the macro output.

This alternative basically follows the token-based expansion model described in #61733 (comment).

Suppose our macro is derive-like and emit the input item (after perhaps tweaking it slightly) + some additional items like impls.

mac! { struct S; }

=>

pub struct S;
impl S { ... }

In this case the behavior is pretty reasonable, #[inert] mac! { struct S; } becomes #[inert] pub struct S;.

It's less reasonable if the macro doesn't have a "primary" item, e.g.

#[inert]
mac!();

=>

#[inert]
struct U;

struct V;

struct W;

, in this case the macro may want to "spread" the attribute across all the produced items.

Note that for post-fix semicolons appending them in accordance with the token-based model is almost always the desired behavior that turns the final expression produced by the macro into a non-trailing statement or has no effect.

Migrating to this model would be a breaking change for attribute and derive macros, we can estimate the scale of the breakage with crater.
It's expected to be somewhat sizeable because inert attributes are commonly used before macro attributes and derives.

@petrochenkov
Copy link
Contributor Author

petrochenkov commented Aug 2, 2019

Alternative 3: Prepending the attributes to the macro input.

This is the most flexible alternative - let the macro itself to decide what to do with the attributes.
Attribute and derive macros use this approach currently.

In case of derive-like macros it works somewhat automatically, similarly to the alternative 2.

#[inert]
mac! { struct S; }

=>

mac! { #[inert] struct S; }

=>

#[inert]
pub struct S;
impl S { ... }

In other cases the macro may implement the spreading semantics or some other semantics itself.

Migrating to this model would be a breaking change for fn-like macros, we can estimate the scale of the breakage with crater.
I'd expect the breakage in this case to be more limited compared to the alternative 2.
The first reason for this is that inert attributes on fn-like macro calls are currently thrown away and sometimes warned about, so they are expected to be rare and written accidentally.
The second reason is that if something like

/// Doc.
mac! {
    struct S;
}

is written accidentally, then it's likely that the macro will expect an item and accept

mac! {
    /// Doc.
    struct S;
}

as well, so in some cases the change may fix the code rather than break it.

We can analyze the breakage and perhaps add some compatibility hacks if necessary (until the next edition or something).

@Centril
Copy link
Contributor

Centril commented Aug 2, 2019

I tend to agree that alternative 3 seems best here.

@Centril
Copy link
Contributor

Centril commented Aug 2, 2019

Plausibly relevant: It would be neat if when given #[derive(Foo, Bar)] the derive macros would know of each other.

@dtolnay
Copy link
Member

dtolnay commented Mar 16, 2020

I think it's worth considering the following fourth approach, not that I necessarily think it's the best one but it is missing above.

Alternative 4: Prepending outer attributes as inner

#[inert]
mac! {
    struct S;
}

=>

mac! {
    #![inert]

    struct S;
}

This is analogous to how #[attr] mod m {}mod m { #![attr] } are interchangeable.

Compared to alternative 3, this is less likely to make existing proc macros "work by accident" but it enables macros to define the semantics of outer attributes on their invocation.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jun 11, 2020
Suggest including unused asm arguments in a comment to avoid error

We require all arguments to an `asm!` to be used in the template string, just like format strings. However in some cases (e.g. `black_box`) it may be desirable to have `asm!` arguments that are not used in the template string.

Currently this is a hard error rather than a lint since `#[allow]` does not work on macros (rust-lang#63221), so this PR suggests using the unused arguments in an asm comment as a workaround.

r? @petrochenkov
@Aaron1011
Copy link
Member

For macro_rules! macros, we currently throw away attributes:

macro_rules! make_fn {
    () => {
        fn foo() {}
    }
}

fn main() {
    #[allow(dead_code)] make_fn!();
}

In the above code, we'll end up with fn main() { fn foo() {} }

I think it would be useful to handle attributes equivalently for macro_rules! and bang-proc-macros.

Aaron1011 added a commit to Aaron1011/rust that referenced this issue Jul 19, 2021
These attributes are currently discarded.
This may change in the future (see rust-lang#63221), but for now,
placing inert attributes on a macro invocation does nothing,
so we should warn users about it.

Technically, it's possible for there to be attribute macro
on the same macro invocation (or at a higher scope), which
inspects the inert attribute. For example:

```rust
#[look_for_inline_attr]
#[inline]
my_macro!()

#[look_for_nested_inline]
mod foo { #[inline] my_macro!() }
```

However, this would be a very strange thing to do.
Anyone running into this can manually suppress the warning.
LeSeulArtichaut added a commit to LeSeulArtichaut/rust that referenced this issue Jul 24, 2021
Warn on inert attributes used on bang macro invocation

These attributes are currently discarded.
This may change in the future (see rust-lang#63221), but for now,
placing inert attributes on a macro invocation does nothing,
so we should warn users about it.

Technically, it's possible for there to be attribute macro
on the same macro invocation (or at a higher scope), which
inspects the inert attribute. For example:

```rust
#[look_for_inline_attr]
#[inline]
my_macro!()

#[look_for_nested_inline]
mod foo { #[inline] my_macro!() }
```

However, this would be a very strange thing to do.
Anyone running into this can manually suppress the warning.
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 24, 2021
Warn on inert attributes used on bang macro invocation

These attributes are currently discarded.
This may change in the future (see rust-lang#63221), but for now,
placing inert attributes on a macro invocation does nothing,
so we should warn users about it.

Technically, it's possible for there to be attribute macro
on the same macro invocation (or at a higher scope), which
inspects the inert attribute. For example:

```rust
#[look_for_inline_attr]
#[inline]
my_macro!()

#[look_for_nested_inline]
mod foo { #[inline] my_macro!() }
```

However, this would be a very strange thing to do.
Anyone running into this can manually suppress the warning.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-frontend Area: frontend (errors, parsing and HIR) 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.
Projects
None yet
Development

No branches or pull requests

4 participants