Skip unused_parens report for Paren(Path(..)) in macro.

[surechen]

fixes #120642

In following code, unused_parens suggest change <($($rest),*)>::bar() to <$rest>::bar() which will cause another err: error: variable 'rest' is still repeating at this depth：

trait Foo {
    fn bar();
}

macro_rules! problem {
    ($ty:ident) => {
        impl<$ty: Foo> Foo for ($ty,) {
            fn bar() { <$ty>::bar() }
        }
    };
    ($ty:ident $(, $rest:ident)*) => {
        impl<$ty: Foo, $($rest: Foo),*> Foo for ($ty, $($rest),*) {
            fn bar() {
                <$ty>::bar();
                <($($rest),*)>::bar()
            }
        }
        problem!($($rest),*);
    }
}

I think maybe we can handle this by avoid warning for Paren(Path(..)) in the macro. Is this reasonable approach?

[rustbot]

r? @Nadrieril

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[Nadrieril]

It would be better to fix the incorrect span detection. I don't know how easy that is however

[cjgillot]

The lint is correct to fire here, only the suggestion is wrong. Looking at this kind of code, the compiler should assume the author meant <(T3,)>::bar() (notice the comma).

[surechen]

The lint is correct to fire here, only the suggestion is wrong. Looking at this kind of code, the compiler should assume the author meant <(T3,)>::bar() (notice the comma).

Thank you. I agree with you. I initially wanted to do like this.
Since this lint detects the ast expr after macro expansion, In fact, I don't how to find the macro by a ast expr, and how to judge the form of the macro. I will try to do it again. If you have some idea, please give me some help.

[cjgillot]

In the call to emit_unused_delims, the spans argument tells where are the parentheses, and to use them for a suggestion. If spans is None, there is no suggestion.
Pedantically, we should strive to suggest <$($rest),*>::bar(), ie just remove the parentheses. Maybe using start_point/end_point instead of looking at the inner type?
From a brief look, all the calls to emit_unused_delims use a spans argument constructed the same way. So they may all suffer from the same bug. It's a good opportunity to exercise all cases in the test.

[surechen]

In the call to emit_unused_delims, the spans argument tells where are the parentheses, and to use them for a suggestion. If spans is None, there is no suggestion. Pedantically, we should strive to suggest <$($rest),*>::bar(), ie just remove the parentheses. Maybe using start_point/end_point instead of looking at the inner type? From a brief look, all the calls to emit_unused_delims use a spans argument constructed the same way. So they may all suffer from the same bug. It's a good opportunity to exercise all cases in the test.

Thank you very much, I will read and think about your suggestions as soon as possible

[surechen]

It would be better to fix the incorrect span detection. I don't know how easy that is however

Thank you very much.

I find there are already some measures to avoid reporting errors for conditions inside macros, such as https://github.com/rust-lang/rust/blob/master/compiler/rustc_lint/src/unused.rs#L1023 and https://github.com/rust-lang/rust/blob/master/compiler/rustc_lint/src/unused.rs#L1353.

Because it's hard to give correct suggestion for <($($rest),*)>::bar() to <($($rest,)*)>::bar().
Other suggestion like <$rest>::bar() or even <$($rest),*>::bar() will also cause compiler error.
So I just give error report and avoid suggestion for in macro expand.

Please see if it meets expectations?

[Nadrieril]

Yep, that's a fine solution. Thank you!

@bors r+

[bors]

📌 Commit c8490a0 has been approved by Nadrieril

It is now in the queue for this repository.