Tracking issue for future-incompatibility lint proc_macro_derive_resolution_fallback

[petrochenkov]

This is a lint for code like

struct S;
mod m {
    type A = S;
}

when it comes from a derive macro.

The code should obviously not compile because S is not in scope at its point of use, but before #51952 it compiled if the code was produced by a derive macro, and some popular crates like actix_derive, diesel_derives and palette_derive exploited this in the wild.

This deprecation lint was added in #51952 as warn-by-default.

[LPGhatguy]

I believe this lint has unveiled an unfortunate case in Rust's name resolution behavior for proc macro authors. Enforcing this makes certain proc macros no longer function if used inside of a fn when they otherwise work.

I originally thought this was a false positive in this lint, but after some digging, it seems like a bug or quirk that I think should be taken into consideration for enforcement of this stabilization lint.

Here's a repro case I created under the impression that this was a false positive: https://github.com/LPGhatguy/rustc-issue-83583-false-positive

A proc macro that generates a mod that refers to the input struct works when the input struct is in the top-level scope:

// this is fine!
#[derive(Foo)]
struct Blah;

...but fails if the input struct is defined inside a function:

// this trips this deprecation lint
fn item() {
    #[derive(Foo)]
    struct Blah;
}

This appears to be due to a difference in the meaning of super when used from within a mod inside a fn. This means that in the second code example here, there is no way to refer to Blah from within a generated mod.

For proc macro authors, the solution seems to be to never generate a mod from within a derive macro, as resolution rules may result in problems for the generated code.

This commit backlinks to this issue and is an example of the confusion that users may encounter due to this deprecation lint.

[petrochenkov]

Diesel had this issue (as discussed in #51952) and migrated to generating some other code pattern, not using mod + super, so a solution exists, but I don't remember what they used exactly, you can check the diesel code.

This commit backlinks to this issue and is an example of the confusion that users may encounter due to this deprecation lint.

Users are right to be confused, because the blame is on the authors of StructOfArray in that case, but we cannot produce the warning until the code is actually generated on the user side, unfortunately.
The general idea is that the code generated by derive should work if it was copy-pasted manually and not through derive, and that's not true in case of StructOfArray.

[LPGhatguy]

Thank you for linking the thread. It appears that the Diesel authors found a solution but were not generally happy with this change.

It would be nice if proc macro authors had better tools for scope isolation. I only found out about this issue after using a proc macro that had existed for almost two years from within a function. I had no idea that name resolution from modules inside functions worked this way until this warning showed up in my terminal.

While there are solutions, and I have now implemented them, they come with trade-offs. For example, one of the solutions from the thread:

fn __implementation_details() {
    impl Something for MyStruct {
       let x: SomethingElseFromTheParentModule = ...;
    }
}

...lacks the ability for other code generated from the proc macro to be able to refer to items from within the implementation details scope. This is the perfect use case for a mod, but it's hampered by resolution rules.

I'd love to have a way to refer to an ident that comes from a user regardless of being in a sub-mod, like how proc macros work today, but without this warning. Otherwise, mod blocks are not very useful for macro authors despite being a core part of the language.

Is there a better place to discuss this issue? I figure that going deeper into a conversation about solutions may be out of scope for this issue.

[petrochenkov]

It would be nice if proc macro authors had better tools for scope isolation.

Yes, it would be good to have something better than the current tools, but it's out of scope for this specific issue.
My preferred solution would probably be transparent modules - #64079 (comment).

[shepmaster]

The SNAFU procedural macro optionally generates a module that starts with use super::*. The direct code works fine. When that code is copy-pasted into a doc-comment, it fails:

use snafu::prelude::*;

#[derive(Debug, Snafu)]
#[snafu(module)]
struct Foo;

/// ```rust
/// use snafu::prelude::*;
///
/// #[derive(Debug, Snafu)]
/// #[snafu(module)]
/// struct Foo;
/// ```
pub fn dummy() {}

% cargo +nightly build
    Finished dev [unoptimized + debuginfo] target(s) in 0.21s

% cargo +nightly test
   Doc-tests repro

running 1 test
test src/lib.rs - dummy (line 7) ... FAILED

failures:

---- src/lib.rs - dummy (line 7) stdout ----
error: cannot find type `Foo` in this scope
 --> src/lib.rs:12:8
  |
7 | struct Foo;
  |        ^^^ names from parent modules are not accessible without an explicit import
  |
  = note: `#[deny(proc_macro_derive_resolution_fallback)]` on by default
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #83583 <https://github.com/rust-lang/rust/issues/83583>

% rustc +nightly --version --verbose
rustc 1.58.0-nightly (936238a92 2021-11-11)
binary: rustc
commit-hash: 936238a92b2f9d6e7afe7dda69b4afd903f96399
commit-date: 2021-11-11
host: aarch64-apple-darwin
release: 1.58.0-nightly
LLVM version: 13.0.0

[shepmaster]

Ah, that's because the doctest is added inside of a fn main by default. Very unfortunate.

[LukasKalbertodt]

In case anyone encounters the same problem as @shepmaster (I just did), remember that you can write fn main in the doctest manually to prevent rustc from inserting one automatically. And that way you can have your proc macro outside of main. It's not a perfect solution, but at least that way, you can compile your tests again.

/// ```rust
/// use snafu::prelude::*;
///
/// #[derive(Debug, Snafu)]
/// #[snafu(module)]
/// struct Foo;
/// # fn main() {}
/// ```
pub fn dummy() {}

[pnkfelix]

@rustbot label: I-lang-nominated

I would like the lang team to discuss the problems that people have encountered from the lint described here, and the language issues that have led them to encounter those problems.

In particular, it seems like macro authors would very much like to be able to write code like this (playpen):

mod outer {
    struct O;
    pub fn foo() {
        // This type is local to this fn implementation. Furthermore,
        // it might have been injected *into* the body of the fn implementation
        // by a macro exapnsion. (For example, that is part of what happens
        // with doc tests that include struct definitions.) So, we cannot
        // try to force the struct definition to live outside of here.
        //
        // and remember, there may be a derive on this thing too.
        #[derive()]
        struct FooS;
        
        // This code is macro-generated, e.g. from a derive on FooS.
        mod inner {
            use super::*; // this use enables us to see `O`, but not `FooS` (!)
            
            fn om(x: O) -> O { x }

            // so how can one reference the type `FooS` from here?
            #[cfg(this_is_the_code_that_breaks_with_a_resolve_error)]
            fn g(f: FooS) -> FooS { f }
        }
    }
}

[pnkfelix]

see also #64079, which I think precisely describes the issue I summarized in my playpen example in the previous comment, and which @petrochenkov closed as not-a-bug. (I'm currently trying to figure out when/if T-lang has previously discussed this matter.)

[pnkfelix]

Okay, @joshtriplett has pointed out to me that one can work around the problem I described in my playpen example by introducing an intermedate module around the struct with the derive on it.

I.e. something like this playpen

[pnkfelix]

See also #84022 which made the deprecation lint here a hard error.

(Which, I admit, I signed off on in that issue. I think I did not appreciate the details that were raised on this issue about the problems with code that has been expanded directly within an fn body.)

[pnkfelix]

I filed rust-lang/lang-team#193 for the lang team to formally follow up on this.

I think that allows us to remove it from the lang-nominated issues

@rustbot label: -I-lang-nominated