Skip to content

Commit

Permalink
Improve diagnostic for const ctors in array repeat expressions
Browse files Browse the repository at this point in the history
  • Loading branch information
clubby789 committed Jul 24, 2023
1 parent c06b2b9 commit 86b1122
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 28 deletions.
35 changes: 28 additions & 7 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
26 changes: 23 additions & 3 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 \
<https://github.com/rust-lang/rfcs/pull/2920> for more information",
Expand Down
6 changes: 5 additions & 1 deletion tests/ui/consts/const-blocks/fn-call-in-non-const.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,17 @@ LL | let _: [Option<Bar>; 2] = [no_copy(); 2];
|
= note: required for `Option<Bar>` 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 <https://github.com/rust-lang/rfcs/pull/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<Bar> = no_copy();
LL ~ let _: [Option<Bar>; 2] = [ARRAY_REPEAT_VALUE; 2];
|

error: aborting due to previous error

Expand Down
6 changes: 5 additions & 1 deletion tests/ui/consts/const-blocks/trait-error.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,13 @@ note: required for `Foo<String>` 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 <https://github.com/rust-lang/rfcs/pull/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<String> = Foo(String::new());
LL ~ [ARRAY_REPEAT_VALUE; 4];
|

error: aborting due to previous error

Expand Down
8 changes: 6 additions & 2 deletions tests/ui/consts/const-fn-in-vec.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
static _MAYBE_STRINGS: [Option<String>; 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<String>; 5] = [None; 5];
//~^ ERROR the trait bound `String: Copy` is not satisfied
println!("{:?}", strings);
}
44 changes: 39 additions & 5 deletions tests/ui/consts/const-fn-in-vec.stderr
Original file line number Diff line number Diff line change
@@ -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<String>; 5] = [None; 5];
| ^^^^ the trait `Copy` is not implemented for `String`
|
= note: required for `Option<String>` 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 <https://github.com/rust-lang/rfcs/pull/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<String> = None;
LL ~ static _MAYBE_STRINGS: [Option<String>; 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 <https://github.com/rust-lang/rfcs/pull/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<String>; 5] = [None; 5];
| ^^^^ the trait `Copy` is not implemented for `String`
|
= note: required for `Option<String>` 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 <https://github.com/rust-lang/rfcs/pull/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<String> = None;
LL ~ let _maybe_strings: [Option<String>; 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`.

0 comments on commit 86b1122

Please sign in to comment.