The Wayback Machine - https://web.archive.org/web/20200621235426/https://github.com/rust-lang/rust/issues/71898
Skip to content

/ rust

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

Jump to bottom

`#[forbid(unused_qualifications)]` is incompatible with all builtin derives #71898

Open
jonas-schievink opened this issue May 4, 2020 · 3 comments · May be fixed by #71969
Open

`#[forbid(unused_qualifications)]` is incompatible with all builtin derives #71898

jonas-schievink opened this issue May 4, 2020 · 3 comments · May be fixed by #71969
Assignees
@rustbot
Labels
A-lint A-macros C-bug E-easy T-compiler

Comments

@jonas-schievink
Copy link
Member

@jonas-schievink jonas-schievink commented May 4, 2020 •
edited by rustbot 

#![forbid(unused_qualifications)]

#[derive(Clone)]
pub struct S;
error[E0453]: allow(unused_qualifications) overruled by outer forbid(unused_qualifications)
 --> src/main.rs:3:10
  |
1 | #![forbid(unused_qualifications)]
  |           --------------------- `forbid` level set here
2 | 
3 | #[derive(Clone)]
  |          ^^^^^ overruled by previous forbid

All built-in custom derives put #[allow(unused_qualifications)] on the generated impl, but the forbid level can not be overridden by that.

Custom derives are not able to trigger the deprecated lint, despite not attaching #[allow(deprecated)], so maybe the same mechanism should be used for unused_qualifications?

This issue has been assigned to @samrat via this comment.
@jonas-schievink
Copy link
Member Author

@jonas-schievink jonas-schievink commented May 4, 2020

Ah, the deprecation lint has this check:

rust/src/librustc_middle/middle/stability.rs

Lines 220 to 222 in 2454a68

if span.in_derive_expansion() {
return;
}

@varkor varkor added the E-easy label May 5, 2020
@samrat
Copy link
Contributor

@samrat samrat commented May 6, 2020

@rustbot claim
@rustbot rustbot self-assigned this May 6, 2020
@samrat
Copy link
Contributor

@samrat samrat commented May 7, 2020

If my understanding is correct, Derives(custom and builtin) will always be defined in a crate that will be external to the one using them. And it seems we turn off suggestions for lints originating from external crates:

rust/src/librustc_middle/lint.rs

Line 224 in 2454a68

// If this code originates in a foreign macro, aka something that this crate

My original approach to this issue was to avoid adding the #[allow(unused_qualifications)] to the impl and add a in_derive_expansion check in

rust/src/librustc_resolve/late.rs

Line 1844 in 2454a68

if path.len() > 1
but I couldn't figure out a case where the in_derive_expansion check makes a difference in the suggestion displayed(I did verify that the lint is captured in the lint_buffer in the case the unnecessary qualification is in an external crate, just not displayed).

Any thoughts on this approach, and suggestions on how I can go about testing this change?
@samrat samrat linked a pull request that will close this issue May 7, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint A-macros C-bug E-easy T-compiler
4 participants
@samrat @jonas-schievink @varkor @rustbot
You can’t perform that action at this time.