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

Introduce macro sub-namespaces and macro_use prelude #14781

Merged
merged 5 commits into from
May 11, 2023

Conversation

lowr
Copy link
Contributor

@lowr lowr commented May 11, 2023

This PR implements two mechanisms needed for correct macro name resolution: macro sub-namespace and macro_use prelude.

  • macro sub-namespaces

    Macros have two sub-namespaces: one for function-like macro and the other for those in attributes (including custom derive macros). When we're resolving a macro name for function-like macro, we should ignore non-function-like macros, and vice versa.

    This helps resolve single-segment macro names because we can (and should, as rustc does) fallback to names in preludes when the name in the current module scope is in different sub-namespace.

  • macro_use prelude

    #[macro_use]'d extern crate declarations (including the standard library) bring their macros into scope, but they should not be prioritized over local macros (those defined in place and those explicitly imported).

    We have been bringing them into legacy (textual) macro scope, which has the highest precedence in name resolution. This PR introduces the macro_use prelude in crate-level DefMaps, whose precedence is lower than local macros but higher than the standard library prelude.

The first 3 commits are drive-by fixes/refactors.

Fixes #8828 (prelude)
Fixes #12505 (prelude)
Fixes #12734 (prelude)
Fixes #13683 (prelude)
Fixes #13821 (prelude)
Fixes #13974 (prelude)
Fixes #14254 (namespace)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 11, 2023
@lowr lowr force-pushed the patch/macro-subns-and-prelude branch from 49f93b2 to 8d32301 Compare May 11, 2023 11:39
lowr added 4 commits May 11, 2023 21:13
We've already removed non-sysroot proc macro server, which effectively
removed support for Rust <1.64.0, so this removal of fallback path
shouldn't be problem at this point.
@lowr lowr force-pushed the patch/macro-subns-and-prelude branch from 8d32301 to f2a35de Compare May 11, 2023 12:15
@Veykril
Copy link
Member

Veykril commented May 11, 2023

That's quite a list of fixes by one PR, awesome 🎉
@bors r+

@bors
Copy link
Contributor

bors commented May 11, 2023

📌 Commit f2a35de has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented May 11, 2023

⌛ Testing commit f2a35de with merge 9b33874...

@bors
Copy link
Contributor

bors commented May 11, 2023

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 9b33874 to master...

@bors bors merged commit 9b33874 into rust-lang:master May 11, 2023
@Veykril
Copy link
Member

Veykril commented May 12, 2023

I think this broke some things. rustc_query_append no longer resolves in multiple places in the compiler. It is generated by a proc-macro here https://github.com/rust-lang/rust/blob/077fc26f0acfa54e9c580534616c17ffc279a9d4/compiler/rustc_middle/src/query/mod.rs#L26 and used here where it no longer resolves https://github.com/rust-lang/rust/blob/077fc26f0acfa54e9c580534616c17ffc279a9d4/compiler/rustc_middle/src/dep_graph/dep_node.rs#L107

Of note is that the rustc_query_append macro resolves in the crate root of rustc_middle, but not in any child modules

bors added a commit that referenced this pull request May 13, 2023
Expand more single ident macro calls upon item collection

Addresses #14781 (comment)

I believe this (almost) brings the number of unresolved names back to pre-#14781:

|r-a version|`analysis-stats compiler/rustc` (rust-lang/rust@69fef92) |
|---|---|
|pre-#14781 (b069eb7) | exprs: 2747778, ??ty: 122236 (4%), ?ty: 107826 (3%), !ty: 728 |
| #14781 (a7944a9) | exprs: 2713080, ??ty: 139651 (5%), ?ty: 114444 (4%), !ty: 730 |
| with this fix | exprs: 2747871, ??ty: 122237 (4%), ?ty: 108171 (3%), !ty: 676 |

(I haven't investigated on the increase in some numbers but hopefully not too much of a problem)

This is only a temporary solution. The core problem is that we haven't fully implemented the textual scope of legacy macros. For example, we *have been* failing to resolve `foo` in the following snippet, even before #14781 or after this patch. As noted in a FIXME, we need a way to resolve names in textual scope without eager expansion during item collection.

```rust
//- /main.rs crate:main deps:lib
lib::mk_foo!();
const A: i32 = foo!();
             //^^^^^^ unresolved-macro-call

//- /lib.rs crate:lib
#[macro_export]
macro_rules! mk_foo {
    () => {
        macro_rules! foo { () => { 42 } }
    }
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment