Skip to content

Commit

Permalink
Improve error messages even more
Browse files Browse the repository at this point in the history
  • Loading branch information
@fee1-dead
fee1-dead committed Feb 12, 2022
1 parent 1b0dcdc commit f7f0f84
Show file tree
Hide file tree
Showing 10 changed files with 433 additions and 238 deletions.
23 changes: 11 additions & 12 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file
@@ -1,4 +1,5 @@
use either::Either;
use rustc_const_eval::util::{CallDesugaringKind, CallKind};
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{Applicability, DiagnosticBuilder};
use rustc_hir as hir;
Expand Down Expand Up @@ -26,7 +27,7 @@ use crate::{

use super::{
explain_borrow::{BorrowExplanation, LaterUseKind},
FnSelfUseKind, IncludingDowncast, RegionName, RegionNameSource, UseSpans,
IncludingDowncast, RegionName, RegionNameSource, UseSpans,
};

#[derive(Debug)]
Expand Down Expand Up @@ -195,7 +196,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
.map(|n| format!("`{}`", n))
.unwrap_or_else(|| "value".to_owned());
match kind {
FnSelfUseKind::FnOnceCall => {
CallKind::FnCall(once_did) if Some(once_did) == self.infcx.tcx.lang_items().fn_once_trait() => {
err.span_label(
fn_call_span,
&format!(
Expand All @@ -208,7 +209,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
"this value implements `FnOnce`, which causes it to be moved when called",
);
}
FnSelfUseKind::Operator { self_arg } => {
CallKind::Operator { self_arg, .. } => {
let self_arg = self_arg.unwrap();
err.span_label(
fn_call_span,
&format!(
Expand All @@ -235,12 +237,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
);
}
}
FnSelfUseKind::Normal {
self_arg,
implicit_into_iter,
is_option_or_result,
} => {
if implicit_into_iter {
CallKind::Normal { self_arg, desugaring, is_option_or_result } => {
let self_arg = self_arg.unwrap();
if let Some((CallDesugaringKind::ForLoopIntoIter, _)) = desugaring {
err.span_label(
fn_call_span,
&format!(
Expand Down Expand Up @@ -305,8 +304,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
);
}
}
// Deref::deref takes &self, which cannot cause a move
FnSelfUseKind::DerefCoercion { .. } => unreachable!(),
// Other desugarings takes &self, which cannot cause a move
_ => unreachable!(),
}
} else {
err.span_label(
Expand Down Expand Up @@ -433,7 +432,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}

if let UseSpans::FnSelfUse {
kind: FnSelfUseKind::DerefCoercion { deref_target, deref_target_ty },
kind: CallKind::DerefCoercion { deref_target, deref_target_ty, .. },
..
} = use_spans
{
Expand Down
115 changes: 22 additions & 93 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
View file
@@ -1,10 +1,10 @@
//! Borrow checker diagnostics.

use rustc_const_eval::util::call_kind;
use rustc_errors::DiagnosticBuilder;
use rustc_hir as hir;
use rustc_hir::def::Namespace;
use rustc_hir::def_id::DefId;
use rustc_hir::lang_items::LangItemGroup;
use rustc_hir::GeneratorKind;
use rustc_middle::mir::{
AggregateKind, Constant, FakeReadCause, Field, Local, LocalInfo, LocalKind, Location, Operand,
Expand All @@ -13,7 +13,7 @@ use rustc_middle::mir::{
use rustc_middle::ty::print::Print;
use rustc_middle::ty::{self, DefIdTree, Instance, Ty, TyCtxt};
use rustc_mir_dataflow::move_paths::{InitLocation, LookupResult};
use rustc_span::{hygiene::DesugaringKind, symbol::sym, Span};
use rustc_span::{symbol::sym, Span};
use rustc_target::abi::VariantIdx;

use super::borrow_set::BorrowData;
Expand All @@ -37,7 +37,7 @@ crate use mutability_errors::AccessKind;
crate use outlives_suggestion::OutlivesSuggestionBuilder;
crate use region_errors::{ErrorConstraintInfo, RegionErrorKind, RegionErrors};
crate use region_name::{RegionName, RegionNameSource};
use rustc_span::symbol::Ident;
crate use rustc_const_eval::util::CallKind;

pub(super) struct IncludingDowncast(pub(super) bool);

Expand Down Expand Up @@ -563,46 +563,23 @@ pub(super) enum UseSpans<'tcx> {
fn_call_span: Span,
/// The definition span of the method being called
fn_span: Span,
kind: FnSelfUseKind<'tcx>,
kind: CallKind<'tcx>,
},
/// This access is caused by a `match` or `if let` pattern.
PatUse(Span),
/// This access has a single span associated to it: common case.
OtherUse(Span),
}

#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub(super) enum FnSelfUseKind<'tcx> {
/// A normal method call of the form `receiver.foo(a, b, c)`
Normal {
self_arg: Ident,
implicit_into_iter: bool,
/// Whether the self type of the method call has an `.as_ref()` method.
/// Used for better diagnostics.
is_option_or_result: bool,
},
/// A call to `FnOnce::call_once`, desugared from `my_closure(a, b, c)`
FnOnceCall,
/// A call to an operator trait, desuraged from operator syntax (e.g. `a << b`)
Operator { self_arg: Ident },
DerefCoercion {
/// The `Span` of the `Target` associated type
/// in the `Deref` impl we are using.
deref_target: Span,
/// The type `T::Deref` we are dereferencing to
deref_target_ty: Ty<'tcx>,
},
}

impl UseSpans<'_> {
pub(super) fn args_or_use(self) -> Span {
match self {
UseSpans::ClosureUse { args_span: span, .. }
| UseSpans::PatUse(span)
| UseSpans::OtherUse(span) => span,
UseSpans::FnSelfUse {
fn_call_span, kind: FnSelfUseKind::DerefCoercion { .. }, ..
} => fn_call_span,
UseSpans::FnSelfUse { fn_call_span, kind: CallKind::DerefCoercion { .. }, .. } => {
fn_call_span
}
UseSpans::FnSelfUse { var_span, .. } => var_span,
}
}
Expand All @@ -613,9 +590,9 @@ impl UseSpans<'_> {
UseSpans::ClosureUse { path_span: span, .. }
| UseSpans::PatUse(span)
| UseSpans::OtherUse(span) => span,
UseSpans::FnSelfUse {
fn_call_span, kind: FnSelfUseKind::DerefCoercion { .. }, ..
} => fn_call_span,
UseSpans::FnSelfUse { fn_call_span, kind: CallKind::DerefCoercion { .. }, .. } => {
fn_call_span
}
UseSpans::FnSelfUse { var_span, .. } => var_span,
}
}
Expand All @@ -626,9 +603,9 @@ impl UseSpans<'_> {
UseSpans::ClosureUse { capture_kind_span: span, .. }
| UseSpans::PatUse(span)
| UseSpans::OtherUse(span) => span,
UseSpans::FnSelfUse {
fn_call_span, kind: FnSelfUseKind::DerefCoercion { .. }, ..
} => fn_call_span,
UseSpans::FnSelfUse { fn_call_span, kind: CallKind::DerefCoercion { .. }, .. } => {
fn_call_span
}
UseSpans::FnSelfUse { var_span, .. } => var_span,
}
}
Expand Down Expand Up @@ -904,67 +881,19 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
return normal_ret;
};

let tcx = self.infcx.tcx;
let parent = tcx.parent(method_did);
let is_fn_once = parent == tcx.lang_items().fn_once_trait();
let is_operator = !from_hir_call
&& parent.map_or(false, |p| tcx.lang_items().group(LangItemGroup::Op).contains(&p));
let is_deref = !from_hir_call && tcx.is_diagnostic_item(sym::deref_method, method_did);
let fn_call_span = *fn_span;

let self_arg = tcx.fn_arg_names(method_did)[0];

debug!(
"terminator = {:?} from_hir_call={:?}",
self.body[location.block].terminator, from_hir_call
let kind = call_kind(
self.infcx.tcx,
self.param_env,
method_did,
method_substs,
*fn_span,
*from_hir_call,
Some(self.infcx.tcx.fn_arg_names(method_did)[0]),
);

// Check for a 'special' use of 'self' -
// an FnOnce call, an operator (e.g. `<<`), or a
// deref coercion.
let kind = if is_fn_once {
Some(FnSelfUseKind::FnOnceCall)
} else if is_operator {
Some(FnSelfUseKind::Operator { self_arg })
} else if is_deref {
let deref_target =
tcx.get_diagnostic_item(sym::deref_target).and_then(|deref_target| {
Instance::resolve(tcx, self.param_env, deref_target, method_substs)
.transpose()
});
if let Some(Ok(instance)) = deref_target {
let deref_target_ty = instance.ty(tcx, self.param_env);
Some(FnSelfUseKind::DerefCoercion {
deref_target: tcx.def_span(instance.def_id()),
deref_target_ty,
})
} else {
None
}
} else {
None
};

let kind = kind.unwrap_or_else(|| {
// This isn't a 'special' use of `self`
debug!("move_spans: method_did={:?}, fn_call_span={:?}", method_did, fn_call_span);
let implicit_into_iter = Some(method_did) == tcx.lang_items().into_iter_fn()
&& fn_call_span.desugaring_kind() == Some(DesugaringKind::ForLoop);
let parent_self_ty = parent
.filter(|did| tcx.def_kind(*did) == rustc_hir::def::DefKind::Impl)
.and_then(|did| match tcx.type_of(did).kind() {
ty::Adt(def, ..) => Some(def.did),
_ => None,
});
let is_option_or_result = parent_self_ty.map_or(false, |def_id| {
matches!(tcx.get_diagnostic_name(def_id), Some(sym::Option | sym::Result))
});
FnSelfUseKind::Normal { self_arg, implicit_into_iter, is_option_or_result }
});

return FnSelfUse {
var_span: stmt.source_info.span,
fn_call_span,
fn_call_span: *fn_span,
fn_span: self
.infcx
.tcx
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/move_errors.rs
View file
@@ -1,3 +1,4 @@
use rustc_const_eval::util::CallDesugaringKind;
use rustc_errors::{Applicability, DiagnosticBuilder};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::mir::*;
Expand All @@ -8,7 +9,7 @@ use rustc_mir_dataflow::move_paths::{
use rustc_span::{sym, Span, DUMMY_SP};
use rustc_trait_selection::traits::type_known_to_meet_bound_modulo_regions;

use crate::diagnostics::{FnSelfUseKind, UseSpans};
use crate::diagnostics::{CallKind, UseSpans};
use crate::prefixes::PrefixSet;
use crate::MirBorrowckCtxt;

Expand Down Expand Up @@ -410,7 +411,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
Applicability::MaybeIncorrect,
);
} else if let Some(UseSpans::FnSelfUse {
kind: FnSelfUseKind::Normal { implicit_into_iter: true, .. },
kind:
CallKind::Normal { desugaring: Some((CallDesugaringKind::ForLoopIntoIter, _)), .. },
..
}) = use_spans
{
Expand Down
30 changes: 24 additions & 6 deletions compiler/rustc_const_eval/src/transform/check_consts/check.rs
View file
Expand Up @@ -293,13 +293,13 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
}

/// Emits an error if an expression cannot be evaluated in the current context.
pub fn check_op(&mut self, op: impl NonConstOp) {
pub fn check_op(&mut self, op: impl NonConstOp<'tcx>) {
self.check_op_spanned(op, self.span);
}

/// Emits an error at the given `span` if an expression cannot be evaluated in the current
/// context.
pub fn check_op_spanned<O: NonConstOp>(&mut self, op: O, span: Span) {
pub fn check_op_spanned<O: NonConstOp<'tcx>>(&mut self, op: O, span: Span) {
let gate = match op.status_in_item(self.ccx) {
Status::Allowed => return,

Expand Down Expand Up @@ -773,7 +773,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
self.super_terminator(terminator, location);

match &terminator.kind {
TerminatorKind::Call { func, args, .. } => {
TerminatorKind::Call { func, args, fn_span, from_hir_call, .. } => {
let ConstCx { tcx, body, param_env, .. } = *self.ccx;
let caller = self.def_id().to_def_id();

Expand All @@ -797,7 +797,13 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
if let Some(trait_id) = tcx.trait_of_item(callee) {
trace!("attempting to call a trait method");
if !self.tcx.features().const_trait_impl {
self.check_op(ops::FnCallNonConst(callee, substs));
self.check_op(ops::FnCallNonConst {
caller,
callee,
substs,
span: *fn_span,
from_hir_call: *from_hir_call,
});
return;
}

Expand Down Expand Up @@ -856,7 +862,13 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
}

if !nonconst_call_permission {
self.check_op(ops::FnCallNonConst(callee, substs));
self.check_op(ops::FnCallNonConst {
caller,
callee,
substs,
span: *fn_span,
from_hir_call: *from_hir_call,
});
return;
}
}
Expand Down Expand Up @@ -925,7 +937,13 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
}

if !nonconst_call_permission {
self.check_op(ops::FnCallNonConst(callee, substs));
self.check_op(ops::FnCallNonConst {
caller,
callee,
substs,
span: *fn_span,
from_hir_call: *from_hir_call,
});
return;
}
}
Expand Down

0 comments on commit f7f0f84

Please sign in to comment.