Skip to content

Commit

Permalink
Rollup merge of #99353 - compiler-errors:gat-where-clause-mismatch, r…
Browse files Browse the repository at this point in the history
…=cjgillot

Slightly improve mismatched GAT where clause error

This makes the error reporting a bit more standardized between `where` on GATs and functions.

cc #99206 (`@BoxyUwU),` don't want to mark this as as "fixed" because they're still not perfect, but this is still an improvement IMO so I want to land it incrementally.

regarding "consider adding where clause to trait definition", we don't actually do that for methods as far as i can tell? i could file an issue to look into that maybe.
  • Loading branch information
JohnTitor authored Jul 26, 2022
2 parents d3acd00 + 3bbe95c commit 2944454
Show file tree
Hide file tree
Showing 22 changed files with 126 additions and 208 deletions.
26 changes: 20 additions & 6 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1883,7 +1883,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
exp_span, exp_found.expected, exp_found.found,
);

if let ObligationCauseCode::CompareImplMethodObligation { .. } = cause.code() {
if let ObligationCauseCode::CompareImplItemObligation { .. } = cause.code() {
return;
}

Expand Down Expand Up @@ -2351,7 +2351,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
GenericKind::Projection(ref p) => format!("the associated type `{}`", p),
};

if let Some(SubregionOrigin::CompareImplMethodObligation {
if let Some(SubregionOrigin::CompareImplItemObligation {
span,
impl_item_def_id,
trait_item_def_id,
Expand Down Expand Up @@ -2788,8 +2788,15 @@ impl<'tcx> ObligationCauseExt<'tcx> for ObligationCause<'tcx> {
use self::FailureCode::*;
use crate::traits::ObligationCauseCode::*;
match self.code() {
CompareImplMethodObligation { .. } => Error0308("method not compatible with trait"),
CompareImplTypeObligation { .. } => Error0308("type not compatible with trait"),
CompareImplItemObligation { kind: ty::AssocKind::Fn, .. } => {
Error0308("method not compatible with trait")
}
CompareImplItemObligation { kind: ty::AssocKind::Type, .. } => {
Error0308("type not compatible with trait")
}
CompareImplItemObligation { kind: ty::AssocKind::Const, .. } => {
Error0308("const not compatible with trait")
}
MatchExpressionArm(box MatchExpressionArmCause { source, .. }) => {
Error0308(match source {
hir::MatchSource::TryDesugar => "`?` operator has incompatible types",
Expand Down Expand Up @@ -2823,8 +2830,15 @@ impl<'tcx> ObligationCauseExt<'tcx> for ObligationCause<'tcx> {
fn as_requirement_str(&self) -> &'static str {
use crate::traits::ObligationCauseCode::*;
match self.code() {
CompareImplMethodObligation { .. } => "method type is compatible with trait",
CompareImplTypeObligation { .. } => "associated type is compatible with trait",
CompareImplItemObligation { kind: ty::AssocKind::Fn, .. } => {
"method type is compatible with trait"
}
CompareImplItemObligation { kind: ty::AssocKind::Type, .. } => {
"associated type is compatible with trait"
}
CompareImplItemObligation { kind: ty::AssocKind::Const, .. } => {
"const is compatible with trait"
}
ExprAssignable => "expression is assignable",
IfExpression { .. } => "`if` and `else` have incompatible types",
IfExpressionWithNoElse => "`if` missing an `else` returns `()`",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@
use crate::infer::error_reporting::nice_region_error::NiceRegionError;
use crate::infer::lexical_region_resolve::RegionResolutionError;
use crate::infer::{SubregionOrigin, Subtype};
use crate::traits::ObligationCauseCode::CompareImplMethodObligation;
use crate::infer::Subtype;
use crate::traits::ObligationCauseCode::CompareImplItemObligation;
use rustc_errors::{ErrorGuaranteed, MultiSpan};
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::Visitor;
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::print::RegionHighlightMode;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitor};
use rustc_span::{Span, Symbol};
use rustc_span::Span;

use std::ops::ControlFlow;

Expand All @@ -22,38 +22,22 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
let error = self.error.as_ref()?;
debug!("try_report_impl_not_conforming_to_trait {:?}", error);
if let RegionResolutionError::SubSupConflict(
_, var_origin, sub_origin, _sub, sup_origin, _sup, _,
_,
var_origin,
sub_origin,
_sub,
sup_origin,
_sup,
_,
) = error.clone()
&& let (&Subtype(ref sup_trace), &Subtype(ref sub_trace)) = (&sup_origin, &sub_origin)
&& let (
sub_expected_found @ Some((sub_expected, sub_found)),
sup_expected_found @ Some(_),
CompareImplMethodObligation { trait_item_def_id, .. },
) = (sub_trace.values.ty(), sup_trace.values.ty(), sub_trace.cause.code())
&& let (Subtype(sup_trace), Subtype(sub_trace)) = (&sup_origin, &sub_origin)
&& let sub_expected_found @ Some((sub_expected, sub_found)) = sub_trace.values.ty()
&& let sup_expected_found @ Some(_) = sup_trace.values.ty()
&& let CompareImplItemObligation { trait_item_def_id, .. } = sub_trace.cause.code()
&& sup_expected_found == sub_expected_found
{
let guar = self.emit_err(
var_origin.span(),
sub_expected,
sub_found,
*trait_item_def_id,
);
return Some(guar);
}
if let RegionResolutionError::ConcreteFailure(origin, _, _)
| RegionResolutionError::GenericBoundFailure(origin, _, _) = error.clone()
&& let SubregionOrigin::CompareImplTypeObligation {
span,
impl_item_def_id,
trait_item_def_id,
} = origin
{
let guar = self.emit_associated_type_err(
span,
self.infcx.tcx.item_name(impl_item_def_id.to_def_id()),
impl_item_def_id,
trait_item_def_id,
);
let guar =
self.emit_err(var_origin.span(), sub_expected, sub_found, *trait_item_def_id);
return Some(guar);
}
None
Expand Down Expand Up @@ -147,25 +131,6 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
}
err.emit()
}

fn emit_associated_type_err(
&self,
span: Span,
item_name: Symbol,
impl_item_def_id: LocalDefId,
trait_item_def_id: DefId,
) -> ErrorGuaranteed {
let impl_sp = self.tcx().def_span(impl_item_def_id);
let trait_sp = self.tcx().def_span(trait_item_def_id);
let mut err = self
.tcx()
.sess
.struct_span_err(span, &format!("`impl` associated type signature for `{}` doesn't match `trait` associated type signature", item_name));
err.span_label(impl_sp, "found");
err.span_label(trait_sp, "expected");

err.emit()
}
}

struct TypeParamSpanVisitor<'tcx> {
Expand Down
18 changes: 2 additions & 16 deletions compiler/rustc_infer/src/infer/error_reporting/note.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
"...so that the declared lifetime parameter bounds are satisfied",
);
}
infer::CompareImplMethodObligation { span, .. } => {
label_or_note(
span,
"...so that the definition in impl matches the definition from the trait",
);
}
infer::CompareImplTypeObligation { span, .. } => {
infer::CompareImplItemObligation { span, .. } => {
label_or_note(
span,
"...so that the definition in impl matches the definition from the trait",
Expand Down Expand Up @@ -329,15 +323,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
);
err
}
infer::CompareImplMethodObligation { span, impl_item_def_id, trait_item_def_id } => {
self.report_extra_impl_obligation(
span,
impl_item_def_id,
trait_item_def_id,
&format!("`{}: {}`", sup, sub),
)
}
infer::CompareImplTypeObligation { span, impl_item_def_id, trait_item_def_id } => self
infer::CompareImplItemObligation { span, impl_item_def_id, trait_item_def_id } => self
.report_extra_impl_obligation(
span,
impl_item_def_id,
Expand Down
27 changes: 5 additions & 22 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,15 +405,7 @@ pub enum SubregionOrigin<'tcx> {

/// Comparing the signature and requirements of an impl method against
/// the containing trait.
CompareImplMethodObligation {
span: Span,
impl_item_def_id: LocalDefId,
trait_item_def_id: DefId,
},

/// Comparing the signature and requirements of an impl associated type
/// against the containing trait
CompareImplTypeObligation { span: Span, impl_item_def_id: LocalDefId, trait_item_def_id: DefId },
CompareImplItemObligation { span: Span, impl_item_def_id: LocalDefId, trait_item_def_id: DefId },

/// Checking that the bounds of a trait's associated type hold for a given impl
CheckAssociatedTypeBounds {
Expand Down Expand Up @@ -1945,8 +1937,7 @@ impl<'tcx> SubregionOrigin<'tcx> {
ReborrowUpvar(a, _) => a,
DataBorrowed(_, a) => a,
ReferenceOutlivesReferent(_, a) => a,
CompareImplMethodObligation { span, .. } => span,
CompareImplTypeObligation { span, .. } => span,
CompareImplItemObligation { span, .. } => span,
CheckAssociatedTypeBounds { ref parent, .. } => parent.span(),
}
}
Expand All @@ -1960,19 +1951,11 @@ impl<'tcx> SubregionOrigin<'tcx> {
SubregionOrigin::ReferenceOutlivesReferent(ref_type, cause.span)
}

traits::ObligationCauseCode::CompareImplMethodObligation {
impl_item_def_id,
trait_item_def_id,
} => SubregionOrigin::CompareImplMethodObligation {
span: cause.span,
impl_item_def_id,
trait_item_def_id,
},

traits::ObligationCauseCode::CompareImplTypeObligation {
traits::ObligationCauseCode::CompareImplItemObligation {
impl_item_def_id,
trait_item_def_id,
} => SubregionOrigin::CompareImplTypeObligation {
kind: _,
} => SubregionOrigin::CompareImplItemObligation {
span: cause.span,
impl_item_def_id,
trait_item_def_id,
Expand Down
12 changes: 2 additions & 10 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,18 +311,10 @@ pub enum ObligationCauseCode<'tcx> {
},

/// Error derived when matching traits/impls; see ObligationCause for more details
CompareImplConstObligation,

/// Error derived when matching traits/impls; see ObligationCause for more details
CompareImplMethodObligation {
impl_item_def_id: LocalDefId,
trait_item_def_id: DefId,
},

/// Error derived when matching traits/impls; see ObligationCause for more details
CompareImplTypeObligation {
CompareImplItemObligation {
impl_item_def_id: LocalDefId,
trait_item_def_id: DefId,
kind: ty::AssocKind,
},

/// Checking that the bounds of a trait's associated type hold for a given impl
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_middle/src/ty/assoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,16 @@ impl AssocKind {
}
}

impl std::fmt::Display for AssocKind {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
AssocKind::Fn => write!(f, "method"),
AssocKind::Const => write!(f, "associated const"),
AssocKind::Type => write!(f, "associated type"),
}
}
}

/// A list of `ty::AssocItem`s in definition order that allows for efficient lookup by name.
///
/// When doing lookup by name, we try to postpone hygienic comparison for as long as possible since
Expand Down
8 changes: 2 additions & 6 deletions compiler/rustc_middle/src/ty/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -660,12 +660,8 @@ impl<T> Trait<T> for X {
| hir::Node::ImplItem(hir::ImplItem { kind: hir::ImplItemKind::Fn(..), .. }),
)
);
let impl_comparison = matches!(
cause_code,
ObligationCauseCode::CompareImplMethodObligation { .. }
| ObligationCauseCode::CompareImplTypeObligation { .. }
| ObligationCauseCode::CompareImplConstObligation
);
let impl_comparison =
matches!(cause_code, ObligationCauseCode::CompareImplItemObligation { .. });
let assoc = self.associated_item(proj_ty.item_def_id);
if !callable_scope || impl_comparison {
// We do not want to suggest calling functions when the reason of the
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ TrivialTypeTraversalAndLiftImpls! {
// general `Region`.
crate::ty::BoundRegionKind,
crate::ty::AssocItem,
crate::ty::AssocKind,
crate::ty::Placeholder<crate::ty::BoundRegionKind>,
crate::ty::ClosureKind,
crate::ty::FreeRegion,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,13 +301,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
span = obligation.cause.span;
}
}
if let ObligationCauseCode::CompareImplMethodObligation {
impl_item_def_id,
trait_item_def_id,
}
| ObligationCauseCode::CompareImplTypeObligation {
if let ObligationCauseCode::CompareImplItemObligation {
impl_item_def_id,
trait_item_def_id,
kind: _,
} = *obligation.cause.code()
{
self.report_extra_impl_obligation(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2682,11 +2682,11 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
)
});
}
ObligationCauseCode::CompareImplMethodObligation { trait_item_def_id, .. } => {
ObligationCauseCode::CompareImplItemObligation { trait_item_def_id, kind, .. } => {
let item_name = self.tcx.item_name(trait_item_def_id);
let msg = format!(
"the requirement `{}` appears on the impl method `{}` but not on the \
corresponding trait method",
"the requirement `{}` appears on the `impl`'s {kind} `{}` but not on the \
corresponding trait's {kind}",
predicate, item_name,
);
let sp = self
Expand All @@ -2697,7 +2697,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
let mut assoc_span: MultiSpan = sp.into();
assoc_span.push_span_label(
sp,
format!("this trait method doesn't have the requirement `{}`", predicate),
format!("this trait's {kind} doesn't have the requirement `{}`", predicate),
);
if let Some(ident) = self
.tcx
Expand All @@ -2708,38 +2708,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}
err.span_note(assoc_span, &msg);
}
ObligationCauseCode::CompareImplTypeObligation { trait_item_def_id, .. } => {
let item_name = self.tcx.item_name(trait_item_def_id);
let msg = format!(
"the requirement `{}` appears on the associated impl type `{}` but not on the \
corresponding associated trait type",
predicate, item_name,
);
let sp = self.tcx.def_span(trait_item_def_id);
let mut assoc_span: MultiSpan = sp.into();
assoc_span.push_span_label(
sp,
format!(
"this trait associated type doesn't have the requirement `{}`",
predicate,
),
);
if let Some(ident) = self
.tcx
.opt_associated_item(trait_item_def_id)
.and_then(|i| self.tcx.opt_item_ident(i.container.id()))
{
assoc_span.push_span_label(ident.span, "in this trait");
}
err.span_note(assoc_span, &msg);
}
ObligationCauseCode::CompareImplConstObligation => {
err.note(&format!(
"the requirement `{}` appears on the associated impl constant \
but not on the corresponding associated trait constant",
predicate
));
}
ObligationCauseCode::TrivialBound => {
err.help("see issue #48214");
if tcx.sess.opts.unstable_features.is_nightly_build() {
Expand Down
9 changes: 2 additions & 7 deletions compiler/rustc_typeck/src/astconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1150,17 +1150,12 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
.expect("missing associated type");

if !assoc_item.vis.is_accessible_from(def_scope, tcx) {
let kind = match assoc_item.kind {
ty::AssocKind::Type => "type",
ty::AssocKind::Const => "const",
_ => unreachable!(),
};
tcx.sess
.struct_span_err(
binding.span,
&format!("associated {kind} `{}` is private", binding.item_name),
&format!("{} `{}` is private", assoc_item.kind, binding.item_name),
)
.span_label(binding.span, &format!("private associated {kind}"))
.span_label(binding.span, &format!("private {}", assoc_item.kind))
.emit();
}
tcx.check_stability(assoc_item.def_id, Some(hir_ref_id), binding.span, None);
Expand Down
Loading

0 comments on commit 2944454

Please sign in to comment.