Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make Thin a supertrait of Sized #122553

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -589,11 +589,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
mutate_fulfillment_errors: impl Fn(&mut Vec<traits::FulfillmentError<'tcx>>),
) {
let mut result = self.fulfillment_cx.borrow_mut().select_where_possible(self);
if !result.is_empty() {
mutate_fulfillment_errors(&mut result);
self.adjust_fulfillment_errors_for_expr_obligation(&mut result);
self.err_ctxt().report_fulfillment_errors(result);
let mut errors = self.fulfillment_cx.borrow_mut().select_where_possible(self);
if !errors.is_empty() {
mutate_fulfillment_errors(&mut errors);
if errors.is_empty() {
// We sometimes skip reporting fulfillment errors while constructing
// a different error.
self.dcx().span_delayed_bug(
self.tcx.def_span(self.body_id),
"skipped reporting fulfillment errors",
);
return;
}
self.adjust_fulfillment_errors_for_expr_obligation(&mut errors);
self.err_ctxt().report_fulfillment_errors(errors);
}
}

Expand Down
20 changes: 4 additions & 16 deletions compiler/rustc_trait_selection/src/solve/normalizes_to/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,22 +541,6 @@ impl<'tcx> assembly::GoalKind<'tcx> for NormalizesTo<'tcx> {
.instantiate(tcx, &[ty::GenericArg::from(goal.predicate.self_ty())])
}

ty::Alias(_, _) | ty::Param(_) | ty::Placeholder(..) => {
// This is the "fallback impl" for type parameters, unnormalizable projections
// and opaque types: If the `self_ty` is `Sized`, then the metadata is `()`.
// FIXME(ptr_metadata): This impl overlaps with the other impls and shouldn't
// exist. Instead, `Pointee<Metadata = ()>` should be a supertrait of `Sized`.
let sized_predicate = ty::TraitRef::from_lang_item(
tcx,
LangItem::Sized,
DUMMY_SP,
[ty::GenericArg::from(goal.predicate.self_ty())],
);
// FIXME(-Znext-solver=coinductive): Should this be `GoalSource::ImplWhereBound`?
ecx.add_goal(GoalSource::Misc, goal.with(tcx, sized_predicate));
tcx.types.unit
}

ty::Adt(def, args) if def.is_struct() => match def.non_enum_variant().tail_opt() {
None => tcx.types.unit,
Some(tail_def) => {
Expand All @@ -571,6 +555,10 @@ impl<'tcx> assembly::GoalKind<'tcx> for NormalizesTo<'tcx> {
Some(&tail_ty) => Ty::new_projection(tcx, metadata_def_id, [tail_ty]),
},

// The metadata of these types can only be known from param env or alias bound
// candidates.
ty::Alias(_, _) | ty::Param(_) | ty::Placeholder(..) => return Err(NoSolution),

ty::Infer(
ty::TyVar(_) | ty::FreshTy(_) | ty::FreshIntTy(_) | ty::FreshFloatTy(_),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,61 +80,81 @@ pub fn suggest_new_overflow_limit<'tcx, G: EmissionGuarantee>(

#[extension(pub trait TypeErrCtxtExt<'tcx>)]
impl<'tcx> TypeErrCtxt<'_, 'tcx> {
#[instrument(skip(self), level = "debug")]
fn report_fulfillment_errors(
&self,
mut errors: Vec<FulfillmentError<'tcx>>,
) -> ErrorGuaranteed {
if errors.is_empty() {
bug!("attempted to report fulfillment errors, but there we no errors");
}

self.sub_relations
.borrow_mut()
.add_constraints(self, errors.iter().map(|e| e.obligation.predicate));

let mut reported = None;

// We want to ignore desugarings when filtering errors: spans are equivalent even
// if one is the result of a desugaring and the other is not.
let strip_desugaring = |span: Span| {
let expn_data = span.ctxt().outer_expn_data();
if let ExpnKind::Desugaring(_) = expn_data.kind { expn_data.call_site } else { span }
};

#[derive(Debug)]
struct ErrorDescriptor<'tcx> {
struct ErrorDescriptor<'tcx, 'err> {
predicate: ty::Predicate<'tcx>,
index: Option<usize>, // None if this is an old error
source: Option<(usize, &'err FulfillmentError<'tcx>)>, // None if this is an old error
}

let mut error_map: FxIndexMap<_, Vec<_>> = self
.reported_trait_errors
.borrow()
.iter()
.map(|(&span, predicates)| {
(
span,
predicates
.0
.iter()
.map(|&predicate| ErrorDescriptor { predicate, index: None })
.collect(),
)
.map(|(&span, &(ref predicates, guar))| {
reported = Some(guar);
let span = strip_desugaring(span);
let reported_errors = predicates
.iter()
.map(|&predicate| ErrorDescriptor { predicate, source: None })
.collect();
(span, reported_errors)
})
.collect();

#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
enum ErrorOrd {
Default,
Sized,
Metadata,
Coerce,
WellFormed,
}

// Ensure `T: Sized` and `T: WF` obligations come last. This lets us display diagnostics
// with more relevant type information and hide redundant E0282 errors.
// with more relevant type information and hide redundant E0282 ("type annotations needed") errors.
errors.sort_by_key(|e| match e.obligation.predicate.kind().skip_binder() {
ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred))
if Some(pred.def_id()) == self.tcx.lang_items().sized_trait() =>
{
1
ErrorOrd::Sized
}
ty::PredicateKind::Clause(ty::ClauseKind::Projection(pred))
if Some(pred.def_id()) == self.tcx.lang_items().metadata_type() =>
{
ErrorOrd::Metadata
}
ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(_)) => 3,
ty::PredicateKind::Coerce(_) => 2,
_ => 0,
ty::PredicateKind::Coerce(_) => ErrorOrd::Coerce,
ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(_)) => ErrorOrd::WellFormed,
_ => ErrorOrd::Default,
});

for (index, error) in errors.iter().enumerate() {
// We want to ignore desugarings here: spans are equivalent even
// if one is the result of a desugaring and the other is not.
let mut span = error.obligation.cause.span;
let expn_data = span.ctxt().outer_expn_data();
if let ExpnKind::Desugaring(_) = expn_data.kind {
span = expn_data.call_site;
}

let span = strip_desugaring(error.obligation.cause.span);
error_map.entry(span).or_default().push(ErrorDescriptor {
predicate: error.obligation.predicate,
index: Some(index),
source: Some((index, error)),
});
}

Expand All @@ -144,59 +164,63 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
for (_, error_set) in error_map.iter() {
// We want to suppress "duplicate" errors with the same span.
for error in error_set {
if let Some(index) = error.index {
let Some((index, error_source)) = error.source else {
continue;
};

for error2 in error_set {
// Suppress errors that are either:
// 1) strictly implied by another error.
// 2) implied by an error with a smaller index.
for error2 in error_set {
if error2.index.is_some_and(|index2| is_suppressed[index2]) {
// Avoid errors being suppressed by already-suppressed
// errors, to prevent all errors from being suppressed
// at once.
continue;
}
if self.error_implied_by(error.predicate, error2.predicate)
&& (!error2.source.is_some_and(|(index2, _)| index2 >= index)
|| !self.error_implied_by(error2.predicate, error.predicate))
{
info!("skipping `{}` (implied by `{}`)", error.predicate, error2.predicate);
is_suppressed[index] = true;
break;
}

if self.error_implies(error2.predicate, error.predicate)
&& !(error2.index >= error.index
&& self.error_implies(error.predicate, error2.predicate))
{
info!("skipping {:?} (implied by {:?})", error, error2);
is_suppressed[index] = true;
break;
}
// Also suppress the error if we are absolutely certain that a different
// error is the one that the user should fix. This will suppress errors
// about `<T as Pointee>::Metadata == ()` that can be fixed by `T: Sized`.
if error.predicate.to_opt_poly_projection_pred().is_some()
&& error2.predicate.to_opt_poly_trait_pred().is_some()
&& self.error_fixed_by(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we can just use error_fixed_by everywhere -- it seems more general than error_implied_by?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily in this PR, but maybe tracked as a FIXME.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I tried first, but there are multiple problems with this approach:

  • error_fixed_by does not work when there are any infer vars in cond (first arg). This adds a lot of duplicate diagnostics.
  • There are some cases where A fixed-by B + not (B fixed-by A) + B fixed-by C + not (C fixed-by B) + C fixed-by A + not (A fixed-by C). In these cases, replacing error_implied_by with error_fixed_by causes no diagnostics to be emitted. This cannot happen with error_implied_by, because A implied-by B is transitive.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • There are some cases where A fixed-by B + not (B fixed-by A) + B fixed-by C + not (C fixed-by B) + C fixed-by A + not (A fixed-by C).

After thinking about this again, I'm actually not sure anymore whether this can actually happen and can't come up with a concrete example. I had a test failing and thought that was due a cycle like that, but after trying it again it turned out to be an unrelated implementation bug.

error_source.obligation.clone(),
error2.predicate.expect_clause(),
)
{
info!("skipping `{}` (fixed by `{}`)", error.predicate, error2.predicate);
is_suppressed[index] = true;
break;
}
}
}
}

let mut reported = None;

for from_expansion in [false, true] {
for (error, suppressed) in iter::zip(&errors, &is_suppressed) {
if !suppressed && error.obligation.cause.span.from_expansion() == from_expansion {
let guar = self.report_fulfillment_error(error);
reported = Some(guar);
// We want to ignore desugarings here: spans are equivalent even
// if one is the result of a desugaring and the other is not.
let mut span = error.obligation.cause.span;
let expn_data = span.ctxt().outer_expn_data();
if let ExpnKind::Desugaring(_) = expn_data.kind {
span = expn_data.call_site;
}
self.reported_trait_errors
.borrow_mut()
.entry(span)
.or_insert_with(|| (vec![], guar))
.0
.push(error.obligation.predicate);
for (error, &suppressed) in iter::zip(&errors, &is_suppressed) {
let span = error.obligation.cause.span;
if suppressed || span.from_expansion() != from_expansion {
continue;
}

let guar = self.report_fulfillment_error(error);
reported = Some(guar);

self.reported_trait_errors
.borrow_mut()
.entry(span)
.or_insert_with(|| (vec![], guar))
.0
.push(error.obligation.predicate);
}
}

// It could be that we don't report an error because we have seen an `ErrorReported` from
// another source. We should probably be able to fix most of these, but some are delayed
// bugs that get a proper error after this function.
reported.unwrap_or_else(|| self.dcx().delayed_bug("failed to report fulfillment errors"))
// If all errors are suppressed, then we must have reported at least one error
// from a previous call to this function.
reported.unwrap_or_else(|| bug!("failed to report fulfillment errors"))
}

/// Reports that an overflow has occurred and halts compilation. We
Expand Down Expand Up @@ -1465,10 +1489,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
&& self.can_eq(param_env, goal.term, assumption.term)
}

// returns if `cond` not occurring implies that `error` does not occur - i.e., that
// `error` occurring implies that `cond` occurs.
/// Returns whether `cond` not occurring implies that `error` does not occur - i.e., that
/// `error` occurring implies that `cond` occurs.
#[instrument(level = "debug", skip(self), ret)]
fn error_implies(&self, cond: ty::Predicate<'tcx>, error: ty::Predicate<'tcx>) -> bool {
fn error_implied_by(&self, error: ty::Predicate<'tcx>, cond: ty::Predicate<'tcx>) -> bool {
if cond == error {
return true;
}
Expand All @@ -1490,6 +1514,29 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
}
}

/// Returns whether fixing `cond` will also fix `error`.
#[instrument(level = "debug", skip(self), ret)]
fn error_fixed_by(&self, mut error: PredicateObligation<'tcx>, cond: ty::Clause<'tcx>) -> bool {
self.probe(|_| {
let ocx = ObligationCtxt::new(self);

let clauses = elaborate(self.tcx, std::iter::once(cond)).collect::<Vec<_>>();
let clauses = ocx.normalize(&error.cause, error.param_env, clauses);
let mut clauses = self.resolve_vars_if_possible(clauses);

if clauses.has_infer() {
return false;
}

clauses.extend(error.param_env.caller_bounds());
let clauses = self.tcx.mk_clauses(&clauses);
error.param_env = ty::ParamEnv::new(clauses, error.param_env.reveal());

ocx.register_obligation(error);
ocx.select_all_or_error().is_empty()
})
}

#[instrument(skip(self), level = "debug")]
fn report_fulfillment_error(&self, error: &FulfillmentError<'tcx>) -> ErrorGuaranteed {
if self.tcx.sess.opts.unstable_opts.next_solver.map(|c| c.dump_tree).unwrap_or_default()
Expand Down
Loading
Loading