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

Document local_inner_macros #53464

Closed
SimonSapin opened this issue Aug 18, 2018 · 9 comments
Closed

Document local_inner_macros #53464

SimonSapin opened this issue Aug 18, 2018 · 9 comments
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@SimonSapin
Copy link
Contributor

SimonSapin commented Aug 18, 2018

Was: `matches` crate regression: error[E0433]: failed to resolve. Did you mean `std::panic`?


Reduced test case:

#[macro_export(local_inner_macros)]
macro_rules! assert_matches {
    () => {
        panic!()
    }
}

fn main() {
    assert_matches!();
}
$ rustup run --install nightly-2018-08-17 rustc -V  
rustc 1.30.0-nightly (b2028828d 2018-08-16)
$ rustup run --install nightly-2018-08-17 rustc a.rs
$ rustup run --install nightly-2018-08-18 rustc -V 
rustc 1.30.0-nightly (1fa944914 2018-08-17)
$ rustup run --install nightly-2018-08-18 rustc a.rs
error[E0433]: failed to resolve. Did you mean `std::panic`?
 --> a.rs:4:9
  |
4 |         panic!()
  |         ^^^^^ Did you mean `std::panic`?
...
9 |     assert_matches!();
  |     ------------------ in this macro invocation

error: aborting due to previous error

For more information about this error, try `rustc --explain E0433`.

Commit range: b202882...1fa9449
Likely candidate: #50911 Stabilize use_extern_macros, CC @petrochenkov

@SimonSapin SimonSapin added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Aug 18, 2018
@petrochenkov
Copy link
Contributor

petrochenkov commented Aug 18, 2018

This is local_inner_macros working as specified.

panic!() inside a local_inner_macros macro is transformed into $crate::panic and there's no panic in the current crate root.
See e.g. rust-lang/log#288 for the correct way to use local_inner_macros.

@petrochenkov
Copy link
Contributor

petrochenkov commented Aug 18, 2018

Perhaps diagnostics could be better though, at least mentioning $crate::panic.

@petrochenkov petrochenkov added the A-diagnostics Area: Messages for errors, warnings, and lints label Aug 18, 2018
@SimonSapin
Copy link
Contributor Author

Where is this specified? I couldn’t find local_inner_macros in the reference or other docs.

Regardless, this is a case of code that compiles on earlier versions and doesn’t anymore. And it’s probably not the only instance.

@petrochenkov petrochenkov added the A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools label Aug 18, 2018
@petrochenkov
Copy link
Contributor

petrochenkov commented Aug 18, 2018

@SimonSapin

Where is this specified?

At least in the pull request introducing the feature :)
Ok, this is a doc issue too now.

@petrochenkov
Copy link
Contributor

petrochenkov commented Aug 18, 2018

Oh, I've just noticed, local_inner_macros isn't on stable yet, it's on beta.
Yes, it's not gated and the syntax was accepted on stable since pre-1.0 and ignored - this is exactly the reason why this syntax was chosen since this is a compatibility feature, but the new behavior was implemented recently and it's assumed that library authors don't add local_inner_macros in the ways that break user code.

@dtolnay dtolnay removed A-diagnostics Area: Messages for errors, warnings, and lints regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Aug 20, 2018
@dtolnay dtolnay changed the title matches crate regression: error[E0433]: failed to resolve. Did you mean std::panic? Document local_inner_macros Aug 20, 2018
@dtolnay
Copy link
Member

dtolnay commented Aug 20, 2018

As @petrochenkov explained, the behavior of local_inner_macros is intentional and designed to be ignored by old compilers. Writing this attribute on a macro containing calls to non-local macros is incorrect even if the attribute is ignored by old compilers.

Let's track documentation here and file separate issues if there are concrete suggestions for improvements to the diagnostic.

@SimonSapin
Copy link
Contributor Author

I’ve published https://crates.io/crates/matches/0.1.8 which removes the "tts as expr" macro hack that is unnecessary since Rust 1.12. This in turn makes local_inner_macros unnecessary, so that’s removed as well.

ubnt-intrepid added a commit to ubnt-intrepid/juniper that referenced this issue Aug 26, 2018
It allows to use the Rust2018-style macro imports correctly:

```rust
use juniper::graphql_object;

graphql_object!(User: () |&self| { ... });
```

In the future when dropping compatibility with pre-2018 compilers,
it is able to be replaced with the explicit `$crate` prefixes instead of
`local_inner_macros`.

In `macro_rules!` with the annotation `local_inner_macros`, all of macro calls
are transformed into `$crate::builtin!(...)` (See [1] for details).
Therefore, name resolutions of built-in macros are not perfomed correctly.
To avoid such situation, some helper macros are introduced to make built-in
macros be interpreted as crate-local macros.

[1]: rust-lang/rust#53464 (comment)
ubnt-intrepid added a commit to ubnt-intrepid/juniper that referenced this issue Aug 26, 2018
It allows to use the Rust2018-style macro imports correctly:

```rust
use juniper::graphql_object;

graphql_object!(User: () |&self| { ... });
```

In the future when dropping compatibility with pre-2018 compilers,
it is able to be replaced with the explicit `$crate` prefixes instead of
`local_inner_macros`.

In `macro_rules!` with the annotation `local_inner_macros`, all of macro calls
are transformed into `$crate::builtin!(...)` (See [1] for details).
Therefore, name resolutions of built-in macros are not perfomed correctly.
To avoid such situation, some helper macros are introduced to make built-in
macros be interpreted as crate-local macros.

[1]: rust-lang/rust#53464 (comment)
ubnt-intrepid added a commit to ubnt-intrepid/juniper that referenced this issue Aug 26, 2018
It allows to use the Rust2018-style macro imports correctly:

```rust
use juniper::graphql_object;

graphql_object!(User: () |&self| { ... });
```

In the future when dropping compatibility with pre-2018 compilers,
it is able to be replaced with the explicit `$crate` prefixes instead of
`local_inner_macros`.

In `macro_rules!` with the annotation `local_inner_macros`, all of macro calls
are transformed into `$crate::builtin!(...)` (See [1] for details).
Therefore, name resolutions of built-in macros are not perfomed correctly.
To avoid such situation, some helper macros are introduced to make built-in
macros be interpreted as crate-local macros.

[1]: rust-lang/rust#53464 (comment)
LegNeato pushed a commit to graphql-rust/juniper that referenced this issue Aug 26, 2018
This enables  Rust2018-style macro imports:

```rust
use juniper::graphql_object;

graphql_object!(User: () |&self| { ... });
```

In the future when dropping compatibility with pre-2018 compilers,
this can be replaced with the explicit `$crate` prefixes instead of
`local_inner_macros`.

In `macro_rules!` with the annotation `local_inner_macros`, all of macro calls
are transformed into `$crate::builtin!(...)` (See [1] for details).
Therefore, name resolutions of built-in macros are not perfomed correctly.
To avoid this, some helper macros are introduced to make built-in
macros be interpreted as crate-local macros.

[1]: rust-lang/rust#53464 (comment)
@pnkfelix pnkfelix added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 27, 2018
@petrochenkov
Copy link
Contributor

Looks like rust-lang/edition-guide#117 resolves this.

@steveklabnik
Copy link
Member

Yep, let's close in favor of that. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants