Skip to content
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

2021 prelude collision lint expands macros when used as an expression #88347

Closed
ehuss opened this issue Aug 25, 2021 · 2 comments · Fixed by #88501
Closed

2021 prelude collision lint expands macros when used as an expression #88347

ehuss opened this issue Aug 25, 2021 · 2 comments · Fixed by #88501
Assignees
Labels
A-edition-2021 Area: The 2021 edition A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ehuss
Copy link
Contributor

ehuss commented Aug 25, 2021

The rust_2021_prelude_collisions lint suggestion seems to expand the expression for a macro, but I don't think it should. The following:

#![warn(rust_2021_prelude_collisions)]

fn bar() {}

macro_rules! foo {
    () => {{
        $crate::bar();
        S
    }};
}

trait MyTry<T> {
    fn try_into(self) -> Result<T, ()>;
}

struct S;

impl MyTry<i32> for S {
    fn try_into(self) -> Result<i32, ()> {
        unimplemented!();
    }
}

fn main() {
    foo!().try_into().unwrap();
}

The suggestion recommends replacing it with:

    MyTry::try_into({
        $crate::bar();
        S
    }).unwrap();

which will not compile. I would expect it to suggest:

MyTry::try_into(foo!()).unwrap();

Found during the crater run for:

Meta

rustc --version --verbose:

rustc 1.56.0-nightly (b03ccace5 2021-08-24)
binary: rustc
commit-hash: b03ccace573bb91e27625c190a0f7807045a1012
commit-date: 2021-08-24
host: x86_64-apple-darwin
release: 1.56.0-nightly
LLVM version: 13.0.0
@ehuss ehuss added C-bug Category: This is a bug. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` A-edition-2021 Area: The 2021 edition labels Aug 25, 2021
@inquisitivecrystal inquisitivecrystal added D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 26, 2021
@m-ou-se m-ou-se self-assigned this Aug 26, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Aug 26, 2021

This is basically the same issue as #87955, and probably happens in many more places. The solution is simple, but this would be the third place we add a loop to follow spans up expansions until the right one is found. So I'll see if I can put it into a method on span.

(Something like Span::find_parent_inside(outer: Span) -> Option<Span> or something. Or maybe we can somehow fix it in a more general way.)

@m-ou-se
Copy link
Member

m-ou-se commented Aug 30, 2021

The same happens for arguments, not just for the receiver:

#![warn(rust_2021_prelude_collisions)]

fn bar() {}

macro_rules! foo {
    () => {{
        $crate::bar();
        S
    }};
}

trait MyTry<T> {
    fn try_into(self, _: u8);
}

struct S;

impl MyTry<i32> for S {
    fn try_into(self, _: u8) {}
}

fn main() {
    foo!().try_into(todo!());
}
help: disambiguate the associated function
   |
29 ~     MyTry::try_into({
30 +         $crate::bar();
31 +         S
32 ~     }, $crate::panicking::panic($msg));
   |

And worse: if the source code of todo!() in core isn't available, this ICEs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edition-2021 Area: The 2021 edition A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants