Skip to content

Commit

Permalink
Rollup merge of #132243 - compiler-errors:no-span, r=jieyouxu
Browse files Browse the repository at this point in the history
Remove `ObligationCause::span()` method

I think it's an incredibly confusing footgun to expose both `obligation_cause.span` and `obligation_cause.span()`. Especially because `ObligationCause::span()` (the method) seems to just be hacking around a single quirk in the way we set up obligation causes for match arms.

First commit removes the need for that hack, with only one diagnostic span changing (but IMO not really getting worse -- I'd argue that it was already confusing).
  • Loading branch information
jieyouxu authored Oct 28, 2024
2 parents f14637b + 7f54b9e commit 3e3feac
Show file tree
Hide file tree
Showing 12 changed files with 38 additions and 46 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(
Err(terr) => {
let mut diag = struct_span_code_err!(
tcx.dcx(),
cause.span(),
cause.span,
E0053,
"method `{}` has an incompatible return type for trait",
trait_m.name
Expand Down Expand Up @@ -1169,7 +1169,7 @@ fn extract_spans_for_error_reporting<'tcx>(
TypeError::ArgumentMutability(i) | TypeError::ArgumentSorts(ExpectedFound { .. }, i) => {
(impl_args.nth(i).unwrap(), trait_args.and_then(|mut args| args.nth(i)))
}
_ => (cause.span(), tcx.hir().span_if_local(trait_m.def_id)),
_ => (cause.span, tcx.hir().span_if_local(trait_m.def_id)),
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,7 @@ pub fn check_function_signature<'tcx>(
match err {
TypeError::ArgumentMutability(i)
| TypeError::ArgumentSorts(ExpectedFound { .. }, i) => args.nth(i).unwrap(),
_ => cause.span(),
_ => cause.span,
}
}

Expand Down
16 changes: 7 additions & 9 deletions compiler/rustc_hir_typeck/src/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
(None, arm.body.span)
};

let (span, code) = match prior_arm {
let code = match prior_arm {
// The reason for the first arm to fail is not that the match arms diverge,
// but rather that there's a prior obligation that doesn't hold.
None => {
(arm_span, ObligationCauseCode::BlockTailExpression(arm.body.hir_id, match_src))
}
Some((prior_arm_block_id, prior_arm_ty, prior_arm_span)) => (
expr.span,
None => ObligationCauseCode::BlockTailExpression(arm.body.hir_id, match_src),
Some((prior_arm_block_id, prior_arm_ty, prior_arm_span)) => {
ObligationCauseCode::MatchExpressionArm(Box::new(MatchExpressionArmCause {
arm_block_id,
arm_span,
Expand All @@ -110,13 +107,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
prior_arm_ty,
prior_arm_span,
scrut_span: scrut.span,
expr_span: expr.span,
source: match_src,
prior_non_diverging_arms: prior_non_diverging_arms.clone(),
tail_defines_return_position_impl_trait,
})),
),
}))
}
};
let cause = self.cause(span, code);
let cause = self.cause(arm_span, code);

// This is the moral equivalent of `coercion.coerce(self, cause, arm.body, arm_ty)`.
// We use it this way to be able to expand on the potential error and detect when a
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2027,7 +2027,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
Err(_) => {
span_bug!(
cause.span(),
cause.span,
"subtyping remaining fields of type changing FRU failed: {target_ty} != {fru_ty}: {}::{}",
variant.name,
ident.name,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/method/confirm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
// This has/will have errored in wfcheck, which we cannot depend on from here, as typeck on functions
// may run before wfcheck if the function is used in const eval.
self.dcx().span_delayed_bug(
cause.span(),
cause.span,
format!("{self_ty} was a subtype of {method_self_ty} but now is not?"),
);
}
Expand Down
19 changes: 8 additions & 11 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,16 +92,6 @@ impl<'tcx> ObligationCause<'tcx> {
ObligationCause { span, body_id: CRATE_DEF_ID, code: Default::default() }
}

pub fn span(&self) -> Span {
match *self.code() {
ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause {
arm_span,
..
}) => arm_span,
_ => self.span,
}
}

#[inline]
pub fn code(&self) -> &ObligationCauseCode<'tcx> {
&self.code
Expand Down Expand Up @@ -517,10 +507,17 @@ pub struct MatchExpressionArmCause<'tcx> {
pub prior_arm_block_id: Option<HirId>,
pub prior_arm_ty: Ty<'tcx>,
pub prior_arm_span: Span,
/// Span of the scrutinee of the match (the matched value).
pub scrut_span: Span,
/// Source of the match, i.e. `match` or a desugaring.
pub source: hir::MatchSource,
/// Span of the *whole* match expr.
pub expr_span: Span,
/// Spans of the previous arms except for those that diverge (i.e. evaluate to `!`).
///
/// These are used for pointing out errors that may affect several arms.
pub prior_non_diverging_arms: Vec<Span>,
// Is the expectation of this match expression an RPIT?
/// Is the expectation of this match expression an RPIT?
pub tail_defines_return_position_impl_trait: Option<LocalDefId>,
}

Expand Down
17 changes: 9 additions & 8 deletions compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
Some(ty) if expected == ty => {
let source_map = self.tcx.sess.source_map();
err.span_suggestion(
source_map.end_point(cause.span()),
source_map.end_point(cause.span),
"try removing this `?`",
"",
Applicability::MachineApplicable,
Expand All @@ -412,6 +412,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
source,
ref prior_non_diverging_arms,
scrut_span,
expr_span,
..
}) => match source {
hir::MatchSource::TryDesugar(scrut_hir_id) => {
Expand All @@ -430,7 +431,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
Some(ty) if expected == ty => {
let source_map = self.tcx.sess.source_map();
err.span_suggestion(
source_map.end_point(cause.span()),
source_map.end_point(cause.span),
"try removing this `?`",
"",
Applicability::MachineApplicable,
Expand Down Expand Up @@ -460,12 +461,12 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
format!("this and all prior arms are found to be of type `{t}`"),
);
}
let outer = if any_multiline_arm || !source_map.is_multiline(cause.span) {
let outer = if any_multiline_arm || !source_map.is_multiline(expr_span) {
// Cover just `match` and the scrutinee expression, not
// the entire match body, to reduce diagram noise.
cause.span.shrink_to_lo().to(scrut_span)
expr_span.shrink_to_lo().to(scrut_span)
} else {
cause.span
expr_span
};
let msg = "`match` arms have incompatible types";
err.span_label(outer, msg);
Expand Down Expand Up @@ -1148,7 +1149,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
terr: TypeError<'tcx>,
prefer_label: bool,
) {
let span = cause.span();
let span = cause.span;

// For some types of errors, expected-found does not make
// sense, so just ignore the values we were given.
Expand Down Expand Up @@ -1642,7 +1643,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
terr: TypeError<'tcx>,
) -> Vec<TypeErrorAdditionalDiags> {
let mut suggestions = Vec::new();
let span = trace.cause.span();
let span = trace.cause.span;
let values = self.resolve_vars_if_possible(trace.values);
if let Some((expected, found)) = values.ty() {
match (expected.kind(), found.kind()) {
Expand Down Expand Up @@ -1792,7 +1793,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
) -> Diag<'a> {
debug!("report_and_explain_type_error(trace={:?}, terr={:?})", trace, terr);

let span = trace.cause.span();
let span = trace.cause.span;
let failure_code = trace.cause.as_failure_code_diag(
terr,
span,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ impl<'tcx> NiceRegionError<'_, 'tcx> {
expected_args: GenericArgsRef<'tcx>,
actual_args: GenericArgsRef<'tcx>,
) -> Diag<'tcx> {
let span = cause.span();
let span = cause.span;

let (leading_ellipsis, satisfy_span, where_span, dup_span, def_id) =
if let ObligationCauseCode::WhereClause(def_id, span)
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1180,7 +1180,7 @@ fn assemble_candidates_from_impls<'cx, 'tcx>(
selcx.tcx(),
selcx.tcx().require_lang_item(
LangItem::Sized,
Some(obligation.cause.span()),
Some(obligation.cause.span),
),
[self_ty],
),
Expand Down Expand Up @@ -1600,7 +1600,7 @@ fn confirm_builtin_candidate<'cx, 'tcx>(
// exist. Instead, `Pointee<Metadata = ()>` should be a supertrait of `Sized`.
let sized_predicate = ty::TraitRef::new(
tcx,
tcx.require_lang_item(LangItem::Sized, Some(obligation.cause.span())),
tcx.require_lang_item(LangItem::Sized, Some(obligation.cause.span)),
[self_ty],
);
obligations.push(obligation.with(tcx, sized_predicate));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl<'tcx> InferCtxt<'tcx> {
assert!(!self.intercrate);
let c_pred =
self.canonicalize_query(param_env.and(obligation.predicate), &mut _orig_values);
self.tcx.at(obligation.cause.span()).evaluate_obligation(c_pred)
self.tcx.at(obligation.cause.span).evaluate_obligation(c_pred)
}
}

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/wf/wf-dyn-incompat-trait-obj-match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ fn main() {
Some(()) => &S,
None => &R, //~ ERROR E0308
}
let t: &dyn Trait = match opt() { //~ ERROR E0038
let t: &dyn Trait = match opt() {
Some(()) => &S, //~ ERROR E0038
None => &R,
None => &R, //~ ERROR E0038
};
}
10 changes: 3 additions & 7 deletions tests/ui/wf/wf-dyn-incompat-trait-obj-match.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,10 @@ LL | trait Trait: Sized {}
= note: required for the cast from `&S` to `&dyn Trait`

error[E0038]: the trait `Trait` cannot be made into an object
--> $DIR/wf-dyn-incompat-trait-obj-match.rs:25:25
--> $DIR/wf-dyn-incompat-trait-obj-match.rs:27:17
|
LL | let t: &dyn Trait = match opt() {
| _________________________^
LL | | Some(()) => &S,
LL | | None => &R,
LL | | };
| |_____^ `Trait` cannot be made into an object
LL | None => &R,
| ^^ `Trait` cannot be made into an object
|
note: for a trait to be "dyn-compatible" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
--> $DIR/wf-dyn-incompat-trait-obj-match.rs:6:14
Expand Down

0 comments on commit 3e3feac

Please sign in to comment.