Move check_mod_impl_wf query call out of track_errors and bubble er…

…rors up instead.

compiler/rustc_hir_analysis/src/impl_wf_check.rs

@@ -17,7 +17,7 @@ use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{LocalDefId, LocalModDefId};
 use rustc_middle::query::Providers;
 use rustc_middle::ty::{self, TyCtxt, TypeVisitableExt};
-use rustc_span::{Span, Symbol};
+use rustc_span::{ErrorGuaranteed, Span, Symbol};
 
 mod min_specialization;
 
@@ -51,24 +51,29 @@ mod min_specialization;
 /// impl<'a> Trait<Foo> for Bar { type X = &'a i32; }
 /// // ^ 'a is unused and appears in assoc type, error
 /// ```
-fn check_mod_impl_wf(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) {
+fn check_mod_impl_wf(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) -> Result<(), ErrorGuaranteed> {
  let min_specialization = tcx.features().min_specialization;
  let module = tcx.hir_module_items(module_def_id);
+ let mut res = Ok(());
  for id in module.items() {
  if matches!(tcx.def_kind(id.owner_id), DefKind::Impl { .. }) {
- enforce_impl_params_are_constrained(tcx, id.owner_id.def_id);
+ res = res.and(enforce_impl_params_are_constrained(tcx, id.owner_id.def_id));
  if min_specialization {
- check_min_specialization(tcx, id.owner_id.def_id);
+ res = res.and(check_min_specialization(tcx, id.owner_id.def_id));
  }
  }
  }
+ res
 }
 
 pub fn provide(providers: &mut Providers) {
  *providers = Providers { check_mod_impl_wf, ..*providers };
 }
 
-fn enforce_impl_params_are_constrained(tcx: TyCtxt<'_>, impl_def_id: LocalDefId) {
+fn enforce_impl_params_are_constrained(
+ tcx: TyCtxt<'_>,
+ impl_def_id: LocalDefId,
+) -> Result<(), ErrorGuaranteed> {
  // Every lifetime used in an associated type must be constrained.
  let impl_self_ty = tcx.type_of(impl_def_id).instantiate_identity();
  if impl_self_ty.references_error() {
@@ -80,7 +85,10 @@ fn enforce_impl_params_are_constrained(tcx: TyCtxt<'_>, impl_def_id: LocalDefId)
  "potentially unconstrained type parameters weren't evaluated: {impl_self_ty:?}",
  ),
  );
- return;
+ // This is super fishy, but our current `rustc_hir_analysis::check_crate` pipeline depends on
+ // `type_of` having been called much earlier, and thus this value being read from cache.
+ // Compilation must continue in order for other important diagnostics to keep showing up.
+ return Ok(());
  }
  let impl_generics = tcx.generics_of(impl_def_id);
  let impl_predicates = tcx.predicates_of(impl_def_id);
@@ -113,41 +121,48 @@ fn enforce_impl_params_are_constrained(tcx: TyCtxt<'_>, impl_def_id: LocalDefId)
  })
  .collect();
 
+ let mut res = Ok(());
  for param in &impl_generics.params {
  match param.kind {
  // Disallow ANY unconstrained type parameters.
  ty::GenericParamDefKind::Type { .. } => {
  let param_ty = ty::ParamTy::for_def(param);
  if !input_parameters.contains(&cgp::Parameter::from(param_ty)) {
- report_unused_parameter(tcx, tcx.def_span(param.def_id), "type", param_ty.name);
+ res = Err(report_unused_parameter(
+ tcx,
+ tcx.def_span(param.def_id),
+ "type",
+ param_ty.name,
+ ));
  }
  }
  ty::GenericParamDefKind::Lifetime => {
  let param_lt = cgp::Parameter::from(param.to_early_bound_region_data());
  if lifetimes_in_associated_types.contains(&param_lt) && // (*)
  !input_parameters.contains(&param_lt)
  {
- report_unused_parameter(
+ res = Err(report_unused_parameter(
  tcx,
  tcx.def_span(param.def_id),
  "lifetime",
  param.name,
- );
+ ));
  }
  }
  ty::GenericParamDefKind::Const { .. } => {
  let param_ct = ty::ParamConst::for_def(param);
  if !input_parameters.contains(&cgp::Parameter::from(param_ct)) {
- report_unused_parameter(
+ res = Err(report_unused_parameter(
  tcx,
  tcx.def_span(param.def_id),
  "const",
  param_ct.name,
- );
+ ));
  }
  }
  }
  }
+ res
 
  // (*) This is a horrible concession to reality. I think it'd be
  // better to just ban unconstrained lifetimes outright, but in
@@ -169,7 +184,12 @@ fn enforce_impl_params_are_constrained(tcx: TyCtxt<'_>, impl_def_id: LocalDefId)
  // used elsewhere are not projected back out.
 }
 
-fn report_unused_parameter(tcx: TyCtxt<'_>, span: Span, kind: &str, name: Symbol) {
+fn report_unused_parameter(
+ tcx: TyCtxt<'_>,
+ span: Span,
+ kind: &str,
+ name: Symbol,
+) -> ErrorGuaranteed {
  let mut err = struct_span_code_err!(
  tcx.dcx(),
  span,
@@ -188,5 +208,5 @@ fn report_unused_parameter(tcx: TyCtxt<'_>, span: Span, kind: &str, name: Symbol
  "proving the result of expressions other than the parameter are unique is not supported",
  );
  }
- err.emit();
+ err.emit()
 }

compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs

@@ -82,10 +82,14 @@ use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt;
 use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt as _;
 use rustc_trait_selection::traits::{self, translate_args_with_cause, wf, ObligationCtxt};
 
-pub(super) fn check_min_specialization(tcx: TyCtxt<'_>, impl_def_id: LocalDefId) {
+pub(super) fn check_min_specialization(
+ tcx: TyCtxt<'_>,
+ impl_def_id: LocalDefId,
+) -> Result<(), ErrorGuaranteed> {
  if let Some(node) = parent_specialization_node(tcx, impl_def_id) {
- check_always_applicable(tcx, impl_def_id, node);
+ check_always_applicable(tcx, impl_def_id, node)?;
  }
+ Ok(())
 }
 
 fn parent_specialization_node(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId) -> Option<Node> {
@@ -109,52 +113,69 @@ fn parent_specialization_node(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId) -> Opti
 
 /// Check that `impl1` is a sound specialization
 #[instrument(level = "debug", skip(tcx))]
-fn check_always_applicable(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId, impl2_node: Node) {
+fn check_always_applicable(
+ tcx: TyCtxt<'_>,
+ impl1_def_id: LocalDefId,
+ impl2_node: Node,
+) -> Result<(), ErrorGuaranteed> {
  let span = tcx.def_span(impl1_def_id);
- check_has_items(tcx, impl1_def_id, impl2_node, span);
-
- if let Ok((impl1_args, impl2_args)) = get_impl_args(tcx, impl1_def_id, impl2_node) {
- let impl2_def_id = impl2_node.def_id();
- debug!(?impl2_def_id, ?impl2_args);
-
- let parent_args = if impl2_node.is_from_trait() {
- impl2_args.to_vec()
- } else {
- unconstrained_parent_impl_args(tcx, impl2_def_id, impl2_args)
- };
-
- check_constness(tcx, impl1_def_id, impl2_node, span);
- check_static_lifetimes(tcx, &parent_args, span);
- check_duplicate_params(tcx, impl1_args, &parent_args, span);
- check_predicates(tcx, impl1_def_id, impl1_args, impl2_node, impl2_args, span);
- }
+ let mut res = check_has_items(tcx, impl1_def_id, impl2_node, span);
+
+ let (impl1_args, impl2_args) = get_impl_args(tcx, impl1_def_id, impl2_node)?;
+ let impl2_def_id = impl2_node.def_id();
+ debug!(?impl2_def_id, ?impl2_args);
+
+ let parent_args = if impl2_node.is_from_trait() {
+ impl2_args.to_vec()
+ } else {
+ unconstrained_parent_impl_args(tcx, impl2_def_id, impl2_args)
+ };
+
+ res = res.and(check_constness(tcx, impl1_def_id, impl2_node, span));
+ res = res.and(check_static_lifetimes(tcx, &parent_args, span));
+ res = res.and(check_duplicate_params(tcx, impl1_args, &parent_args, span));
+ res = res.and(check_predicates(tcx, impl1_def_id, impl1_args, impl2_node, impl2_args, span));
+
+ res
 }
 
-fn check_has_items(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId, impl2_node: Node, span: Span) {
+fn check_has_items(
+ tcx: TyCtxt<'_>,
+ impl1_def_id: LocalDefId,
+ impl2_node: Node,
+ span: Span,
+) -> Result<(), ErrorGuaranteed> {
  if let Node::Impl(impl2_id) = impl2_node
  && tcx.associated_item_def_ids(impl1_def_id).is_empty()
  {
  let base_impl_span = tcx.def_span(impl2_id);
- tcx.dcx().emit_err(errors::EmptySpecialization { span, base_impl_span });
+ return Err(tcx.dcx().emit_err(errors::EmptySpecialization { span, base_impl_span }));
  }
+ Ok(())
 }
 
 /// Check that the specializing impl `impl1` is at least as const as the base
 /// impl `impl2`
-fn check_constness(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId, impl2_node: Node, span: Span) {
+fn check_constness(
+ tcx: TyCtxt<'_>,
+ impl1_def_id: LocalDefId,
+ impl2_node: Node,
+ span: Span,
+) -> Result<(), ErrorGuaranteed> {
  if impl2_node.is_from_trait() {
  // This isn't a specialization
- return;
+ return Ok(());
  }
 
  let impl1_constness = tcx.constness(impl1_def_id.to_def_id());
  let impl2_constness = tcx.constness(impl2_node.def_id());
 
  if let hir::Constness::Const = impl2_constness {
  if let hir::Constness::NotConst = impl1_constness {
- tcx.dcx().emit_err(errors::ConstSpecialize { span });
+ return Err(tcx.dcx().emit_err(errors::ConstSpecialize { span }));
  }
  }
+ Ok(())
 }
 
 /// Given a specializing impl `impl1`, and the base impl `impl2`, returns two
@@ -290,15 +311,17 @@ fn check_duplicate_params<'tcx>(
  impl1_args: GenericArgsRef<'tcx>,
  parent_args: &Vec<GenericArg<'tcx>>,
  span: Span,
-) {
+) -> Result<(), ErrorGuaranteed> {
  let mut base_params = cgp::parameters_for(parent_args, true);
  base_params.sort_by_key(|param| param.0);
  if let (_, [duplicate, ..]) = base_params.partition_dedup() {
  let param = impl1_args[duplicate.0 as usize];
- tcx.dcx()
+ return Err(tcx
+ .dcx()
  .struct_span_err(span, format!("specializing impl repeats parameter `{param}`"))
- .emit();
+ .emit());
  }
+ Ok(())
 }
 
 /// Check that `'static` lifetimes are not introduced by the specializing impl.
@@ -313,10 +336,11 @@ fn check_static_lifetimes<'tcx>(
  tcx: TyCtxt<'tcx>,
  parent_args: &Vec<GenericArg<'tcx>>,
  span: Span,
-) {
+) -> Result<(), ErrorGuaranteed> {
  if tcx.any_free_region_meets(parent_args, |r| r.is_static()) {
- tcx.dcx().emit_err(errors::StaticSpecialize { span });
+ return Err(tcx.dcx().emit_err(errors::StaticSpecialize { span }));
  }
+ Ok(())
 }
 
 /// Check whether predicates on the specializing impl (`impl1`) are allowed.
@@ -337,7 +361,7 @@ fn check_predicates<'tcx>(
  impl2_node: Node,
  impl2_args: GenericArgsRef<'tcx>,
  span: Span,
-) {
+) -> Result<(), ErrorGuaranteed> {
  let impl1_predicates: Vec<_> = traits::elaborate(
  tcx,
  tcx.predicates_of(impl1_def_id).instantiate(tcx, impl1_args).into_iter(),
@@ -399,14 +423,16 @@ fn check_predicates<'tcx>(
  }
  impl2_predicates.extend(traits::elaborate(tcx, always_applicable_traits));
 
+ let mut res = Ok(());
  for (clause, span) in impl1_predicates {
  if !impl2_predicates
  .iter()
  .any(|pred2| trait_predicates_eq(tcx, clause.as_predicate(), *pred2, span))
  {
- check_specialization_on(tcx, clause, span)
+ res = res.and(check_specialization_on(tcx, clause, span))
  }
  }
+ res
 }
 
 /// Checks if some predicate on the specializing impl (`predicate1`) is the same
@@ -443,37 +469,43 @@ fn trait_predicates_eq<'tcx>(
 }
 
 #[instrument(level = "debug", skip(tcx))]
-fn check_specialization_on<'tcx>(tcx: TyCtxt<'tcx>, clause: ty::Clause<'tcx>, span: Span) {
+fn check_specialization_on<'tcx>(
+ tcx: TyCtxt<'tcx>,
+ clause: ty::Clause<'tcx>,
+ span: Span,
+) -> Result<(), ErrorGuaranteed> {
  match clause.kind().skip_binder() {
  // Global predicates are either always true or always false, so we
  // are fine to specialize on.
- _ if clause.is_global() => (),
+ _ if clause.is_global() => Ok(()),
  // We allow specializing on explicitly marked traits with no associated
  // items.
  ty::ClauseKind::Trait(ty::TraitPredicate { trait_ref, polarity: _ }) => {
- if !matches!(
+ if matches!(
  trait_specialization_kind(tcx, clause),
  Some(TraitSpecializationKind::Marker)
  ) {
- tcx.dcx()
+ Ok(())
+ } else {
+ Err(tcx
+ .dcx()
  .struct_span_err(
  span,
  format!(
  "cannot specialize on trait `{}`",
  tcx.def_path_str(trait_ref.def_id),
  ),
  )
- .emit();
+ .emit())
  }
  }
- ty::ClauseKind::Projection(ty::ProjectionPredicate { projection_ty, term }) => {
- tcx.dcx()
- .struct_span_err(
- span,
- format!("cannot specialize on associated type `{projection_ty} == {term}`",),
- )
- .emit();
- }
+ ty::ClauseKind::Projection(ty::ProjectionPredicate { projection_ty, term }) => Err(tcx
+ .dcx()
+ .struct_span_err(
+ span,
+ format!("cannot specialize on associated type `{projection_ty} == {term}`",),
+ )
+ .emit()),
  ty::ClauseKind::ConstArgHasType(..) => {
  // FIXME(min_specialization), FIXME(const_generics):
  // It probably isn't right to allow _every_ `ConstArgHasType` but I am somewhat unsure
@@ -483,12 +515,12 @@ fn check_specialization_on<'tcx>(tcx: TyCtxt<'tcx>, clause: ty::Clause<'tcx>, sp
  // While we do not support constructs like `<T, const N: T>` there is probably no risk of
  // soundness bugs, but when we support generic const parameter types this will need to be
  // revisited.
+ Ok(())
  }
- _ => {
- tcx.dcx()
- .struct_span_err(span, format!("cannot specialize on predicate `{clause}`"))
- .emit();
- }
+ _ => Err(tcx
+ .dcx()
+ .struct_span_err(span, format!("cannot specialize on predicate `{clause}`"))
+ .emit()),
  }
 }