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

Improve suggestions for NonZeroT <- T coercion error #99438

Merged
merged 5 commits into from
Jul 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,16 @@ symbols! {
LintPass,
Mutex,
N,
NonZeroI128,
NonZeroI16,
NonZeroI32,
NonZeroI64,
NonZeroI8,
NonZeroU128,
NonZeroU16,
NonZeroU32,
NonZeroU64,
NonZeroU8,
None,
Ok,
Option,
Expand Down
79 changes: 73 additions & 6 deletions compiler/rustc_typeck/src/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,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;
}
Expand Down Expand Up @@ -347,14 +348,26 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

let compatible_variants: Vec<String> = expected_adt
let compatible_variants: Vec<(String, Option<String>)> = expected_adt
.variants()
.iter()
.filter(|variant| {
variant.fields.len() == 1 && variant.ctor_kind == hir::def::CtorKind::Fn
})
.filter_map(|variant| {
let sole_field = &variant.fields[0];

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 =
Expand All @@ -363,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
}
Expand All @@ -379,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()),
Expand All @@ -397,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()),
Expand All @@ -410,6 +426,57 @@ 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<NonZero*> is wanted, but * is provided, suggest calling new
ty::Adt(adt, substs) if tcx.is_diagnostic_item(sym::Option, adt.did()) => {
// Unwrap option
let ty::Adt(adt, _) = substs.type_at(0).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, 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);

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,
Expand Down
1 change: 1 addition & 0 deletions library/core/src/num/nonzero.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
9 changes: 9 additions & 0 deletions src/test/ui/mismatched_types/non_zero_assigned_something.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
fn main() {
let _: std::num::NonZeroU64 = 1;
//~^ ERROR mismatched types
//~| HELP consider calling `NonZeroU64::new`

let _: Option<std::num::NonZeroU64> = 1;
//~^ ERROR mismatched types
//~| HELP consider calling `NonZeroU64::new`
}
31 changes: 31 additions & 0 deletions src/test/ui/mismatched_types/non_zero_assigned_something.stderr
Original file line number Diff line number Diff line change
@@ -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<std::num::NonZeroU64> = 1;
| ---------------------------- ^ expected enum `Option`, found integer
| |
| expected due to this
|
= note: expected enum `Option<NonZeroU64>`
found type `{integer}`
help: consider calling `NonZeroU64::new`
|
LL | let _: Option<std::num::NonZeroU64> = NonZeroU64::new(1);
| ++++++++++++++++ +

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0308`.
24 changes: 24 additions & 0 deletions src/test/ui/mismatched_types/wrap-suggestion-privacy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
mod inner {
pub struct Wrapper<T>(T);
}

fn needs_wrapper(t: inner::Wrapper<i32>) {}
fn needs_wrapping(t: std::num::Wrapping<i32>) {}
fn needs_ready(t: std::future::Ready<i32>) {}

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
}
59 changes: 59 additions & 0 deletions src/test/ui/mismatched_types/wrap-suggestion-privacy.stderr
Original file line number Diff line number Diff line change
@@ -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<i32>`
found type `{integer}`
note: function defined here
--> $DIR/wrap-suggestion-privacy.rs:5:4
|
LL | fn needs_wrapper(t: inner::Wrapper<i32>) {}
| ^^^^^^^^^^^^^ ----------------------
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));
| +++++++++++++++ +

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<i32>`
found type `{integer}`
note: function defined here
--> $DIR/wrap-suggestion-privacy.rs:6:4
|
LL | fn needs_wrapping(t: std::num::Wrapping<i32>) {}
| ^^^^^^^^^^^^^^ --------------------------
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<i32>`
found enum `Option<{integer}>`
note: function defined here
--> $DIR/wrap-suggestion-privacy.rs:7:4
|
LL | fn needs_ready(t: std::future::Ready<i32>) {}
| ^^^^^^^^^^^ --------------------------

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0308`.