Skip to content

Conversation

@Aditya-PS-05
Copy link
Contributor

@Aditya-PS-05 Aditya-PS-05 commented Nov 20, 2025

closes #21070

Parameter names are bindings, not references, thus should not be qualified.

Previously, when implementing trait members, parameter names that matched macro names (e.g., vec, format, panic) were incorrectly qualified as std::vec, std::format, etc.

The issue was in transform_ident_pat() which was too aggressive - it qualified all identifier patterns that resolved to definitions, including macros.

Added a test also to test this functionality,

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 20, 2025
@fennewald
Copy link

This looks great, thank you for implementing something so quickly!

It's worth noting that even if I remove your fixes, the test you added still passes. I spent a few hours trying to dream up a test that could cover this, and was unable to make headway.

Adding minicore: panic does not help trigger the behavior. I also tried re-creating the std scheme directly, using something like:

//- /lib.rs crate:alloc
#[macro_use]
mod macros {
    #[macro_export]
    macro_rules! vec {
        () => {};
    }
}

//- /lib.rs crate:std deps:alloc extern-prelude:alloc
#[allow(unused_imports)]
#[macro_use]
extern crate alloc as alloc_crate;
pub use alloc_crate::vec;

// minicore: panic
//- /main.rs crate:bin deps:std extern-prelude:std
#[prelude_import]
pub use std::*;

trait Foo {
    fn foo(&self, vec: usize);
}

struct Bar;

impl Foo for Bar$0 {}

However, I was also unable to find a form of this that worked. I experimented with adding vec to minicore directly, but was again unsuccessful.

It may be moot, though, I believe that your fix does resolve the issue at hand.

@fennewald
Copy link

Lovely.

I can confirm on my machine that the test accurately fails without your fix, and passes with it!

@ChayimFriedman2
Copy link
Contributor

I don't think we should be treating parameters specifically, when local variables suffer from the same problem.

The real question is why we're resolving them to the macro and not the local variable, which will work automatically.

@Aditya-PS-05
Copy link
Contributor Author

I don't think we should be treating parameters specifically, when local variables suffer from the same problem.

The real question is why we're resolving them to the macro and not the local variable, which will work automatically.

done

@ChayimFriedman2
Copy link
Contributor

This isn't perfect, I would prefer to fix the resolution to resolve to the variable, but it's better than nothing.

Thanks!

@ChayimFriedman2 ChayimFriedman2 added this pull request to the merge queue Nov 22, 2025
Merged via the queue into rust-lang:master with commit 2cba0b6 Nov 22, 2025
15 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect handling of parameter names that are also macros in trait member generation

4 participants