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

Inherent associated types: Better type inference #108430

Closed
wants to merge 1 commit into from
Closed
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
69 changes: 55 additions & 14 deletions compiler/rustc_hir_analysis/src/astconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::intravisit::{walk_generics, Visitor as _};
use rustc_hir::{GenericArg, GenericArgs, OpaqueTyOrigin};
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_infer::infer::{InferCtxt, TyCtxtInferExt};
use rustc_infer::traits::ObligationCause;
use rustc_infer::infer::{InferCtxt, InferOk, TyCtxtInferExt};
use rustc_infer::traits::{ObligationCause, PredicateObligations};
use rustc_middle::infer::unify_key::{ConstVariableOrigin, ConstVariableOriginKind};
use rustc_middle::middle::stability::AllowUnstable;
use rustc_middle::ty::subst::{self, GenericArgKind, InternalSubsts, SubstsRef};
Expand All @@ -46,7 +46,9 @@ use rustc_trait_selection::traits::error_reporting::{
report_object_safety_error, suggestions::NextTypeParamName,
};
use rustc_trait_selection::traits::wf::object_region_bounds;
use rustc_trait_selection::traits::{self, astconv_object_safety_violations, ObligationCtxt};
use rustc_trait_selection::traits::{
self, astconv_object_safety_violations, NormalizeExt, ObligationCtxt,
};

use smallvec::{smallvec, SmallVec};
use std::collections::BTreeSet;
Expand Down Expand Up @@ -127,6 +129,8 @@ pub trait AstConv<'tcx> {

fn record_ty(&self, hir_id: hir::HirId, ty: Ty<'tcx>, span: Span);

fn register_predicate_obligations(&self, obligations: PredicateObligations<'tcx>);

fn astconv(&self) -> &dyn AstConv<'tcx>
where
Self: Sized,
Expand Down Expand Up @@ -2234,7 +2238,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
let mut fulfillment_errors = Vec::new();
let mut applicable_candidates: Vec<_> = candidates
.iter()
.filter_map(|&(impl_, (assoc_item, def_scope))| {
.filter_map(|&(impl_, item)| {
infcx.probe(|_| {
let ocx = ObligationCtxt::new_in_snapshot(&infcx);

Expand All @@ -2250,15 +2254,12 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
// Check whether the impl imposes obligations we have to worry about.
let impl_bounds = tcx.predicates_of(impl_);
let impl_bounds = impl_bounds.instantiate(tcx, impl_substs);

let impl_bounds = ocx.normalize(&cause, param_env, impl_bounds);

let impl_obligations = traits::predicates_for_generics(
|_, _| cause.clone(),
param_env,
impl_bounds,
);

ocx.register_obligations(impl_obligations);

let mut errors = ocx.select_where_possible();
Expand All @@ -2267,26 +2268,66 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
return None;
}

// FIXME(fmease): Unsolved vars can escape this InferCtxt snapshot.
Some((assoc_item, def_scope, infcx.resolve_vars_if_possible(impl_substs)))
let impl_substs = if !self.allow_ty_infer() {
let substs = infcx.resolve_vars_if_possible(impl_substs);
assert!(!substs.needs_infer());
Some(substs)
} else {
None
};

Some((item, impl_, impl_substs))
})
})
.collect();

if applicable_candidates.len() > 1 {
return Err(self.complain_about_ambiguous_inherent_assoc_type(
name,
applicable_candidates.into_iter().map(|(candidate, ..)| candidate).collect(),
applicable_candidates.into_iter().map(|((candidate, ..), ..)| candidate).collect(),
span,
));
}

if let Some((assoc_item, def_scope, impl_substs)) = applicable_candidates.pop() {
if let Some(((assoc_item, def_scope), impl_, probe_impl_substs)) =
applicable_candidates.pop()
{
self.check_assoc_ty(assoc_item, name, def_scope, block, span);

// FIXME(inherent_associated_types): To fully *confirm* the *probed* candidate, we still
// need to relate the Self-type with fresh item substs & register region obligations for
// regionck to prove/disprove.
// FIXME(fmease, inherent_associated_types): Register WF obligations for the Self type.
Copy link
Member

Choose a reason for hiding this comment

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

This would be fixed by representing it as a projection, so we see the Self type during wfcheck.

Copy link
Member Author

Choose a reason for hiding this comment

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

I do plan on implementing something akin to InherentProjection at least experimentally as we've already discussed here. Since I haven't looked into it that much yet, I wasn't sure if this covers wf-checking and inference for the Self type or just for the parameters & bounds of an inherent GAT.

// At the moment, we don't regionck the Self type or the substitutions.

let impl_substs;
if let Some(probe_impl_substs) = probe_impl_substs {
impl_substs = probe_impl_substs;
} else {
impl_substs = infcx.fresh_item_substs(impl_);

let impl_bounds = tcx.predicates_of(impl_);
let impl_bounds = impl_bounds.instantiate(tcx, impl_substs);
let InferOk { value: impl_bounds, obligations: norm_obligations } =
infcx.at(&cause, param_env).normalize(impl_bounds);
self.register_predicate_obligations(norm_obligations);
let impl_obligations =
traits::predicates_for_generics(|_, _| cause.clone(), param_env, impl_bounds);
self.register_predicate_obligations(impl_obligations.collect());

let impl_ty = tcx.type_of(impl_);
let impl_ty = impl_ty.subst(tcx, impl_substs);
let InferOk { value: impl_ty, obligations: norm_obligations } =
infcx.at(&cause, param_env).normalize(impl_ty);
self.register_predicate_obligations(norm_obligations);

match infcx.at(&cause, param_env).eq(impl_ty, self_ty) {
Ok(ok) => self.register_predicate_obligations(ok.obligations),
Err(_) => {
tcx.sess.delay_span_bug(
span,
&format!("{self_ty:?} was a subtype of {impl_ty:?} but now it is not?"),
);
}
}
}

let item_substs =
self.create_substs_for_associated_item(span, assoc_item, segment, impl_substs);
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::{GenericParamKind, Node};
use rustc_infer::infer::{InferCtxt, TyCtxtInferExt};
use rustc_infer::traits::ObligationCause;
use rustc_infer::traits::{ObligationCause, PredicateObligations};
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::util::{Discr, IntTypeExt};
Expand Down Expand Up @@ -516,6 +516,10 @@ impl<'tcx> AstConv<'tcx> for ItemCtxt<'tcx> {
// There's no place to record types from signatures?
}

fn register_predicate_obligations(&self, _: PredicateObligations<'tcx>) {
// There's no place to track this, so just let it go.
Copy link
Member

Choose a reason for hiding this comment

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

This isn't right. We should at least ICE here.

}

fn infcx(&self) -> Option<&InferCtxt<'tcx>> {
None
}
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ mod suggestions;

pub use _impl::*;
use rustc_errors::ErrorGuaranteed;
use rustc_infer::traits::PredicateObligations;
pub use suggestions::*;

use crate::coercion::DynamicCoerceMany;
Expand Down Expand Up @@ -326,6 +327,10 @@ impl<'a, 'tcx> AstConv<'tcx> for FnCtxt<'a, 'tcx> {
self.write_ty(hir_id, ty)
}

fn register_predicate_obligations(&self, obligations: PredicateObligations<'tcx>) {
self.register_predicates(obligations)
}

fn infcx(&self) -> Option<&infer::InferCtxt<'tcx>> {
Some(&self.infcx)
}
Expand Down
23 changes: 0 additions & 23 deletions tests/ui/associated-inherent-types/bugs/ice-substitution.rs

This file was deleted.

This file was deleted.

15 changes: 0 additions & 15 deletions tests/ui/associated-inherent-types/bugs/inference-fail.rs

This file was deleted.

16 changes: 16 additions & 0 deletions tests/ui/associated-inherent-types/former-subst-ice.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// check-pass
Copy link
Member Author

@fmease fmease Feb 24, 2023

Choose a reason for hiding this comment

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

Not sure if this test is still useful since it's kind of superseded by inference.rs. I've solely kept it to explicitly demonstrate that bugs/ice-substitution.rs is fixed in this PR.


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

struct Cont<T>(T);

impl<T: Copy> Cont<T> {
type Out = Vec<T>;
}

pub fn weird<T: Copy>(x: T) {
let _: Cont<_>::Out = vec![true];
}

fn main() {}
11 changes: 11 additions & 0 deletions tests/ui/associated-inherent-types/inference-fail.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#![feature(inherent_associated_types)]
#![allow(incomplete_features)]

struct S<T>(T);

impl<T> S<T> { type P = (); }

fn main() {
// There is no way to infer this type.
let _: S<_>::P = (); //~ ERROR type annotations needed
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0282]: type annotations needed
--> $DIR/inference-fail.rs:14:14
--> $DIR/inference-fail.rs:10:12
|
LL | let _: S<_>::P;
| ^ cannot infer type
LL | let _: S<_>::P = ();
| ^^^^^^^ cannot infer type

error: aborting due to previous error

Expand Down
39 changes: 39 additions & 0 deletions tests/ui/associated-inherent-types/inference.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Testing inference capabilities.
// check-pass

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

use std::convert::identity;

struct Container<T>(T);

impl Container<u32> {
type Sink = ();
}

impl<Any> Container<Any> {
type Thing = Any;
}

impl<T> Container<(T, ())> {
type Output = ((), Wrapped<T>);
}

fn main() {
// Inferred via the Self type of the impl.
let _: Container<_>::Sink;

// Inferred via the RHS:

let _: Container<_>::Thing = 0;

let _: Container<Wrapped<_>>::Thing = Wrapped(false);

let _: Container<_>::Output = (drop(1), Wrapped("..."));

let binding: Container<_>::Thing = Default::default(); // unsolved at this point
identity::<String>(binding); // constrained and solved here
}

struct Wrapped<T>(T);
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#![feature(inherent_associated_types)]
#![allow(incomplete_features)]

struct S<T>(T);

impl<T: Copy> S<T> {
type T = T;
}

fn main() {
let _: S<_>::T = String::new(); //~ ERROR the trait bound `String: Copy` is not satisfied
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0277]: the trait bound `String: Copy` is not satisfied
--> $DIR/unsatisfied-bounds-inferred-type.rs:11:12
|
LL | let _: S<_>::T = String::new();
| ^^^^^^^ the trait `Copy` is not implemented for `String`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.