Skip to content

Commit

Permalink
Auto merge of #120780 - fmease:lta-in-impls, r=oli-obk
Browse files Browse the repository at this point in the history
Properly deal with weak alias types as self types of impls

Fixes #114216.
Fixes #116100.

Not super happy about the two ad hoc “normalization” implementations for weak alias types:

1. In `inherent_impls`: The “peeling”, normalization to [“WHNF”][whnf]: Semantically that's exactly what we want (neither proper normalization nor shallow normalization would be correct here). Basically a weak alias type is “nominal” (well...^^) if the WHNF is nominal. [#97974](#97974) followed the same approach.
2. In `constrained_generic_params`: Generic parameters are constrained by a weak alias type if the corresp. “normalized” type constrains them (where we only normalize *weak* alias types not arbitrary ones). Weak alias types are injective if the corresp. “normalized” type is injective.

Both have ad hoc overflow detection mechanisms.

**Coherence** is handled in #117164.

r? `@oli-obk` or types

[whnf]: https://en.wikipedia.org/wiki/Lambda_calculus_definition#Weak_head_normal_form
  • Loading branch information
bors committed Feb 18, 2024
2 parents 1f8e824 + fde4556 commit d3df8ff
Show file tree
Hide file tree
Showing 18 changed files with 298 additions and 18 deletions.
31 changes: 30 additions & 1 deletion compiler/rustc_hir_analysis/src/coherence/inherent_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ impl<'tcx> InherentCollect<'tcx> {
let id = id.owner_id.def_id;
let item_span = self.tcx.def_span(id);
let self_ty = self.tcx.type_of(id).instantiate_identity();
let self_ty = peel_off_weak_aliases(self.tcx, self_ty);
match *self_ty.kind() {
ty::Adt(def, _) => self.check_def_id(id, self_ty, def.did()),
ty::Foreign(did) => self.check_def_id(id, self_ty, did),
Expand All @@ -166,14 +167,15 @@ impl<'tcx> InherentCollect<'tcx> {
| ty::Never
| ty::FnPtr(_)
| ty::Tuple(..) => self.check_primitive_impl(id, self_ty),
ty::Alias(..) | ty::Param(_) => {
ty::Alias(ty::Projection | ty::Inherent | ty::Opaque, _) | ty::Param(_) => {
Err(self.tcx.dcx().emit_err(errors::InherentNominal { span: item_span }))
}
ty::FnDef(..)
| ty::Closure(..)
| ty::CoroutineClosure(..)
| ty::Coroutine(..)
| ty::CoroutineWitness(..)
| ty::Alias(ty::Weak, _)
| ty::Bound(..)
| ty::Placeholder(_)
| ty::Infer(_) => {
Expand All @@ -184,3 +186,30 @@ impl<'tcx> InherentCollect<'tcx> {
}
}
}

/// Peel off all weak alias types in this type until there are none left.
///
/// <div class="warning">
///
/// This assumes that `ty` gets normalized later and that any overflows occurring
/// during said normalization get reported.
///
/// </div>
fn peel_off_weak_aliases<'tcx>(tcx: TyCtxt<'tcx>, mut ty: Ty<'tcx>) -> Ty<'tcx> {
let ty::Alias(ty::Weak, _) = ty.kind() else { return ty };

let limit = tcx.recursion_limit();
let mut depth = 0;

while let ty::Alias(ty::Weak, alias) = ty.kind() {
if !limit.value_within_limit(depth) {
let guar = tcx.dcx().delayed_bug("overflow expanding weak alias type");
return Ty::new_error(tcx, guar);
}

ty = tcx.type_of(alias.def_id).instantiate(tcx, alias.args);
depth += 1;
}

ty
}
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/collect/predicates_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen
tcx,
&mut predicates,
trait_ref,
&mut cgp::parameters_for_impl(self_ty, trait_ref),
&mut cgp::parameters_for_impl(tcx, self_ty, trait_ref),
);
}

Expand Down
43 changes: 33 additions & 10 deletions compiler/rustc_hir_analysis/src/constrained_generic_params.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_middle::ty::visit::{TypeSuperVisitable, TypeVisitable, TypeVisitor};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_span::Span;
Expand Down Expand Up @@ -27,12 +28,13 @@ impl From<ty::ParamConst> for Parameter {

/// Returns the set of parameters constrained by the impl header.
pub fn parameters_for_impl<'tcx>(
tcx: TyCtxt<'tcx>,
impl_self_ty: Ty<'tcx>,
impl_trait_ref: Option<ty::TraitRef<'tcx>>,
) -> FxHashSet<Parameter> {
let vec = match impl_trait_ref {
Some(tr) => parameters_for(&tr, false),
None => parameters_for(&impl_self_ty, false),
Some(tr) => parameters_for(tcx, &tr, false),
None => parameters_for(tcx, &impl_self_ty, false),
};
vec.into_iter().collect()
}
Expand All @@ -43,26 +45,47 @@ pub fn parameters_for_impl<'tcx>(
/// of parameters whose values are needed in order to constrain `ty` - these
/// differ, with the latter being a superset, in the presence of projections.
pub fn parameters_for<'tcx>(
tcx: TyCtxt<'tcx>,
t: &impl TypeVisitable<TyCtxt<'tcx>>,
include_nonconstraining: bool,
) -> Vec<Parameter> {
let mut collector = ParameterCollector { parameters: vec![], include_nonconstraining };
let mut collector =
ParameterCollector { tcx, parameters: vec![], include_nonconstraining, depth: 0 };
t.visit_with(&mut collector);
collector.parameters
}

struct ParameterCollector {
struct ParameterCollector<'tcx> {
tcx: TyCtxt<'tcx>,
parameters: Vec<Parameter>,
include_nonconstraining: bool,
depth: usize,
}

impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for ParameterCollector {
impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for ParameterCollector<'tcx> {
fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
match *t.kind() {
ty::Alias(..) if !self.include_nonconstraining => {
// projections are not injective
ty::Alias(ty::Projection | ty::Inherent | ty::Opaque, _)
if !self.include_nonconstraining =>
{
// Projections are not injective in general.
return ControlFlow::Continue(());
}
ty::Alias(ty::Weak, alias) if !self.include_nonconstraining => {
if !self.tcx.recursion_limit().value_within_limit(self.depth) {
// Other constituent types may still constrain some generic params, consider
// `<T> (Overflow, T)` for example. Therefore we want to continue instead of
// breaking. Only affects diagnostics.
return ControlFlow::Continue(());
}
self.depth += 1;
return ensure_sufficient_stack(|| {
self.tcx
.type_of(alias.def_id)
.instantiate(self.tcx, alias.args)
.visit_with(self)
});
}
ty::Param(data) => {
self.parameters.push(Parameter::from(data));
}
Expand All @@ -82,7 +105,7 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for ParameterCollector {
fn visit_const(&mut self, c: ty::Const<'tcx>) -> ControlFlow<Self::BreakTy> {
match c.kind() {
ty::ConstKind::Unevaluated(..) if !self.include_nonconstraining => {
// Constant expressions are not injective
// Constant expressions are not injective in general.
return c.ty().visit_with(self);
}
ty::ConstKind::Param(data) => {
Expand Down Expand Up @@ -201,12 +224,12 @@ pub fn setup_constraining_predicates<'tcx>(
// `<<T as Bar>::Baz as Iterator>::Output = <U as Iterator>::Output`
// Then the projection only applies if `T` is known, but it still
// does not determine `U`.
let inputs = parameters_for(&projection.projection_ty, true);
let inputs = parameters_for(tcx, &projection.projection_ty, true);
let relies_only_on_inputs = inputs.iter().all(|p| input_parameters.contains(p));
if !relies_only_on_inputs {
continue;
}
input_parameters.extend(parameters_for(&projection.term, false));
input_parameters.extend(parameters_for(tcx, &projection.term, false));
} else {
continue;
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_analysis/src/impl_wf_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ fn enforce_impl_params_are_constrained(
let impl_predicates = tcx.predicates_of(impl_def_id);
let impl_trait_ref = tcx.impl_trait_ref(impl_def_id).map(ty::EarlyBinder::instantiate_identity);

let mut input_parameters = cgp::parameters_for_impl(impl_self_ty, impl_trait_ref);
let mut input_parameters = cgp::parameters_for_impl(tcx, impl_self_ty, impl_trait_ref);
cgp::identify_constrained_generic_params(
tcx,
impl_predicates,
Expand All @@ -111,7 +111,7 @@ fn enforce_impl_params_are_constrained(
match item.kind {
ty::AssocKind::Type => {
if item.defaultness(tcx).has_value() {
cgp::parameters_for(&tcx.type_of(def_id).instantiate_identity(), true)
cgp::parameters_for(tcx, &tcx.type_of(def_id).instantiate_identity(), true)
} else {
vec![]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,15 +266,15 @@ fn unconstrained_parent_impl_args<'tcx>(
continue;
}

unconstrained_parameters.extend(cgp::parameters_for(&projection_ty, true));
unconstrained_parameters.extend(cgp::parameters_for(tcx, &projection_ty, true));

for param in cgp::parameters_for(&projected_ty, false) {
for param in cgp::parameters_for(tcx, &projected_ty, false) {
if !unconstrained_parameters.contains(&param) {
constrained_params.insert(param.0);
}
}

unconstrained_parameters.extend(cgp::parameters_for(&projected_ty, true));
unconstrained_parameters.extend(cgp::parameters_for(tcx, &projected_ty, true));
}
}

Expand Down Expand Up @@ -312,7 +312,7 @@ fn check_duplicate_params<'tcx>(
parent_args: &Vec<GenericArg<'tcx>>,
span: Span,
) -> Result<(), ErrorGuaranteed> {
let mut base_params = cgp::parameters_for(parent_args, true);
let mut base_params = cgp::parameters_for(tcx, 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];
Expand Down
27 changes: 27 additions & 0 deletions tests/ui/lazy-type-alias/constrained-params.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//@ check-pass

#![feature(lazy_type_alias)]
#![allow(incomplete_features)]

type Injective<T> = Local<T>;
struct Local<T>(T);

impl<T> Injective<T> {
fn take(_: T) {}
}

trait Trait {
type Out;
fn produce() -> Self::Out;
}

impl<T: Default> Trait for Injective<T> {
type Out = T;
fn produce() -> Self::Out { T::default() }
}

fn main() {
Injective::take(0);
let _: String = Injective::produce();
let _: bool = Local::produce();
}
10 changes: 10 additions & 0 deletions tests/ui/lazy-type-alias/inherent-impls-conflicting.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#![feature(lazy_type_alias)]
#![allow(incomplete_features)]

type Alias = Local;
struct Local;

impl Alias { fn method() {} } //~ ERROR duplicate definitions with name `method`
impl Local { fn method() {} }

fn main() {}
11 changes: 11 additions & 0 deletions tests/ui/lazy-type-alias/inherent-impls-conflicting.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0592]: duplicate definitions with name `method`
--> $DIR/inherent-impls-conflicting.rs:7:14
|
LL | impl Alias { fn method() {} }
| ^^^^^^^^^^^ duplicate definitions for `method`
LL | impl Local { fn method() {} }
| ----------- other definition for `method`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0592`.
12 changes: 12 additions & 0 deletions tests/ui/lazy-type-alias/inherent-impls-not-nominal.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#![feature(lazy_type_alias)]
#![allow(incomplete_features)]

type Alias = <() as Trait>::Out;

trait Trait { type Out; }
impl Trait for () { type Out = Local; }
struct Local;

impl Alias {} //~ ERROR no nominal type found for inherent implementation

fn main() {}
11 changes: 11 additions & 0 deletions tests/ui/lazy-type-alias/inherent-impls-not-nominal.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0118]: no nominal type found for inherent implementation
--> $DIR/inherent-impls-not-nominal.rs:10:1
|
LL | impl Alias {}
| ^^^^^^^^^^ impl requires a nominal type
|
= note: either implement a trait on it or create a newtype to wrap it instead

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0118`.
43 changes: 43 additions & 0 deletions tests/ui/lazy-type-alias/inherent-impls-overflow.classic.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
error[E0275]: overflow evaluating the requirement `Loop`
--> $DIR/inherent-impls-overflow.rs:7:13
|
LL | type Loop = Loop;
| ^^^^
|
= note: in case this is a recursive type alias, consider using a struct, enum, or union instead

error[E0275]: overflow evaluating the requirement `Loop`
--> $DIR/inherent-impls-overflow.rs:9:1
|
LL | impl Loop {}
| ^^^^^^^^^^^^
|
= note: in case this is a recursive type alias, consider using a struct, enum, or union instead

error[E0275]: overflow evaluating the requirement `Poly0<((((((...,),),),),),)>`
--> $DIR/inherent-impls-overflow.rs:11:17
|
LL | type Poly0<T> = Poly1<(T,)>;
| ^^^^^^^^^^^
|
= note: in case this is a recursive type alias, consider using a struct, enum, or union instead

error[E0275]: overflow evaluating the requirement `Poly1<((((((...,),),),),),)>`
--> $DIR/inherent-impls-overflow.rs:14:17
|
LL | type Poly1<T> = Poly0<(T,)>;
| ^^^^^^^^^^^
|
= note: in case this is a recursive type alias, consider using a struct, enum, or union instead

error[E0275]: overflow evaluating the requirement `Poly1<((((((...,),),),),),)>`
--> $DIR/inherent-impls-overflow.rs:18:1
|
LL | impl Poly0<()> {}
| ^^^^^^^^^^^^^^^^^
|
= note: in case this is a recursive type alias, consider using a struct, enum, or union instead

error: aborting due to 5 previous errors

For more information about this error, try `rustc --explain E0275`.
38 changes: 38 additions & 0 deletions tests/ui/lazy-type-alias/inherent-impls-overflow.next.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
error[E0275]: overflow evaluating the requirement `Loop == _`
--> $DIR/inherent-impls-overflow.rs:9:6
|
LL | impl Loop {}
| ^^^^
|
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`inherent_impls_overflow`)

error[E0392]: type parameter `T` is never used
--> $DIR/inherent-impls-overflow.rs:11:12
|
LL | type Poly0<T> = Poly1<(T,)>;
| ^ unused type parameter
|
= help: consider removing `T` or referring to it in the body of the type alias
= help: if you intended `T` to be a const parameter, use `const T: /* Type */` instead

error[E0392]: type parameter `T` is never used
--> $DIR/inherent-impls-overflow.rs:14:12
|
LL | type Poly1<T> = Poly0<(T,)>;
| ^ unused type parameter
|
= help: consider removing `T` or referring to it in the body of the type alias
= help: if you intended `T` to be a const parameter, use `const T: /* Type */` instead

error[E0275]: overflow evaluating the requirement `Poly0<()> == _`
--> $DIR/inherent-impls-overflow.rs:18:6
|
LL | impl Poly0<()> {}
| ^^^^^^^^^
|
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`inherent_impls_overflow`)

error: aborting due to 4 previous errors

Some errors have detailed explanations: E0275, E0392.
For more information about an error, try `rustc --explain E0275`.
20 changes: 20 additions & 0 deletions tests/ui/lazy-type-alias/inherent-impls-overflow.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//@ revisions: classic next
//@[next] compile-flags: -Znext-solver

#![feature(lazy_type_alias)]
#![allow(incomplete_features)]

type Loop = Loop; //[classic]~ ERROR overflow evaluating the requirement

impl Loop {} //~ ERROR overflow evaluating the requirement

type Poly0<T> = Poly1<(T,)>;
//[classic]~^ ERROR overflow evaluating the requirement
//[next]~^^ ERROR type parameter `T` is never used
type Poly1<T> = Poly0<(T,)>;
//[classic]~^ ERROR overflow evaluating the requirement
//[next]~^^ ERROR type parameter `T` is never used

impl Poly0<()> {} //~ ERROR overflow evaluating the requirement

fn main() {}
Loading

0 comments on commit d3df8ff

Please sign in to comment.