Move `check_mod_impl_wf` query call out of track_errors and bubble errors up instead.

This commit is contained in:
Oli Scherer 2024-01-11 22:13:39 +00:00
parent 4db93c5750
commit b1ce8a4ecd
7 changed files with 160 additions and 72 deletions

View File

@ -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,13 +121,19 @@ 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 => {
@ -127,27 +141,28 @@ fn enforce_impl_params_are_constrained(tcx: TyCtxt<'_>, impl_def_id: LocalDefId)
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()
}

View File

@ -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,42 +113,58 @@ 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);
let mut res = 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 (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)
};
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);
}
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());
@ -152,9 +172,10 @@ fn check_constness(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId, impl2_node: Node,
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,19 +469,26 @@ 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!(
@ -463,17 +496,16 @@ fn check_specialization_on<'tcx>(tcx: TyCtxt<'tcx>, clause: ty::Clause<'tcx>, sp
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()),
}
}

View File

@ -166,17 +166,18 @@ pub fn check_crate(tcx: TyCtxt<'_>) -> Result<(), ErrorGuaranteed> {
tcx.hir().for_each_module(|module| tcx.ensure().collect_mod_item_types(module))
});
// FIXME(matthewjasper) We shouldn't need to use `track_errors` anywhere in this function
// or the compiler in general.
if tcx.features().rustc_attrs {
tcx.sess.time("outlives_testing", || outlives::test::test_inferred_outlives(tcx))?;
}
tcx.sess.track_errors(|| {
tcx.sess.time("coherence_checking", || {
// Check impls constrain their parameters
tcx.hir().for_each_module(|module| tcx.ensure().check_mod_impl_wf(module));
tcx.sess.time("coherence_checking", || {
// Check impls constrain their parameters
let res =
tcx.hir().try_par_for_each_module(|module| tcx.ensure().check_mod_impl_wf(module));
// FIXME(matthewjasper) We shouldn't need to use `track_errors` anywhere in this function
// or the compiler in general.
res.and(tcx.sess.track_errors(|| {
for &trait_def_id in tcx.all_local_trait_impls(()).keys() {
tcx.ensure().coherent_trait(trait_def_id);
}
@ -184,7 +185,7 @@ pub fn check_crate(tcx: TyCtxt<'_>) -> Result<(), ErrorGuaranteed> {
// these queries are executed for side-effects (error reporting):
tcx.ensure().crate_inherent_impls(());
tcx.ensure().crate_inherent_impls_overlap_check(());
});
}))
})?;
if tcx.features().rustc_attrs {

View File

@ -961,8 +961,9 @@ rustc_queries! {
desc { |tcx| "checking deathness of variables in {}", describe_as_module(key, tcx) }
}
query check_mod_impl_wf(key: LocalModDefId) -> () {
query check_mod_impl_wf(key: LocalModDefId) -> Result<(), ErrorGuaranteed> {
desc { |tcx| "checking that impls are well-formed in {}", describe_as_module(key, tcx) }
ensure_forwards_result_if_red
}
query check_mod_type_wf(key: LocalModDefId) -> Result<(), ErrorGuaranteed> {

View File

@ -7,6 +7,8 @@ struct Bar;
impl Foo<char> for Bar {
fn foo<F2: Foo<u8>>(self) -> impl Foo<u8> {
//~^ ERROR: the trait bound `impl Foo<u8>: Foo<char>` is not satisfied [E0277]
//~| ERROR: the trait bound `Bar: Foo<u8>` is not satisfied [E0277]
//~| ERROR: impl has stricter requirements than trait
self
}
}

View File

@ -11,6 +11,28 @@ note: required by a bound in `Foo::{opaque#0}`
LL | fn foo<F2>(self) -> impl Foo<T>;
| ^^^^^^ required by this bound in `Foo::{opaque#0}`
error: aborting due to 1 previous error
error[E0276]: impl has stricter requirements than trait
--> $DIR/return-dont-satisfy-bounds.rs:8:16
|
LL | fn foo<F2>(self) -> impl Foo<T>;
| -------------------------------- definition of `foo` from trait
...
LL | fn foo<F2: Foo<u8>>(self) -> impl Foo<u8> {
| ^^^^^^^ impl has extra requirement `F2: Foo<u8>`
For more information about this error, try `rustc --explain E0277`.
error[E0277]: the trait bound `Bar: Foo<u8>` is not satisfied
--> $DIR/return-dont-satisfy-bounds.rs:8:34
|
LL | fn foo<F2: Foo<u8>>(self) -> impl Foo<u8> {
| ^^^^^^^^^^^^ the trait `Foo<u8>` is not implemented for `Bar`
...
LL | self
| ---- return type was inferred to be `Bar` here
|
= help: the trait `Foo<char>` is implemented for `Bar`
= help: for that trait implementation, expected `char`, found `u8`
error: aborting due to 3 previous errors
Some errors have detailed explanations: E0276, E0277.
For more information about an error, try `rustc --explain E0276`.

View File

@ -7,6 +7,16 @@ LL | impl<T: Default> A for T {
LL | impl<T: Default + ~const Sup> const A for T {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation
error: aborting due to 1 previous error
error[E0308]: mismatched types
--> $DIR/specializing-constness-2.rs:27:5
|
LL | <T as A>::a();
| ^^^^^^^^^^^^^ expected `host`, found `true`
|
= note: expected constant `host`
found constant `true`
For more information about this error, try `rustc --explain E0119`.
error: aborting due to 2 previous errors
Some errors have detailed explanations: E0119, E0308.
For more information about an error, try `rustc --explain E0119`.