From 86b112204ade0203d1c2272a523e26b4593fb60a Mon Sep 17 00:00:00 2001 From: clubby789 Date: Fri, 21 Jul 2023 15:59:28 +0000 Subject: [PATCH] Improve diagnostic for const ctors in array repeat expressions --- compiler/rustc_hir_typeck/src/expr.rs | 35 ++++++++++++--- compiler/rustc_middle/src/traits/mod.rs | 26 +++++++++-- .../src/traits/error_reporting/suggestions.rs | 29 ++++++++---- .../const-blocks/fn-call-in-non-const.stderr | 6 ++- .../ui/consts/const-blocks/trait-error.stderr | 6 ++- tests/ui/consts/const-fn-in-vec.rs | 8 +++- tests/ui/consts/const-fn-in-vec.stderr | 44 ++++++++++++++++--- 7 files changed, 126 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 29488c9011a19..b04d83a884f56 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -1507,21 +1507,42 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } _ => {} } - // If someone calls a const fn, they can extract that call out into a separate constant (or a const - // block in the future), so we check that to tell them that in the diagnostic. Does not affect typeck. - let is_const_fn = match element.kind { + // If someone calls a const fn or constructs a const value, they can extract that + // out into a separate constant (or a const block in the future), so we check that + // to tell them that in the diagnostic. Does not affect typeck. + let is_constable = match element.kind { hir::ExprKind::Call(func, _args) => match *self.node_ty(func.hir_id).kind() { - ty::FnDef(def_id, _) => tcx.is_const_fn(def_id), - _ => false, + ty::FnDef(def_id, _) if tcx.is_const_fn(def_id) => traits::IsConstable::Fn, + _ => traits::IsConstable::No, }, - _ => false, + hir::ExprKind::Path(qpath) => { + match self.typeck_results.borrow().qpath_res(&qpath, element.hir_id) { + Res::Def(DefKind::Ctor(_, CtorKind::Const), _) => traits::IsConstable::Ctor, + _ => traits::IsConstable::No, + } + } + _ => traits::IsConstable::No, }; // If the length is 0, we don't create any elements, so we don't copy any. If the length is 1, we // don't copy that one element, we move it. Only check for Copy if the length is larger. if count.try_eval_target_usize(tcx, self.param_env).map_or(true, |len| len > 1) { let lang_item = self.tcx.require_lang_item(LangItem::Copy, None); - let code = traits::ObligationCauseCode::RepeatElementCopy { is_const_fn }; + let code = traits::ObligationCauseCode::RepeatElementCopy { + is_constable, + elt_type: element_ty, + elt_span: element.span, + elt_stmt_span: self + .tcx + .hir() + .parent_iter(element.hir_id) + .find_map(|(_, node)| match node { + hir::Node::Item(it) => Some(it.span), + hir::Node::Stmt(stmt) => Some(stmt.span), + _ => None, + }) + .expect("array repeat expressions must be inside an item or statement"), + }; self.require_type_meets(element_ty, element.span, code, lang_item); } } diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index b7ffed57a0be0..2e1c3fe1fed24 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -301,9 +301,14 @@ pub enum ObligationCauseCode<'tcx> { InlineAsmSized, /// `[expr; N]` requires `type_of(expr): Copy`. RepeatElementCopy { - /// If element is a `const fn` we display a help message suggesting to move the - /// function call to a new `const` item while saying that `T` doesn't implement `Copy`. - is_const_fn: bool, + /// If element is a `const fn` or const ctor we display a help message suggesting + /// to move it to a new `const` item while saying that `T` doesn't implement `Copy`. + is_constable: IsConstable, + elt_type: Ty<'tcx>, + elt_span: Span, + /// Span of the statement/item in which the repeat expression occurs. We can use this to + /// place a `const` declaration before it + elt_stmt_span: Span, }, /// Types of fields (other than the last, except for packed structs) in a struct must be sized. @@ -448,6 +453,21 @@ pub enum ObligationCauseCode<'tcx> { TypeAlias(InternedObligationCauseCode<'tcx>, Span, DefId), } +/// Whether a value can be extracted into a const. +/// Used for diagnostics around array repeat expressions. +#[derive(Copy, Clone, Debug, PartialEq, Eq, HashStable, TyEncodable, TyDecodable)] +pub enum IsConstable { + No, + /// Call to a const fn + Fn, + /// Use of a const ctor + Ctor, +} + +crate::TrivialTypeTraversalAndLiftImpls! { + IsConstable, +} + /// The 'location' at which we try to perform HIR-based wf checking. /// This information is used to obtain an `hir::Ty`, which /// we can walk in order to obtain precise spans for any diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 073a2a2b1a0c9..edae9e10818b4 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -27,6 +27,7 @@ use rustc_infer::infer::error_reporting::TypeErrCtxt; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use rustc_infer::infer::{DefineOpaqueTypes, InferOk, LateBoundRegionConversionTime}; use rustc_middle::hir::map; +use rustc_middle::traits::IsConstable; use rustc_middle::ty::error::TypeError::{self, Sorts}; use rustc_middle::ty::{ self, suggest_arbitrary_trait_bound, suggest_constraining_type_param, AdtKind, @@ -2832,20 +2833,30 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { )); } } - ObligationCauseCode::RepeatElementCopy { is_const_fn } => { + ObligationCauseCode::RepeatElementCopy { is_constable, elt_type, elt_span, elt_stmt_span } => { err.note( "the `Copy` trait is required because this value will be copied for each element of the array", ); - - if is_const_fn { - err.help( - "consider creating a new `const` item and initializing it with the result \ - of the function call to be used in the repeat position, like \ - `const VAL: Type = const_fn();` and `let x = [VAL; 42];`", - ); + let value_kind = match is_constable { + IsConstable::Fn => Some("the result of the function call"), + IsConstable::Ctor => Some("the result of the constructor"), + _ => None + }; + let sm = tcx.sess.source_map(); + if let Some(value_kind) = value_kind && + let Ok(snip) = sm.span_to_snippet(elt_span) + { + let help_msg = format!( + "consider creating a new `const` item and initializing it with {value_kind} \ + to be used in the repeat position"); + let indentation = sm.indentation_before(elt_stmt_span).unwrap_or_default(); + err.multipart_suggestion(help_msg, vec![ + (elt_stmt_span.shrink_to_lo(), format!("const ARRAY_REPEAT_VALUE: {elt_type} = {snip};\n{indentation}")), + (elt_span, "ARRAY_REPEAT_VALUE".to_string()) + ], Applicability::MachineApplicable); } - if self.tcx.sess.is_nightly_build() && is_const_fn { + if self.tcx.sess.is_nightly_build() && matches!(is_constable, IsConstable::Fn|IsConstable::Ctor) { err.help( "create an inline `const` block, see RFC #2920 \ for more information", diff --git a/tests/ui/consts/const-blocks/fn-call-in-non-const.stderr b/tests/ui/consts/const-blocks/fn-call-in-non-const.stderr index 174103eeba4c4..eb8b8ac753430 100644 --- a/tests/ui/consts/const-blocks/fn-call-in-non-const.stderr +++ b/tests/ui/consts/const-blocks/fn-call-in-non-const.stderr @@ -6,13 +6,17 @@ LL | let _: [Option; 2] = [no_copy(); 2]; | = note: required for `Option` to implement `Copy` = note: the `Copy` trait is required because this value will be copied for each element of the array - = help: consider creating a new `const` item and initializing it with the result of the function call to be used in the repeat position, like `const VAL: Type = const_fn();` and `let x = [VAL; 42];` = help: create an inline `const` block, see RFC #2920 for more information help: consider annotating `Bar` with `#[derive(Copy)]` | LL + #[derive(Copy)] LL | struct Bar; | +help: consider creating a new `const` item and initializing it with the result of the function call to be used in the repeat position + | +LL ~ const ARRAY_REPEAT_VALUE: Option = no_copy(); +LL ~ let _: [Option; 2] = [ARRAY_REPEAT_VALUE; 2]; + | error: aborting due to previous error diff --git a/tests/ui/consts/const-blocks/trait-error.stderr b/tests/ui/consts/const-blocks/trait-error.stderr index 06fa4b0b1f30c..858ffa820e215 100644 --- a/tests/ui/consts/const-blocks/trait-error.stderr +++ b/tests/ui/consts/const-blocks/trait-error.stderr @@ -10,9 +10,13 @@ note: required for `Foo` to implement `Copy` LL | #[derive(Copy, Clone)] | ^^^^ unsatisfied trait bound introduced in this `derive` macro = note: the `Copy` trait is required because this value will be copied for each element of the array - = help: consider creating a new `const` item and initializing it with the result of the function call to be used in the repeat position, like `const VAL: Type = const_fn();` and `let x = [VAL; 42];` = help: create an inline `const` block, see RFC #2920 for more information = note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider creating a new `const` item and initializing it with the result of the function call to be used in the repeat position + | +LL ~ const ARRAY_REPEAT_VALUE: Foo = Foo(String::new()); +LL ~ [ARRAY_REPEAT_VALUE; 4]; + | error: aborting due to previous error diff --git a/tests/ui/consts/const-fn-in-vec.rs b/tests/ui/consts/const-fn-in-vec.rs index a40290eca0972..0483800efefd1 100644 --- a/tests/ui/consts/const-fn-in-vec.rs +++ b/tests/ui/consts/const-fn-in-vec.rs @@ -1,7 +1,11 @@ +static _MAYBE_STRINGS: [Option; 5] = [None; 5]; +//~^ ERROR the trait bound `String: Copy` is not satisfied + fn main() { // should hint to create an inline `const` block // or to create a new `const` item - let strings: [String; 5] = [String::new(); 5]; + let _strings: [String; 5] = [String::new(); 5]; + //~^ ERROR the trait bound `String: Copy` is not satisfied + let _maybe_strings: [Option; 5] = [None; 5]; //~^ ERROR the trait bound `String: Copy` is not satisfied - println!("{:?}", strings); } diff --git a/tests/ui/consts/const-fn-in-vec.stderr b/tests/ui/consts/const-fn-in-vec.stderr index 9eb7524b5044b..4593034bfaea3 100644 --- a/tests/ui/consts/const-fn-in-vec.stderr +++ b/tests/ui/consts/const-fn-in-vec.stderr @@ -1,13 +1,47 @@ error[E0277]: the trait bound `String: Copy` is not satisfied - --> $DIR/const-fn-in-vec.rs:4:33 + --> $DIR/const-fn-in-vec.rs:1:47 | -LL | let strings: [String; 5] = [String::new(); 5]; - | ^^^^^^^^^^^^^ the trait `Copy` is not implemented for `String` +LL | static _MAYBE_STRINGS: [Option; 5] = [None; 5]; + | ^^^^ the trait `Copy` is not implemented for `String` | + = note: required for `Option` to implement `Copy` = note: the `Copy` trait is required because this value will be copied for each element of the array - = help: consider creating a new `const` item and initializing it with the result of the function call to be used in the repeat position, like `const VAL: Type = const_fn();` and `let x = [VAL; 42];` = help: create an inline `const` block, see RFC #2920 for more information +help: consider creating a new `const` item and initializing it with the result of the constructor to be used in the repeat position + | +LL + const ARRAY_REPEAT_VALUE: Option = None; +LL ~ static _MAYBE_STRINGS: [Option; 5] = [ARRAY_REPEAT_VALUE; 5]; + | + +error[E0277]: the trait bound `String: Copy` is not satisfied + --> $DIR/const-fn-in-vec.rs:7:34 + | +LL | let _strings: [String; 5] = [String::new(); 5]; + | ^^^^^^^^^^^^^ the trait `Copy` is not implemented for `String` + | + = note: the `Copy` trait is required because this value will be copied for each element of the array + = help: create an inline `const` block, see RFC #2920 for more information +help: consider creating a new `const` item and initializing it with the result of the function call to be used in the repeat position + | +LL ~ const ARRAY_REPEAT_VALUE: String = String::new(); +LL ~ let _strings: [String; 5] = [ARRAY_REPEAT_VALUE; 5]; + | + +error[E0277]: the trait bound `String: Copy` is not satisfied + --> $DIR/const-fn-in-vec.rs:9:48 + | +LL | let _maybe_strings: [Option; 5] = [None; 5]; + | ^^^^ the trait `Copy` is not implemented for `String` + | + = note: required for `Option` to implement `Copy` + = note: the `Copy` trait is required because this value will be copied for each element of the array + = help: create an inline `const` block, see RFC #2920 for more information +help: consider creating a new `const` item and initializing it with the result of the constructor to be used in the repeat position + | +LL ~ const ARRAY_REPEAT_VALUE: Option = None; +LL ~ let _maybe_strings: [Option; 5] = [ARRAY_REPEAT_VALUE; 5]; + | -error: aborting due to previous error +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0277`.