rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
99.09k stars 12.8k forks source link

Unexpected warning when doc string invokes a macro which is defined within the same module #133656

Open rami3l opened 1 day ago

rami3l commented 1 day ago

This is a follow-up of https://github.com/rust-lang/rust/issues/124535#issuecomment-2466005846.

TLDR: it looks like https://github.com/rust-lang/rust/issues/124535 might have not been completely resolved, or at least there are some undetected edge cases in https://github.com/rust-lang/rust/pull/125741/commits/c4c7859e40efcfff640af442fb5d1fab3718d374...

Issue Description

In the minimal reproduction (rami3l/repro_rust_124535), we have:

// lib.rs
macro_rules! pm_mods {
    ( $( $vis:vis $mod:ident; )+ ) => {
        $(
            $vis mod $mod;
            pub use self::$mod::$mod;
        )+
    }
}

pm_mods! {
    dnf;
}
// dnf.rs
#![doc = doc_self!()]

macro_rules! doc_self {
    () => {
        "The Dandified YUM."
    };
}
use doc_self; // <- Please note that the suggested fix has already been applied.

#[doc = doc_self!()]
pub fn dnf() {}

I expected to see this happen: ✅

Instead, this happened:

> cargo check
    Checking repro_rust_124535 v0.1.0 (/path/censored)
warning: cannot find macro `doc_self` in this scope
 --> src/dnf.rs:1:10
  |
1 | #![doc = doc_self!()]
  |          ^^^^^^^^
  |
  = 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 #124535 <https://github.com/rust-lang/rust/issues/124535>
  = help: import `macro_rules` with `use` to make it callable above its definition
  = note: `#[warn(out_of_scope_macro_calls)]` on by default

warning: `repro_rust_124535` (lib) generated 1 warning
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.06s

Also, it's worth noticing that after inlining the macro invocation in lib.rs, i.e.:

// lib.rs
mod dnf;
pub use self::dnf::dnf;

... the error seems gone.

Meta

rustc --version --verbose:

rustc 1.83.0 (90b35a623 2024-11-26)
binary: rustc
commit-hash: 90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf
commit-date: 2024-11-26
host: aarch64-apple-darwin
release: 1.83.0
LLVM version: 19.1.1
rami3l commented 1 day ago

Additionally, I've come up with a similar single-file version that can be easily checked within the Rust Playground:

macro_rules! mods {
    ( $( $mod:ident; )+ ) => {
        $(
            mod $mod {
                #![doc = doc_self!()]

                macro_rules! doc_self {
                    () => { stringify!($mod) };
                }

                use doc_self; // <- Please note that the suggested fix has already been applied.
            }
        )+
    }
}

mods! {
    foo;
}
   Compiling playground v0.0.1 (/playground)
warning: cannot find macro `doc_self` in this scope
  --> src/lib.rs:5:26
   |
5  |                   #![doc = doc_self!()]
   |                            ^^^^^^^^
...
17 | / mods! {
18 | |     foo;
19 | | }
   | |_- in this macro invocation
   |
   = 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 #124535 <https://github.com/rust-lang/rust/issues/124535>
   = help: import `macro_rules` with `use` to make it callable above its definition
   = note: `#[warn(out_of_scope_macro_calls)]` on by default
   = note: this warning originates in the macro `mods` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: `playground` (lib) generated 1 warning
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.46s
jieyouxu commented 1 day ago

cc @petrochenkov (in case you know if this is intended behavior or not)

rami3l commented 19 hours ago

cc @petrochenkov (in case you know if this is intended behavior or not)

@jieyouxu Thanks a lot for the triage! To me this looks more like a false positive of the lint condition, given the fact that the mods!{} macro's manual expansion won't trigger the warning, but its invocation will.

I'm not familiar with that part of the internals, but as long as the lint condition is not directly connected to the tree-visiting logic related to the upcoming breaking change, there is a possibility that this particular case will not break with that change in place, even if the warning says it will?

If that is the case, then it's probably okay to admit the temporary nature of the lint and do nothing for the moment :)