borrowcheck error in code generated in a macro due to 2024 impl trait capture changes

[ehuss]

Code generated in a proc-macro in the 2021 edition may fail to compile when used in a 2024 crate due to changes in impl trait capturing. This will leave the author of the 2024 crate without any ability to fix the issue.

Example:

// lib.rs
#[pm::pm]
pub struct S;

// proc-macro `pm`
use proc_macro::TokenStream;

#[proc_macro_attribute]
pub fn pm(_attr: TokenStream, _item: TokenStream) -> TokenStream {
    "
pub struct X<'a>(::std::marker::PhantomData<&'a str>);

pub trait Tr<'a> {
    fn foo(self, x: &mut X<'a>) where Self: Sized {}
}

pub struct Y<'a>(::std::marker::PhantomData<&'a str>);

impl<'a> Tr<'a> for Y<'a> {

}

pub fn f<'a>(x: &mut X<'a>) -> impl Tr<'a> {
    Y(::std::marker::PhantomData)
}

pub fn g<'a>(x: &mut X<'a>) {
    f(x).foo(x)
}
"
    .parse()
    .unwrap()
}

When the library is migrated with cargo fix --edition, there is a warning that the proc-macro has an issue, but with no detail on what's going on (since all the code is hidden):

warning: `impl Tr<'a>` will capture more lifetimes than possibly intended in edition 2024
 --> src/lib.rs:3:1
  |
3 | #[pm::pm]
  | ^^^^^^^^^
  |
  = warning: this changes meaning in Rust 2024
  = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/rpit-lifetime-capture.html>
note: specifically, this lifetime is in scope but not mentioned in the type's bounds
 --> src/lib.rs:3:1
  |
3 | #[pm::pm]
  | ^
  = note: all lifetimes in scope will be captured by `impl Trait`s in edition 2024
  = note: `--force-warn impl-trait-overcaptures` implied by `--force-warn rust-2024-compatibility`
  = note: this warning originates in the attribute macro `pm::pm` (in Nightly builds, run with -Z macro-backtrace for more info)

After migrating to 2024, it fails to compile:

error[E0499]: cannot borrow `*x` as mutable more than once at a time
 --> src/lib.rs:3:1
  |
3 | #[pm::pm]
  | ^^^^^^^^^
  | |
  | `*x` was mutably borrowed here in the previous iteration of the loop
  | first borrow later used by call
  |
note: this call may capture more lifetimes than intended, because Rust 2024 has adjusted the `impl Trait` lifetime capture rules
 --> src/lib.rs:3:1
  |
3 | #[pm::pm]
  | ^^^^^^^^^
  = note: this error originates in the attribute macro `pm::pm` (in Nightly builds, run with -Z macro-backtrace for more info)
help: add a precise capturing bound to avoid overcapturing
  |
3 | #[pm::pm] + use<'a>
  |           +++++++++

Additionally, the suggestion provided there is incorrect, since it is not valid syntax.

I would expect the functions generated by the proc-macro to use the capturing rules from 2021. This is an issue because editions are supposed to be independent of the crate.

Meta

rustc --version --verbose:

rustc 1.84.0-nightly (59cec72a5 2024-11-08)
binary: rustc
commit-hash: 59cec72a57af178767a7b8e7f624b06cc50f1087
commit-date: 2024-11-08
host: aarch64-apple-darwin
release: 1.84.0-nightly
LLVM version: 19.1.3

[compiler-errors]

I don't think this is fixable

[compiler-errors]

That pm::pm expansion uses the span edition of the usage site rather than the definition site. Users can't even fix this by respanning their tokens in the proc-macro b/c Span::def_site is unstable.

[ehuss]

This also happens with an external crate that defines code with macro_rules!:

#[macro_export]
macro_rules! m {
    () => {
        pub struct X<'a>(::std::marker::PhantomData<&'a str>);

        pub trait Tr<'a> {
            fn foo(self, x: &mut X<'a>)
            where
                Self: Sized,
            {
            }
        }

        pub struct Y<'a>(::std::marker::PhantomData<&'a str>);

        impl<'a> Tr<'a> for Y<'a> {}

        pub fn f<'a>(x: &mut X<'a>) -> impl Tr<'a> {
            Y(::std::marker::PhantomData)
        }

        pub fn g<'a>(x: &mut X<'a>) {
            f(x).foo(x)
        }
    };
}

[ehuss]

@compiler-errors How does the compiler decide to use the new precise capturing rules? Does it look at the edition of the entire crate, or is there a specific span it is looking at? Or...?

[ehuss]

Looking closer, it looks like this is due to this (the switch of the edition here).

@compiler-errors Can that be changed to not muck with the edition of the span? That is, apply something like this patch:

diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs
index 5a0e9e8aec0..d53280751fc 100644
--- a/compiler/rustc_ast_lowering/src/lib.rs
+++ b/compiler/rustc_ast_lowering/src/lib.rs
@@ -738,7 +738,7 @@ fn mark_span_with_reason(
         allow_internal_unstable: Option<Lrc<[Symbol]>>,
     ) -> Span {
         self.tcx.with_stable_hashing_context(|hcx| {
-            span.mark_with_reason(allow_internal_unstable, reason, self.tcx.sess.edition(), hcx)
+            span.mark_with_reason(allow_internal_unstable, reason, span.edition(), hcx)
         })
     }

It doesn't seem correct to me to change the edition of a span.

[compiler-errors]

@ehuss: Yeah, that seems odd and comes from #50307, where afaict it doesn't seem intended.

That would fix the macro by example version of this, but I don't believe it changes the proc macro version? I still think that that uses the span of the crate that calls the proc macro (which would be edition 2024).

[compiler-errors]

Ah, lol, the parse function uses call-site hygiene but that "call-site" span is still using the edition of the proc macro. How confusing. That's probably impossible to fix now, and probably debatable whether or not it should be fixed at all.

@ehuss: Yeah, that fix you provided is both correct and likely not going to affect anything other than opaques today, which is probably why this is the first time we've seen it now. Feel free to put up a PR with the tests you've provided (maybe minimized), or I can do that instead.

[ehuss]

I tested with a proc-macro, and the patch above seems to fix it. [...deleted rest of comment seeing that you just posted...]

[ehuss]

I can post a PR.

[spastorino]

@lqd was mentioning this to me and we were wondering how related this could be with #132425

Was actually looking around for places where we do similar things and found ...

diff --git a/compiler/rustc_resolve/src/def_collector.rs b/compiler/rustc_resolve/src/def_collector.rs
index a825458dc89..1378639de17 100644
--- a/compiler/rustc_resolve/src/def_collector.rs
+++ b/compiler/rustc_resolve/src/def_collector.rs
@@ -200,7 +200,7 @@ fn visit_item(&mut self, i: &'a Item) {
             ItemKind::Const(..) => DefKind::Const,
             ItemKind::Fn(..) | ItemKind::Delegation(..) => DefKind::Fn,
             ItemKind::MacroDef(def) => {
-                let edition = self.resolver.tcx.sess.edition();
+                let edition = i.span.edition();
                 let macro_data =
                     self.resolver.compile_macro(def, i.ident, &i.attrs, i.span, i.id, edition);
                 let macro_kind = macro_data.ext.macro_kind();

I wonder why don't we use i.span.edition() there. I guess it may be what @compiler-errors was talking about and we consider definition site hygiene instead of call site hygiene.