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

Tracking issue for custom inner attributes #54726

Open
alexcrichton opened this issue Oct 1, 2018 · 24 comments
Open

Tracking issue for custom inner attributes #54726

alexcrichton opened this issue Oct 1, 2018 · 24 comments
Assignees
Labels
A-attributes Area: #[attributes(..)] A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-proc-macros Area: Procedural macros B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. S-tracking-design-concerns Status: There are blocking ❌ design concerns. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@alexcrichton
Copy link
Member

alexcrichton commented Oct 1, 2018

Added in #54093 inner attributes are not allowed to be procedural macros right now, and this issue is tracking that feature of procedural macros!

Assistance in filling in this issue would be greatly appreciated!
UPDATE: #54726 (comment) below contains the detailed description.

@alexcrichton alexcrichton added 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. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Oct 1, 2018
@petrochenkov petrochenkov self-assigned this Oct 1, 2018
@dhardy
Copy link
Contributor

dhardy commented Oct 18, 2018

Summary:

error[E0658]: non-builtin inner attributes are unstable (see issue #54726)
 --> src/lib.rs:3:1
  |
3 | #![foo]
  | ^^^^^^^
  |
  = help: add #![feature(custom_inner_attributes)] to the crate attributes to enable

error[E0658]: The attribute `foo` is currently unknown to the compiler and may have meaning added to it in the future (see issue #29642)
 --> src/lib.rs:3:4
  |
3 | #![foo]
  |    ^^^
  |
  = help: add #![feature(custom_attribute)] to the crate attributes to enable

@petrochenkov
Copy link
Contributor

(I'll fill in details about why this is unstable soon.)

@petrochenkov
Copy link
Contributor

So, the primary open question about inner attributes is what scope they are resolved in.

For example:

use xxx::attr;

mod m {
    #![attr]

    use yyy::attr;
}

Does #![attr] refer to xxx::attr or yyy::attr?

Right now, #![attr] is resolved from outside of the module in the same way as outer attributes (with exception of the crate attributes maybe, not sure).

However, if something is inside a module (or block, etc) it's normally reasonable to expect that it's resolved from inside of that module as well.
IIRC, @nrc argued for applying this logic to attributes too (UPDATE: this happened in #41430).

With attributes such resolution from the inside causes problems though.


First of all, there's no "inside" before we expand all the macro attributes.

mod m {
    #![attr]
    
    /* items */
}

is just a token stream, even if it looks like module and items, it effectively doesn't exist in the module structure of the crate yet.

#![attr] can transform that token stream in any way, for example turn module into a block, or struct, or even into nothing, possibly invalidating our "inner" resolution for attr even if we somehow obtained it speculatively from the unexpanded mod m.


Possible solutions to this problem:

  • Keep status quo and resolve inner attributes from the outside (i.e. treat them purely as syntactic sugar for the "true form" - outer attributes).
    This is the simplest and most straightforward solution.
    Some details need to be figured out for crate attributes though - what exactly is the "outer scope" for the crate root (apparently it's preludes/builtins, but some prelude names actually come from items in the root module) and whether compiler-injected stuff counts as input for macros in this position.

  • Resolve from the inside, but prohibit macro attributes in inner attribute positions.
    This way we can be sure that the item to which the attribute is attached is fixed after all its outer attributes are expanded, so we can add it to the module structure of the crate immediately.
    Later, if the attribute does resolve to a macro, we can report an error.
    It turns out this is not entirely backward compatible, things like

    fn test() {
        #![test]
    
        /* body */
    }

    are already used in practice, even in the compiler itself.
    I expect the "no-macro" condition also being perceived as restrictive, given that some people want macros to work even at crate level (!) (Attribute macros invoked at crate root have issues #41430).

  • Use the outer resolution for expanding #![attr], but then validate that the inner resolution after expansion gives the same result.
    This gives effectively the same result as status quo, but with future-proofing.
    I think this would be a great way forward, because it allows to remove the feature gate without having to decide on the outer vs inner question right now.

  • Use the inner resolution before expansion for expanding #![attr], but then validate that the inner resolution after expansion gives the same result.
    This requires full-blown speculative eager expansion and we don't have it right now.
    We have some form of eager expansion in the compiler, but it's very ad-hoc, buggy and unhygienic, so it's suitable only for very simple stuff a la env!(stringify!(something)).

@petrochenkov
Copy link
Contributor

Use the outer resolution for expanding #![attr], but then validate that the inner resolution after expansion gives the same result.

This variant is the way to go right now, I think.

bors added a commit to rust-lang/rust-clippy that referenced this issue Jan 14, 2019
…phansch

Add several run rustfix annotations

Adds `run-rustfix` to 18 of the tests from the tracking issue #3630.
Each test has its own commit, to make reviewing easier (hopefully this is easier to review than 18 separate PRs).

## Changes
- `cfg_attr_rustfmt`: Custom inner attributes are unstable. Let's disable the lint for inner attributes until [#54726](rust-lang/rust#54726) stabilizes
- `collapsible_if`: unrelated cyclomatic_complexity warning that can be ignored
- `duration_subsec`: Simply needed `#![allow(dead_code)]`
- `excessive_precision`: Fixed by `#!allow(dead_code,unused_variables)`
- `explicit_write`: Fixed by `#![allow(unused_imports)]`
- `inconsistent_digit_grouping`: Avoid triggering `clippy::excessive_precision` lint
- `infallible_destructuring_match`: Fixed by `#![allow(dead_code, unreachable_code, unused_variables)]`
- `into_iter_on_ref`: Triggered unrelated `clippy::useless_vec` lint
- `large_digit_groups`: Avoid triggering `clippy::excessive_precision` lint
- `map_clone`: Fixed by `#![allow(clippy::iter_cloned_collect)]`
- `mem_replace`: Suggestion causes import to be unused, fixed by `#![allow(unused_imports)]`
- `precedence`: Allow some unrelated lints, and change out-of-range `0b1111_1111i8` literal
- `redundant_field_names`: Allow dead code, and remove stabilized feature toggles
- `replace_consts`: Fixed by `#![allow(unused_variables)]`
- `starts_ends_with`: Fixed by `#![allow(unused_must_use)]`
- `types`: Fixed by `#![allow(dead_code, unused_variables)]`
- `unit_arg`: Fixed by `#[allow(unused_must_use)]`
- `unnecessary_fold`: Fixed by adding type annotations and adding `#![allow(dead_code)]`
@dhardy
Copy link
Contributor

dhardy commented Jan 16, 2019

The caveat is that adding use yyy::attr; into the inner module may cause breakage. Overall though it seems a reasonable compromise.

@mehcode
Copy link
Contributor

mehcode commented Jan 24, 2019

My use case for this feature would be to have a custom crate level attribute in a main.rs.

#![custom_attr_here]

fn main() {
 // [...]
}

There is no outer scope here to resolve that custom attribute.

@petrochenkov
Copy link
Contributor

@mehcode
There's some outer scope even in this case

  • built-in attributes / attribute macros (not custom)
  • tool attributes, built-in or passed through command line in the future (#![rustfmt::skip])
  • extern crates passed with --extern (#![my_crate::my_attr]).

For locally defined/imported attributes that's probably the hardest case though, it certainly requires eager expansion.

@macpp
Copy link

macpp commented Feb 17, 2019

Sorry if I'm missing something important, but it seems to me that current behaviour of custom inner attributes is rather inconsistent. For example, if i have file named foo.rs with following content :

#![my_macro]

mod bar {
    #![my_macro]
    fn some_fn () {

    }
}

then first invocation of #![my_macro] receives token stream equivalent to mod foo; (without content of module) but second invocation receives tokent stream with module and full content :

mod bar {
    fn some_fn() { }
}

So custom inner attribute receives no module content if that module is placed in it's own file, but what's even stranger is that this is not always the case. If i add #![my_macro_crate::my_macro] at the top of main.rs then my_macro receives tokenStream with module and it's content, together with prelude import and fn main() .

Is there any chance to make this behavior more consistent? Current behavior could result in unwanted suprises when somebody decides for example to do refactoring and split modules to dedicated files.

@nhynes
Copy link
Contributor

nhynes commented Mar 7, 2019

I tried adding a custom attribute using syn (after a small patch), but got the error

error: macro-expanded `extern crate` items cannot shadow names passed with `--extern`
thread 'rustc' panicked at 'internal error: entered unreachable code', src/libsyntax/ext/expand.rs:294:18

using rustc 1.35.0-nightly (f22dca0a1 2019-03-05) running on x86_64-unknown-linux-gnu.

lib.rs

#![feature(custom_inner_attributes, proc_macro_hygiene)]
#![my_attr]

and in the proc_macro crate

#[proc_macro_attribute]
pub fn my_attr(_args: TokenStream, input: TokenStream) -> TokenStream {
    let module = parse_macro_input!(input as syn::ItemMod);
    let content = module.content.unwrap().1; // workaround for mod having no name
    proc_macro::TokenStream::from(quote! { #(#content)* })
}

The error is coming from the extern crate std emitted during the addition of the prelude.

@petrochenkov
Copy link
Contributor

@nhynes
This specific error about extern crate shadowing is too conservative in this case and can be fixed, but crate-level attribute macros are still generally unsupported.

The recommendation is to make a module and apply the attribute to it instead.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 8, 2019

This broke stdsimd, which uses #![rustfmt::skip] as an inner attribute. I thought inner tool attributes were ok, but since recently they fail (playground)[https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=e0cee9e1b9a9c8a7aba84b7cd700e184]:

error[E0658]: non-builtin inner attributes are unstable
 --> src/main.rs:1:1
  |
1 | #![rustfmt::skip]
  | ^^^^^^^^^^^^^^^^^
  |
  = note: for more information, see https://github.com/rust-lang/rust/issues/54726
  = help: add #![feature(custom_inner_attributes)] to the crate attributes to enable

@petrochenkov
Copy link
Contributor

@gnzlbg
"Since recently" here is 1.29, this feature gate was introduced before stabilization of tool attributes.
So, inner tool attributes were never available on stable.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 8, 2019

Weird, this started erroring this week.

@petrochenkov
Copy link
Contributor

petrochenkov commented Jul 8, 2019

@gnzlbg
I've just realized this is caused by #62133.

Previously #![feature(custom_inner_attributes)] was implicitly enabled by #![feature(custom_attributes)] or #![feature(rustc_attrs)], but the linked PR removed that because custom_attributes is being retired and rustc_attrs changed its meaning.
So, #![feature(custom_inner_attributes)] needs to be written explicitly now.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 4, 2020
Fix 59191 - ICE when macro replaces crate root with non-module item

Hi,

This should fix rust-lang#59191! My friend and I are working on learning the rustc codebase through contributions, so please feel free to mention anything amiss or that could be done better.

The code adds an explicit case for when a macro applied to the crate root (via an inner attribute) replaces it with something nonsensical, like a function. The crate root must be a module, and the error message reflects this.

---

I should note that there are a few other weird edge cases here, like if they do output a module, it succeeds but uses that module's name as a prefix for all names in the crate. I'm assuming that's an issue for stabilizing rust-lang#54726, though.
@Fishrock123
Copy link
Contributor

I'd like this so I can expand a whole bunch of lint rules org-wide from one library attribute line. That'd be quite nicer than the current mess it gives us.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 1, 2021
expand: Turn `ast::Crate` into a first class expansion target

And stop creating a fake `mod` item for the crate root when expanding a crate, thus addressing FIXMEs left in rust-lang#82238, and making a step towards a proper support for crate-level macro attributes (cc rust-lang#54726).

I haven't added token collection support for the whole crate in this PR, maybe later.
r? `@Aaron1011`
@joshtriplett joshtriplett added the S-tracking-design-concerns Status: There are blocking ❌ design concerns. label Jun 1, 2022
@afetisov
Copy link

afetisov commented Jun 1, 2022

So, the primary open question about inner attributes is what scope they are resolved in.

I don't know whether this was already implicitly finalized, but I haven't seen two other possibilities suggested;

  1. Never resolve inner attributes within inner modules. All inner attributes must always be used with a fully qualified path. Perhaps the inner attributes wouldn't be common enough to make it onerous?
  2. Introduce a separate "import inner attribute at crate level" syntax. For example, it could be a crate-level inner attribute similar to #[register_tool], e.g. #[register_attr(::path::to::attr)].

@joshlf
Copy link
Contributor

joshlf commented Oct 20, 2022

Is there any way to work around this limitation at the crate root?

termoshtt added a commit to termoshtt/katexit that referenced this issue Oct 29, 2022
@sam0x17
Copy link

sam0x17 commented Dec 8, 2022

My use case for this is I am the author of a crate that allows for annotating a module with a cryptographic signature indicating that it has been audited (based on a hash of the underlying AST nodes of the module). Ideally we would only allow using this for top-level whole-file inner attributes, as for these you can safely assume that no unaudited items are being sideloaded into the module. Right now we have to do complex filtering of the module to ensure that every item referenced in the module is part of the audited footprint. With top-of-file inner attributes, this filtering is a lot simpler.

So would love to see this stabilized, with the ability to tell whether a particular attribute is "top-level" in a file or not. That would help us out a lot.

@Qix-
Copy link

Qix- commented Dec 14, 2022

Not sure if this should be a separate issue or not, or where it should live (let me know and I can create it elsewhere), but there seems to be a weird quirk with #![no_std] being emitted from a custom inner proc macro that is itself used otherwise legally.

Repro case: https://github.com/Qix-/repro-no_std-custom-inner-attr

Results in this error message:

   Compiling test-exe v0.1.0 (/src/qix-/repro-no_std-custom-inner-attr/test-exe)
error[E0658]: `#[prelude_import]` is for use by rustc only
 --> test-exe/src/main.rs:1:1
  |
1 | / #![feature(custom_inner_attributes)]
2 | | #![::custom_attr::add_no_std]
3 | |
4 | | fn main() {}
  | |____________^
  |
  = help: add `#![feature(prelude_import)]` to the crate attributes to enable

warning: unused import: `#![feature(custom_inner_attributes)]
         #![::custom_attr::add_no_std]

         fn main() {}`
 --> test-exe/src/main.rs:1:1
  |
1 | / #![feature(custom_inner_attributes)]
2 | | #![::custom_attr::add_no_std]
3 | |
4 | | fn main() {}
  | |____________^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused `#[macro_use]` import
 --> test-exe/src/main.rs:1:1
  |
1 | / #![feature(custom_inner_attributes)]
2 | | #![::custom_attr::add_no_std]
3 | |
4 | | fn main() {}
  | |____________^

For more information about this error, try `rustc --explain E0658`.
warning: `test-exe` (bin "test-exe") generated 2 warnings
error: could not compile `test-exe` due to previous error; 2 warnings emitted

@DanielJoyce
Copy link

Yeah this is a rather sizable wart when you want to use an attribute macro in the root crate ( lib.rs ) because there is no way to decorate the crate itself outside of it. :/

@lukesneeringer
Copy link

lukesneeringer commented Oct 27, 2023

Would it be possible to remove the feature gate if the inner attribute is at the crate root? It seems like the ambiguity doesn't actually apply there, and that's the key spot where there's no alternative / workaround.

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, ..) A-proc-macros Area: Procedural macros B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. S-tracking-design-concerns Status: There are blocking ❌ design concerns. 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