From 37c21d6fc714748bb41bb1b9a53152632849a3f3 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Mon, 18 Jul 2022 21:55:47 +0400 Subject: [PATCH 1/5] Do not suggest "wrapping the expression in `std::num::NonZeroU64`" --- compiler/rustc_typeck/src/check/demand.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index 740261cfe74..867570b42da 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -19,6 +19,7 @@ use rustc_span::{BytePos, Span}; use super::method::probe; use std::iter; +use std::ops::Bound; impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pub fn emit_coerce_suggestions( @@ -347,6 +348,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + // Avoid suggesting wrapping in `NonZeroU64` and alike + if self.tcx.layout_scalar_valid_range(expected_adt.did()) + != (Bound::Unbounded, Bound::Unbounded) + { + return; + } + let compatible_variants: Vec = expected_adt .variants() .iter() From 7163e7ff6542b200971a783dc2a720a0ff676e70 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Tue, 19 Jul 2022 00:11:21 +0400 Subject: [PATCH 2/5] Suggest a fix for `NonZero*` <- `*` coercion error --- compiler/rustc_span/src/symbol.rs | 10 ++++ compiler/rustc_typeck/src/check/demand.rs | 54 +++++++++++++++++++ library/core/src/num/nonzero.rs | 1 + .../non_zero_assigned_something.rs | 9 ++++ .../non_zero_assigned_something.stderr | 31 +++++++++++ 5 files changed, 105 insertions(+) create mode 100644 src/test/ui/mismatched_types/non_zero_assigned_something.rs create mode 100644 src/test/ui/mismatched_types/non_zero_assigned_something.stderr diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index d5e65c0b442..6ddcfbb05dc 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -223,6 +223,16 @@ symbols! { LintPass, Mutex, N, + NonZeroI128, + NonZeroI16, + NonZeroI32, + NonZeroI64, + NonZeroI8, + NonZeroU128, + NonZeroU16, + NonZeroU32, + NonZeroU64, + NonZeroU8, None, Ok, Option, diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index 867570b42da..31da5cfe7fa 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -34,6 +34,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.annotate_expected_due_to_let_ty(err, expr, error); self.suggest_deref_ref_or_into(err, expr, expected, expr_ty, expected_ty_expr); self.suggest_compatible_variants(err, expr, expected, expr_ty); + self.suggest_non_zero_new_unwrap(err, expr, expected, expr_ty); if self.suggest_calling_boxed_future_when_appropriate(err, expr, expected, expr_ty) { return; } @@ -418,6 +419,59 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + fn suggest_non_zero_new_unwrap( + &self, + err: &mut Diagnostic, + expr: &hir::Expr<'_>, + expected: Ty<'tcx>, + expr_ty: Ty<'tcx>, + ) { + let tcx = self.tcx; + let (adt, unwrap) = match expected.kind() { + // In case Option is wanted, but * is provided, suggest calling new + ty::Adt(adt, substs) if tcx.is_diagnostic_item(sym::Option, adt.did()) => { + // Unwrap option + let Some(fst) = substs.first() else { return }; + let ty::Adt(adt, _) = fst.expect_ty().kind() else { return }; + + (adt, "") + } + // In case NonZero* is wanted, but * is provided also add `.unwrap()` to satisfy types + ty::Adt(adt, _) => (adt, ".unwrap()"), + _ => return, + }; + + let map = [ + (sym::NonZeroU8, tcx.types.u8), + (sym::NonZeroU16, tcx.types.u16), + (sym::NonZeroU32, tcx.types.u32), + (sym::NonZeroU64, tcx.types.u64), + (sym::NonZeroU128, tcx.types.u128), + (sym::NonZeroI8, tcx.types.i8), + (sym::NonZeroI16, tcx.types.i16), + (sym::NonZeroI32, tcx.types.i32), + (sym::NonZeroI64, tcx.types.i64), + (sym::NonZeroI128, tcx.types.i128), + ]; + + let Some((s, _)) = map + .iter() + .find(|&&(s, _)| self.tcx.is_diagnostic_item(s, adt.did())) + .filter(|&&(_, t)| { self.can_coerce(expr_ty, t) }) + else { return }; + + let path = self.tcx.def_path_str(adt.non_enum_variant().def_id); + + err.multipart_suggestion( + format!("consider calling `{s}::new`"), + vec![ + (expr.span.shrink_to_lo(), format!("{path}::new(")), + (expr.span.shrink_to_hi(), format!("){unwrap}")), + ], + Applicability::MaybeIncorrect, + ); + } + pub fn get_conversion_methods( &self, span: Span, diff --git a/library/core/src/num/nonzero.rs b/library/core/src/num/nonzero.rs index 92d03b724b4..4de0a0cf564 100644 --- a/library/core/src/num/nonzero.rs +++ b/library/core/src/num/nonzero.rs @@ -39,6 +39,7 @@ macro_rules! nonzero_integers { #[repr(transparent)] #[rustc_layout_scalar_valid_range_start(1)] #[rustc_nonnull_optimization_guaranteed] + #[rustc_diagnostic_item = stringify!($Ty)] pub struct $Ty($Int); impl $Ty { diff --git a/src/test/ui/mismatched_types/non_zero_assigned_something.rs b/src/test/ui/mismatched_types/non_zero_assigned_something.rs new file mode 100644 index 00000000000..d2adbe01c18 --- /dev/null +++ b/src/test/ui/mismatched_types/non_zero_assigned_something.rs @@ -0,0 +1,9 @@ +fn main() { + let _: std::num::NonZeroU64 = 1; + //~^ ERROR mismatched types + //~| HELP consider calling `NonZeroU64::new` + + let _: Option = 1; + //~^ ERROR mismatched types + //~| HELP consider calling `NonZeroU64::new` +} diff --git a/src/test/ui/mismatched_types/non_zero_assigned_something.stderr b/src/test/ui/mismatched_types/non_zero_assigned_something.stderr new file mode 100644 index 00000000000..d4b2c902f9b --- /dev/null +++ b/src/test/ui/mismatched_types/non_zero_assigned_something.stderr @@ -0,0 +1,31 @@ +error[E0308]: mismatched types + --> $DIR/non_zero_assigned_something.rs:2:35 + | +LL | let _: std::num::NonZeroU64 = 1; + | -------------------- ^ expected struct `NonZeroU64`, found integer + | | + | expected due to this + | +help: consider calling `NonZeroU64::new` + | +LL | let _: std::num::NonZeroU64 = NonZeroU64::new(1).unwrap(); + | ++++++++++++++++ ++++++++++ + +error[E0308]: mismatched types + --> $DIR/non_zero_assigned_something.rs:6:43 + | +LL | let _: Option = 1; + | ---------------------------- ^ expected enum `Option`, found integer + | | + | expected due to this + | + = note: expected enum `Option` + found type `{integer}` +help: consider calling `NonZeroU64::new` + | +LL | let _: Option = NonZeroU64::new(1); + | ++++++++++++++++ + + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. From da2752e00f0139fb13282b2bd97aa0f8c665aee9 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Tue, 19 Jul 2022 01:16:25 +0400 Subject: [PATCH 3/5] check accessibility before suggesting wrapping expressions --- compiler/rustc_typeck/src/check/demand.rs | 18 +++--- .../wrap-suggestion-privacy.rs | 24 ++++++++ .../wrap-suggestion-privacy.stderr | 59 +++++++++++++++++++ 3 files changed, 93 insertions(+), 8 deletions(-) create mode 100644 src/test/ui/mismatched_types/wrap-suggestion-privacy.rs create mode 100644 src/test/ui/mismatched_types/wrap-suggestion-privacy.stderr diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index 31da5cfe7fa..30d7cc2e5fc 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -19,7 +19,6 @@ use rustc_span::{BytePos, Span}; use super::method::probe; use std::iter; -use std::ops::Bound; impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pub fn emit_coerce_suggestions( @@ -349,13 +348,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - // Avoid suggesting wrapping in `NonZeroU64` and alike - if self.tcx.layout_scalar_valid_range(expected_adt.did()) - != (Bound::Unbounded, Bound::Unbounded) - { - return; - } - let compatible_variants: Vec = expected_adt .variants() .iter() @@ -364,6 +356,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }) .filter_map(|variant| { let sole_field = &variant.fields[0]; + + if !sole_field.did.is_local() + && !sole_field.vis.is_accessible_from( + self.tcx.parent_module(expr.hir_id).to_def_id(), + self.tcx, + ) + { + return None; + } + let sole_field_ty = sole_field.ty(self.tcx, substs); if self.can_coerce(expr_ty, sole_field_ty) { let variant_path = diff --git a/src/test/ui/mismatched_types/wrap-suggestion-privacy.rs b/src/test/ui/mismatched_types/wrap-suggestion-privacy.rs new file mode 100644 index 00000000000..63cb1a12991 --- /dev/null +++ b/src/test/ui/mismatched_types/wrap-suggestion-privacy.rs @@ -0,0 +1,24 @@ +mod inner { + pub struct Wrapper(T); +} + +fn needs_wrapper(t: inner::Wrapper) {} +fn needs_wrapping(t: std::num::Wrapping) {} +fn needs_ready(t: std::future::Ready) {} + +fn main() { + // Suggest wrapping expression because type is local + // and its privacy can be easily changed + needs_wrapper(0); + //~^ ERROR mismatched types + //~| HELP try wrapping the expression in `inner::Wrapper` + + // Suggest wrapping expression because field is accessible + needs_wrapping(0); + //~^ ERROR mismatched types + //~| HELP try wrapping the expression in `std::num::Wrapping` + + // Do not suggest wrapping expression + needs_ready(Some(0)); + //~^ ERROR mismatched types +} diff --git a/src/test/ui/mismatched_types/wrap-suggestion-privacy.stderr b/src/test/ui/mismatched_types/wrap-suggestion-privacy.stderr new file mode 100644 index 00000000000..536638632de --- /dev/null +++ b/src/test/ui/mismatched_types/wrap-suggestion-privacy.stderr @@ -0,0 +1,59 @@ +error[E0308]: mismatched types + --> $DIR/wrap-suggestion-privacy.rs:12:19 + | +LL | needs_wrapper(0); + | ------------- ^ expected struct `Wrapper`, found integer + | | + | arguments to this function are incorrect + | + = note: expected struct `Wrapper` + found type `{integer}` +note: function defined here + --> $DIR/wrap-suggestion-privacy.rs:5:4 + | +LL | fn needs_wrapper(t: inner::Wrapper) {} + | ^^^^^^^^^^^^^ ---------------------- +help: try wrapping the expression in `inner::Wrapper` + | +LL | needs_wrapper(inner::Wrapper(0)); + | +++++++++++++++ + + +error[E0308]: mismatched types + --> $DIR/wrap-suggestion-privacy.rs:17:20 + | +LL | needs_wrapping(0); + | -------------- ^ expected struct `Wrapping`, found integer + | | + | arguments to this function are incorrect + | + = note: expected struct `Wrapping` + found type `{integer}` +note: function defined here + --> $DIR/wrap-suggestion-privacy.rs:6:4 + | +LL | fn needs_wrapping(t: std::num::Wrapping) {} + | ^^^^^^^^^^^^^^ -------------------------- +help: try wrapping the expression in `std::num::Wrapping` + | +LL | needs_wrapping(std::num::Wrapping(0)); + | +++++++++++++++++++ + + +error[E0308]: mismatched types + --> $DIR/wrap-suggestion-privacy.rs:22:17 + | +LL | needs_ready(Some(0)); + | ----------- ^^^^^^^ expected struct `std::future::Ready`, found enum `Option` + | | + | arguments to this function are incorrect + | + = note: expected struct `std::future::Ready` + found enum `Option<{integer}>` +note: function defined here + --> $DIR/wrap-suggestion-privacy.rs:7:4 + | +LL | fn needs_ready(t: std::future::Ready) {} + | ^^^^^^^^^^^ -------------------------- + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0308`. From 2edad7d77c91145b594ac80c3dc7906998b7674a Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Tue, 19 Jul 2022 02:15:56 +0400 Subject: [PATCH 4/5] Apply suggestions from the review - Use `expr.hir_id.owner` instead of `self.tcx.parent_module(expr.hir_id)` - Use `.type_at()` instead of `.first()` + `.expect_ty()` - Use single `.find()` with `&&` condition Co-authored-by: Michael Goulet --- compiler/rustc_typeck/src/check/demand.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index 30d7cc2e5fc..e5f8b6b5f1e 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -358,10 +358,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let sole_field = &variant.fields[0]; if !sole_field.did.is_local() - && !sole_field.vis.is_accessible_from( - self.tcx.parent_module(expr.hir_id).to_def_id(), - self.tcx, - ) + && !sole_field + .vis + .is_accessible_from(expr.hir_id.owner.to_def_id(), self.tcx) { return None; } @@ -433,8 +432,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // In case Option is wanted, but * is provided, suggest calling new ty::Adt(adt, substs) if tcx.is_diagnostic_item(sym::Option, adt.did()) => { // Unwrap option - let Some(fst) = substs.first() else { return }; - let ty::Adt(adt, _) = fst.expect_ty().kind() else { return }; + let ty::Adt(adt, _) = substs.type_at(0).kind() else { return }; (adt, "") } @@ -458,8 +456,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let Some((s, _)) = map .iter() - .find(|&&(s, _)| self.tcx.is_diagnostic_item(s, adt.did())) - .filter(|&&(_, t)| { self.can_coerce(expr_ty, t) }) + .find(|&&(s, t)| self.tcx.is_diagnostic_item(s, adt.did()) && self.can_coerce(expr_ty, t)) else { return }; let path = self.tcx.def_path_str(adt.non_enum_variant().def_id); From 5bd88dfa8a0b5d5bcdd84d21e2f465719ce8c328 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Tue, 19 Jul 2022 02:33:36 +0400 Subject: [PATCH 5/5] Add a note about privacy to wrapping suggestion --- compiler/rustc_typeck/src/check/demand.rs | 28 +++++++++++-------- .../wrap-suggestion-privacy.stderr | 2 +- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index e5f8b6b5f1e..a2d8765289c 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -348,7 +348,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - let compatible_variants: Vec = expected_adt + let compatible_variants: Vec<(String, Option)> = expected_adt .variants() .iter() .filter(|variant| { @@ -357,14 +357,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .filter_map(|variant| { let sole_field = &variant.fields[0]; - if !sole_field.did.is_local() - && !sole_field - .vis - .is_accessible_from(expr.hir_id.owner.to_def_id(), self.tcx) - { + let field_is_local = sole_field.did.is_local(); + let field_is_accessible = + sole_field.vis.is_accessible_from(expr.hir_id.owner.to_def_id(), self.tcx); + + if !field_is_local && !field_is_accessible { return None; } + let note_about_variant_field_privacy = (field_is_local && !field_is_accessible) + .then(|| format!(" (its field is private, but it's local to this crate and its privacy can be changed)")); + let sole_field_ty = sole_field.ty(self.tcx, substs); if self.can_coerce(expr_ty, sole_field_ty) { let variant_path = @@ -373,9 +376,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if let Some(path) = variant_path.strip_prefix("std::prelude::") && let Some((_, path)) = path.split_once("::") { - return Some(path.to_string()); + return Some((path.to_string(), note_about_variant_field_privacy)); } - Some(variant_path) + Some((variant_path, note_about_variant_field_privacy)) } else { None } @@ -389,10 +392,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { match &compatible_variants[..] { [] => { /* No variants to format */ } - [variant] => { + [(variant, note)] => { // Just a single matching variant. err.multipart_suggestion_verbose( - &format!("try wrapping the expression in `{variant}`"), + &format!( + "try wrapping the expression in `{variant}`{note}", + note = note.as_deref().unwrap_or("") + ), vec![ (expr.span.shrink_to_lo(), format!("{prefix}{variant}(")), (expr.span.shrink_to_hi(), ")".to_string()), @@ -407,7 +413,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { "try wrapping the expression in a variant of `{}`", self.tcx.def_path_str(expected_adt.did()) ), - compatible_variants.into_iter().map(|variant| { + compatible_variants.into_iter().map(|(variant, _)| { vec![ (expr.span.shrink_to_lo(), format!("{prefix}{variant}(")), (expr.span.shrink_to_hi(), ")".to_string()), diff --git a/src/test/ui/mismatched_types/wrap-suggestion-privacy.stderr b/src/test/ui/mismatched_types/wrap-suggestion-privacy.stderr index 536638632de..e8eb8d263ec 100644 --- a/src/test/ui/mismatched_types/wrap-suggestion-privacy.stderr +++ b/src/test/ui/mismatched_types/wrap-suggestion-privacy.stderr @@ -13,7 +13,7 @@ note: function defined here | LL | fn needs_wrapper(t: inner::Wrapper) {} | ^^^^^^^^^^^^^ ---------------------- -help: try wrapping the expression in `inner::Wrapper` +help: try wrapping the expression in `inner::Wrapper` (its field is private, but it's local to this crate and its privacy can be changed) | LL | needs_wrapper(inner::Wrapper(0)); | +++++++++++++++ +