From 7d99866bfc43f34dbdd84f4bf982c48a51b70a99 Mon Sep 17 00:00:00 2001 From: yukang Date: Sat, 24 Dec 2022 02:41:06 +0800 Subject: [PATCH 1/4] fix #105061, Fix unused_parens issue for higher ranked function pointers --- compiler/rustc_lint/src/early.rs | 6 ++++++ compiler/rustc_lint/src/unused.rs | 1 - tests/ui/lint/unused/issue-105061.rs | 17 +++++++++++++++++ tests/ui/lint/unused/issue-105061.stderr | 20 ++++++++++++++++++++ 4 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 tests/ui/lint/unused/issue-105061.rs create mode 100644 tests/ui/lint/unused/issue-105061.stderr diff --git a/compiler/rustc_lint/src/early.rs b/compiler/rustc_lint/src/early.rs index f9b2df4959224..3901751c79fb0 100644 --- a/compiler/rustc_lint/src/early.rs +++ b/compiler/rustc_lint/src/early.rs @@ -248,6 +248,12 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T> } fn visit_where_predicate(&mut self, p: &'a ast::WherePredicate) { + use rustc_ast::{WhereBoundPredicate, WherePredicate}; + if let WherePredicate::BoundPredicate(WhereBoundPredicate { bounded_ty, .. }) = p && + let ast::TyKind::BareFn(b) = &bounded_ty.kind && + b.generic_params.len() > 0 { + return; + } ast_visit::walk_where_predicate(self, p); } diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index ac2b32b44e6a1..94a3313810717 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -1002,7 +1002,6 @@ impl EarlyLintPass for UnusedParens { if let ast::TyKind::Paren(r) = &ty.kind { match &r.kind { ast::TyKind::TraitObject(..) => {} - ast::TyKind::BareFn(b) if b.generic_params.len() > 0 => {} ast::TyKind::ImplTrait(_, bounds) if bounds.len() > 1 => {} ast::TyKind::Array(_, len) => { self.check_unused_delims_expr( diff --git a/tests/ui/lint/unused/issue-105061.rs b/tests/ui/lint/unused/issue-105061.rs new file mode 100644 index 0000000000000..92d636d0ac62d --- /dev/null +++ b/tests/ui/lint/unused/issue-105061.rs @@ -0,0 +1,17 @@ +#![warn(unused)] +#![deny(warnings)] + +struct Inv<'a>(&'a mut &'a ()); + +trait Trait {} +impl Trait for (for<'a> fn(Inv<'a>),) {} + + +fn with_bound() +where + ((for<'a> fn(Inv<'a>)),): Trait, //~ ERROR unnecessary parentheses around type +{} + +fn main() { + with_bound(); +} diff --git a/tests/ui/lint/unused/issue-105061.stderr b/tests/ui/lint/unused/issue-105061.stderr new file mode 100644 index 0000000000000..f07aa2012df5f --- /dev/null +++ b/tests/ui/lint/unused/issue-105061.stderr @@ -0,0 +1,20 @@ +error: unnecessary parentheses around type + --> $DIR/issue-105061.rs:12:6 + | +LL | ((for<'a> fn(Inv<'a>)),): Trait, + | ^ ^ + | +note: the lint level is defined here + --> $DIR/issue-105061.rs:2:9 + | +LL | #![deny(warnings)] + | ^^^^^^^^ + = note: `#[deny(unused_parens)]` implied by `#[deny(warnings)]` +help: remove these parentheses + | +LL - ((for<'a> fn(Inv<'a>)),): Trait, +LL + (for<'a> fn(Inv<'a>),): Trait, + | + +error: aborting due to previous error + From c67903ef21d18024f14609a7996fcf14b6b8d5b6 Mon Sep 17 00:00:00 2001 From: yukang Date: Sat, 14 Jan 2023 16:52:46 +0800 Subject: [PATCH 2/4] fix issues in unused lint --- compiler/rustc_lint/src/early.rs | 8 +--- compiler/rustc_lint/src/lib.rs | 2 +- compiler/rustc_lint/src/passes.rs | 3 ++ compiler/rustc_lint/src/unused.rs | 51 +++++++++++++++++----- library/std/src/io/error/repr_bitpacked.rs | 8 ++-- 5 files changed, 50 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_lint/src/early.rs b/compiler/rustc_lint/src/early.rs index 3901751c79fb0..337a19dd024d2 100644 --- a/compiler/rustc_lint/src/early.rs +++ b/compiler/rustc_lint/src/early.rs @@ -248,13 +248,9 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T> } fn visit_where_predicate(&mut self, p: &'a ast::WherePredicate) { - use rustc_ast::{WhereBoundPredicate, WherePredicate}; - if let WherePredicate::BoundPredicate(WhereBoundPredicate { bounded_ty, .. }) = p && - let ast::TyKind::BareFn(b) = &bounded_ty.kind && - b.generic_params.len() > 0 { - return; - } + lint_callback!(self, enter_where_predicate, p); ast_visit::walk_where_predicate(self, p); + lint_callback!(self, exit_where_predicate, p); } fn visit_poly_trait_ref(&mut self, t: &'a ast::PolyTraitRef) { diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 3d818154cb94f..d6be4da03286f 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -145,7 +145,7 @@ early_lint_methods!( [ pub BuiltinCombinedEarlyLintPass, [ - UnusedParens: UnusedParens, + UnusedParens: UnusedParens::new(), UnusedBraces: UnusedBraces, UnusedImportBraces: UnusedImportBraces, UnsafeCode: UnsafeCode, diff --git a/compiler/rustc_lint/src/passes.rs b/compiler/rustc_lint/src/passes.rs index 5558156a4b9ef..0bf01c4e56781 100644 --- a/compiler/rustc_lint/src/passes.rs +++ b/compiler/rustc_lint/src/passes.rs @@ -171,6 +171,9 @@ macro_rules! early_lint_methods { /// Counterpart to `enter_lint_attrs`. fn exit_lint_attrs(a: &[ast::Attribute]); + + fn enter_where_predicate(a: &ast::WherePredicate); + fn exit_where_predicate(a: &ast::WherePredicate); ]); ) } diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index 94a3313810717..65f2644a858af 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -824,7 +824,17 @@ declare_lint! { "`if`, `match`, `while` and `return` do not need parentheses" } -declare_lint_pass!(UnusedParens => [UNUSED_PARENS]); +pub struct UnusedParens { + with_self_ty_parens: bool, +} + +impl UnusedParens { + pub fn new() -> Self { + Self { with_self_ty_parens: false } + } +} + +impl_lint_pass!(UnusedParens => [UNUSED_PARENS]); impl UnusedDelimLint for UnusedParens { const DELIM_STR: &'static str = "parentheses"; @@ -999,20 +1009,22 @@ impl EarlyLintPass for UnusedParens { } fn check_ty(&mut self, cx: &EarlyContext<'_>, ty: &ast::Ty) { + if let ast::TyKind::Array(_, len) = &ty.kind { + self.check_unused_delims_expr( + cx, + &len.value, + UnusedDelimsCtx::ArrayLenExpr, + false, + None, + None, + ); + } if let ast::TyKind::Paren(r) = &ty.kind { match &r.kind { ast::TyKind::TraitObject(..) => {} + ast::TyKind::BareFn(b) + if self.with_self_ty_parens && b.generic_params.len() > 0 => {} ast::TyKind::ImplTrait(_, bounds) if bounds.len() > 1 => {} - ast::TyKind::Array(_, len) => { - self.check_unused_delims_expr( - cx, - &len.value, - UnusedDelimsCtx::ArrayLenExpr, - false, - None, - None, - ); - } _ => { let spans = if let Some(r) = r.span.find_ancestor_inside(ty.span) { Some((ty.span.with_hi(r.lo()), ty.span.with_lo(r.hi()))) @@ -1028,6 +1040,23 @@ impl EarlyLintPass for UnusedParens { fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) { ::check_item(self, cx, item) } + + fn enter_where_predicate(&mut self, _: &EarlyContext<'_>, pred: &ast::WherePredicate) { + use rustc_ast::{WhereBoundPredicate, WherePredicate}; + if let WherePredicate::BoundPredicate(WhereBoundPredicate { + bounded_ty, + bound_generic_params, + .. + }) = pred && + let ast::TyKind::Paren(_) = &bounded_ty.kind && + bound_generic_params.is_empty() { + self.with_self_ty_parens = true; + } + } + + fn exit_where_predicate(&mut self, _: &EarlyContext<'_>, _: &ast::WherePredicate) { + self.with_self_ty_parens = false; + } } declare_lint! { diff --git a/library/std/src/io/error/repr_bitpacked.rs b/library/std/src/io/error/repr_bitpacked.rs index 601c01c2128c8..3581484050dd1 100644 --- a/library/std/src/io/error/repr_bitpacked.rs +++ b/library/std/src/io/error/repr_bitpacked.rs @@ -374,10 +374,10 @@ static_assert!((TAG_MASK + 1).is_power_of_two()); static_assert!(align_of::() >= TAG_MASK + 1); static_assert!(align_of::() >= TAG_MASK + 1); -static_assert!(@usize_eq: (TAG_MASK & TAG_SIMPLE_MESSAGE), TAG_SIMPLE_MESSAGE); -static_assert!(@usize_eq: (TAG_MASK & TAG_CUSTOM), TAG_CUSTOM); -static_assert!(@usize_eq: (TAG_MASK & TAG_OS), TAG_OS); -static_assert!(@usize_eq: (TAG_MASK & TAG_SIMPLE), TAG_SIMPLE); +static_assert!(@usize_eq: TAG_MASK & TAG_SIMPLE_MESSAGE, TAG_SIMPLE_MESSAGE); +static_assert!(@usize_eq: TAG_MASK & TAG_CUSTOM, TAG_CUSTOM); +static_assert!(@usize_eq: TAG_MASK & TAG_OS, TAG_OS); +static_assert!(@usize_eq: TAG_MASK & TAG_SIMPLE, TAG_SIMPLE); // This is obviously true (`TAG_CUSTOM` is `0b01`), but in `Repr::new_custom` we // offset a pointer by this value, and expect it to both be within the same From 644ee8d2507f80dab7408c90102517e8c9321b5e Mon Sep 17 00:00:00 2001 From: yukang Date: Sat, 14 Jan 2023 17:03:25 +0800 Subject: [PATCH 3/4] add test case for issue 105601 --- .../ui/lint/unused/issue-105061-array-lint.rs | 11 ++++ .../unused/issue-105061-array-lint.stderr | 56 +++++++++++++++++++ .../lint/unused/issue-105061-should-lint.rs | 17 ++++++ .../unused/issue-105061-should-lint.stderr | 20 +++++++ 4 files changed, 104 insertions(+) create mode 100644 tests/ui/lint/unused/issue-105061-array-lint.rs create mode 100644 tests/ui/lint/unused/issue-105061-array-lint.stderr create mode 100644 tests/ui/lint/unused/issue-105061-should-lint.rs create mode 100644 tests/ui/lint/unused/issue-105061-should-lint.stderr diff --git a/tests/ui/lint/unused/issue-105061-array-lint.rs b/tests/ui/lint/unused/issue-105061-array-lint.rs new file mode 100644 index 0000000000000..9b06a4fde04d7 --- /dev/null +++ b/tests/ui/lint/unused/issue-105061-array-lint.rs @@ -0,0 +1,11 @@ +#![warn(unused)] +#![deny(warnings)] + +fn main() { + let _x: ([u32; 3]); //~ ERROR unnecessary parentheses around type + let _y: [u8; (3)]; //~ ERROR unnecessary parentheses around const expression + let _z: ([u8; (3)]); + //~^ ERROR unnecessary parentheses around const expression + //~| ERROR unnecessary parentheses around type + +} diff --git a/tests/ui/lint/unused/issue-105061-array-lint.stderr b/tests/ui/lint/unused/issue-105061-array-lint.stderr new file mode 100644 index 0000000000000..7eb761aee431f --- /dev/null +++ b/tests/ui/lint/unused/issue-105061-array-lint.stderr @@ -0,0 +1,56 @@ +error: unnecessary parentheses around type + --> $DIR/issue-105061-array-lint.rs:5:13 + | +LL | let _x: ([u32; 3]); + | ^ ^ + | +note: the lint level is defined here + --> $DIR/issue-105061-array-lint.rs:2:9 + | +LL | #![deny(warnings)] + | ^^^^^^^^ + = note: `#[deny(unused_parens)]` implied by `#[deny(warnings)]` +help: remove these parentheses + | +LL - let _x: ([u32; 3]); +LL + let _x: [u32; 3]; + | + +error: unnecessary parentheses around const expression + --> $DIR/issue-105061-array-lint.rs:6:18 + | +LL | let _y: [u8; (3)]; + | ^ ^ + | +help: remove these parentheses + | +LL - let _y: [u8; (3)]; +LL + let _y: [u8; 3]; + | + +error: unnecessary parentheses around type + --> $DIR/issue-105061-array-lint.rs:7:13 + | +LL | let _z: ([u8; (3)]); + | ^ ^ + | +help: remove these parentheses + | +LL - let _z: ([u8; (3)]); +LL + let _z: [u8; (3)]; + | + +error: unnecessary parentheses around const expression + --> $DIR/issue-105061-array-lint.rs:7:19 + | +LL | let _z: ([u8; (3)]); + | ^ ^ + | +help: remove these parentheses + | +LL - let _z: ([u8; (3)]); +LL + let _z: ([u8; 3]); + | + +error: aborting due to 4 previous errors + diff --git a/tests/ui/lint/unused/issue-105061-should-lint.rs b/tests/ui/lint/unused/issue-105061-should-lint.rs new file mode 100644 index 0000000000000..ff47e1734f7b0 --- /dev/null +++ b/tests/ui/lint/unused/issue-105061-should-lint.rs @@ -0,0 +1,17 @@ +#![warn(unused)] +#![deny(warnings)] + +struct Inv<'a>(&'a mut &'a ()); + +trait Trait<'a> {} +impl<'b> Trait<'b> for for<'a> fn(Inv<'a>) {} + + +fn with_bound() +where + for<'b> (for<'a> fn(Inv<'a>)): Trait<'b>, //~ ERROR unnecessary parentheses around type +{} + +fn main() { + with_bound(); +} diff --git a/tests/ui/lint/unused/issue-105061-should-lint.stderr b/tests/ui/lint/unused/issue-105061-should-lint.stderr new file mode 100644 index 0000000000000..60b1af71e0e56 --- /dev/null +++ b/tests/ui/lint/unused/issue-105061-should-lint.stderr @@ -0,0 +1,20 @@ +error: unnecessary parentheses around type + --> $DIR/issue-105061-should-lint.rs:12:13 + | +LL | for<'b> (for<'a> fn(Inv<'a>)): Trait<'b>, + | ^ ^ + | +note: the lint level is defined here + --> $DIR/issue-105061-should-lint.rs:2:9 + | +LL | #![deny(warnings)] + | ^^^^^^^^ + = note: `#[deny(unused_parens)]` implied by `#[deny(warnings)]` +help: remove these parentheses + | +LL - for<'b> (for<'a> fn(Inv<'a>)): Trait<'b>, +LL + for<'b> for<'a> fn(Inv<'a>): Trait<'b>, + | + +error: aborting due to previous error + From 9d74bb832f2529535a9896ba0ff2797679907415 Mon Sep 17 00:00:00 2001 From: yukang Date: Mon, 16 Jan 2023 20:44:14 +0800 Subject: [PATCH 4/4] comments feedback --- compiler/rustc_lint/src/unused.rs | 52 ++++++++++--------- .../lint/unused/issue-105061-should-lint.rs | 8 ++- .../unused/issue-105061-should-lint.stderr | 16 +++++- 3 files changed, 49 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index 65f2644a858af..e40530a6dd67a 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -1009,31 +1009,35 @@ impl EarlyLintPass for UnusedParens { } fn check_ty(&mut self, cx: &EarlyContext<'_>, ty: &ast::Ty) { - if let ast::TyKind::Array(_, len) = &ty.kind { - self.check_unused_delims_expr( - cx, - &len.value, - UnusedDelimsCtx::ArrayLenExpr, - false, - None, - None, - ); - } - if let ast::TyKind::Paren(r) = &ty.kind { - match &r.kind { - ast::TyKind::TraitObject(..) => {} - ast::TyKind::BareFn(b) - if self.with_self_ty_parens && b.generic_params.len() > 0 => {} - ast::TyKind::ImplTrait(_, bounds) if bounds.len() > 1 => {} - _ => { - let spans = if let Some(r) = r.span.find_ancestor_inside(ty.span) { - Some((ty.span.with_hi(r.lo()), ty.span.with_lo(r.hi()))) - } else { - None - }; - self.emit_unused_delims(cx, ty.span, spans, "type", (false, false)); + match &ty.kind { + ast::TyKind::Array(_, len) => { + self.check_unused_delims_expr( + cx, + &len.value, + UnusedDelimsCtx::ArrayLenExpr, + false, + None, + None, + ); + } + ast::TyKind::Paren(r) => { + match &r.kind { + ast::TyKind::TraitObject(..) => {} + ast::TyKind::BareFn(b) + if self.with_self_ty_parens && b.generic_params.len() > 0 => {} + ast::TyKind::ImplTrait(_, bounds) if bounds.len() > 1 => {} + _ => { + let spans = if let Some(r) = r.span.find_ancestor_inside(ty.span) { + Some((ty.span.with_hi(r.lo()), ty.span.with_lo(r.hi()))) + } else { + None + }; + self.emit_unused_delims(cx, ty.span, spans, "type", (false, false)); + } } + self.with_self_ty_parens = false; } + _ => {} } } @@ -1055,7 +1059,7 @@ impl EarlyLintPass for UnusedParens { } fn exit_where_predicate(&mut self, _: &EarlyContext<'_>, _: &ast::WherePredicate) { - self.with_self_ty_parens = false; + assert!(!self.with_self_ty_parens); } } diff --git a/tests/ui/lint/unused/issue-105061-should-lint.rs b/tests/ui/lint/unused/issue-105061-should-lint.rs index ff47e1734f7b0..7e4e09473493a 100644 --- a/tests/ui/lint/unused/issue-105061-should-lint.rs +++ b/tests/ui/lint/unused/issue-105061-should-lint.rs @@ -6,12 +6,18 @@ struct Inv<'a>(&'a mut &'a ()); trait Trait<'a> {} impl<'b> Trait<'b> for for<'a> fn(Inv<'a>) {} - fn with_bound() where for<'b> (for<'a> fn(Inv<'a>)): Trait<'b>, //~ ERROR unnecessary parentheses around type {} +trait Hello {} +fn with_dyn_bound() +where + (dyn Hello<(for<'b> fn(&'b ()))>): Hello //~ ERROR unnecessary parentheses around type +{} + fn main() { with_bound(); + with_dyn_bound(); } diff --git a/tests/ui/lint/unused/issue-105061-should-lint.stderr b/tests/ui/lint/unused/issue-105061-should-lint.stderr index 60b1af71e0e56..e591f1ffb6b89 100644 --- a/tests/ui/lint/unused/issue-105061-should-lint.stderr +++ b/tests/ui/lint/unused/issue-105061-should-lint.stderr @@ -1,5 +1,5 @@ error: unnecessary parentheses around type - --> $DIR/issue-105061-should-lint.rs:12:13 + --> $DIR/issue-105061-should-lint.rs:11:13 | LL | for<'b> (for<'a> fn(Inv<'a>)): Trait<'b>, | ^ ^ @@ -16,5 +16,17 @@ LL - for<'b> (for<'a> fn(Inv<'a>)): Trait<'b>, LL + for<'b> for<'a> fn(Inv<'a>): Trait<'b>, | -error: aborting due to previous error +error: unnecessary parentheses around type + --> $DIR/issue-105061-should-lint.rs:17:16 + | +LL | (dyn Hello<(for<'b> fn(&'b ()))>): Hello + | ^ ^ + | +help: remove these parentheses + | +LL - (dyn Hello<(for<'b> fn(&'b ()))>): Hello +LL + (dyn Hello fn(&'b ())>): Hello + | + +error: aborting due to 2 previous errors