-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Regression importing log’s warn! macro inside of include!()ed file #53205
Comments
It took me a few reads to find the key word "attribute" in this explanation, which refers to the lint-control one used for example as Why do function-like macros shadow attributes? (Even proc macro ones.) Shouldn’t they be in separate namespaces? At the very least, the error message here is wrong. But avoiding the breakage entirely would be nice. |
Waiting on #53089 |
Work around a rustc regression in macro name resolution rust-lang/rust#53205 <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21367) <!-- Reviewable:end -->
Yeah, the diagnostics weren't properly adjusted for #52841
Well, this is an interesting question. If the annoying cases like this continue to arise, we can move to "sub-namespaces" during relative path resolution (attributes are ignored when resolving non-attributes and vice versa, but It's also important to note that this particular case shouldn't even be a restricted shadowing error! It's an error only because the restricted shadowing rules are very conservative and coarse-grained - we report an error if "macro-expanded macro shadows any other macro", but what we really want is to error on "more macro-expanded macro shadows less macro-expanded macro". macro_rules! my_include {() => {
macro m() {} // 1
{
macro m() {} // 2
m!(); // ERROR `m` is ambiguous (1 or 2)
}
}}
fn main() {
my_include!();
} |
Work around a rustc regression in macro name resolution rust-lang/rust#53205 <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21367) <!-- Reviewable:end -->
Visiting for triage. Let's mark as P-high, but @petrochenkov seems to be on this. |
Work around a rustc regression in macro name resolution rust-lang/rust#53205 <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21367) <!-- Reviewable:end -->
Fix a few regressions from enabling macro modularization The first commit restores the old behavior for some minor unstable stuff (`rustc_*` and `derive_*` attributes) and adds a new feature gate for arbitrary tokens in non-macro attributes. The second commit fixes #53205 The third commit fixes #53144. Same technique is used as for other things blocking expansion progress - if something causes indeterminacy too often, then prohibit it. In this case referring to crate-local macro-expanded `#[macro_export]` macros via module-relative paths is prohibited, see comments in code for more details. cc #50911
resolve: Introduce two sub-namespaces in macro namespace Two sub-namespaces are introduced in the macro namespace - one for bang macros and one for attribute-like macros (attributes, derives). "Sub-namespace" means this is not a newly introduced full namespace, the single macro namespace is still in place. I.e. you still can't define/import two macros with the same name in a single module, `use` imports still import only one name in macro namespace (from any sub-namespace) and not possibly two. However, when we are searching for a name used in a `!` macro call context (`my_macro!()`) we skip attribute names in scope, and when we are searching for a name used in attribute context (`#[my_macro]`/`#[derive(my_macro)]`) we are skipping bang macro names in scope. In other words, bang macros cannot shadow attribute macros and vice versa. For a non-macro analogy, we could e.g. skip non-traits when searching for `MyTrait` in `impl MyTrait for Type { ... }`. However we do not do it in non-macro namespaces because we don't have practical issues with e.g. non-traits shadowing traits with the same name, but with macros we do, especially after macro modularization. For `#[test]` and `#[bench]` we have a hack in the compiler right now preventing their shadowing by `macro_rules! test` and similar things. This hack was introduced after making `#[test]`/`#[bench]` built-in macros instead of built-in attributes (#53410), something that needed to be done from the start since they are "active" attributes transforming their inputs. Now they are passed through normal name resolution and can be shadowed, but that's a breaking change, so we have a special hack basically applying this PR for `#[test]` and `#[bench]` only. Soon all potentially built-in attributes will be passed through normal name resolution (#53913) and that uncovers even more cases where the strict "macro namespace is a single namespace" rule needs to be broken. For example, with strict rules, built-in macro `cfg!(...)` would shadow built-in attribute `#[cfg]` (they are different things), standard library macro `thread_local!(...)` would shadow built-in attribute `#[thread_local]` - both of these cases are covered by special hacks in #53913 as well. Crater run uncovered more cases of attributes being shadowed by user-defined macros (`warn`, `doc`, `main`, even `deprecated`), we cannot add exceptions in the compiler for all of them. Regressions with user-defined attributes like #53583 and #53898 also appeared after enabling macro modularization. People are also usually confused (#53205 (comment), #53583 (comment)) when they see conflicts between attributes and non-attribute macros for the first time. So my proposed solution is to solve this issue by introducing two sub-namespaces and thus skipping resolutions of the wrong kind and preventing more error-causing cases of shadowing. Fixes #53583
Cargo.toml
src/main.rs
src/real_main.rs
Code like this used to build correctly:
And now produces an error whose message seems wrong: it points at the call site of a macro as a "name also defined here":
Commit range between these two nightlies: 97085f9...7e8ca9f
#52841 is the only PR in the range that seems relevant. CC @petrochenkov, @alexcrichton
The text was updated successfully, but these errors were encountered: