Type-directed probing for inherent associated types #105961

fmease · 2022-12-20T15:46:34Z

When probing for inherent associated types (IATs), equate the Self-type found in the projection with the Self-type of the relevant inherent impl blocks and check if all predicates are satisfied.
Previously, we didn't look at the Self-type or at the bounds and just picked the first inherent impl block containing an associated type with the name we were searching for which is obviously incorrect.

Regarding the implementation, I basically copied what we do during method probing (assemble_inherent_impl_probe, consider_probe). Unfortunately, I had to duplicate a lot of the diagnostic code found in rustc_hir_typeck::method::suggest which we don't have access to in rustc_hir_analysis. Not sure if there is a simple way to unify the error handling. Note that in the future, rustc_hir_analysis::astconv might not actually be the place where we resolve inherent associated types (see #103621 (comment)) but rustc_hir_typeck (?) in which case the duplication may naturally just disappear. While inherent associated constants are currently resolved during "method" probing, I did not find a straightforward way to incorporate IAT lookup into it as types and values (functions & constants) are two separate entities for which distinct code paths are taken.

Fixes #104251 (incl. #104251 (comment)).
Fixes #105305.
Fixes #107468.

@rustbot label T-types F-inherent_associated_types
r? types

bors · 2023-01-09T19:07:54Z

☔ The latest upstream changes (presumably #101947) made this pull request unmergeable. Please resolve the merge conflicts.

fmease · 2023-01-11T13:46:12Z

Unfortunately, I had to duplicate a lot of the diagnostic code found in rustc_hir_typeck::method::suggest which we don't have access to in rustc_hir_analysis. Not sure if there is a simple way to unify the error handling.

One makeshift solution I am thinking of is to make the subset of the error handling code that I need public definitions in rustc_hir_analysis and use it from rustc_hir_typeck to get rid of the duplication. Should I go forward with it?

jackh726 · 2023-01-11T16:38:10Z

Sorry for the delay here @fmease. I plan on getting to this by the end of the week.

fmease · 2023-01-11T16:54:04Z

No worries :) Thanks in advance!

compiler-errors · 2023-01-11T23:09:03Z

compiler/rustc_hir_analysis/src/astconv/mod.rs

+            let Ok(InferOk { obligations: sub_obligations, value: () }) = infcx
+                .at(&ObligationCause::dummy(), param_env)
+                .define_opaque_types(false)
+                .sup(impl_ty, self_ty)


Does method selection use sup? We probably should use eq here, since that's what match_impl does.

Yes, it does use sup but tbh I am not sure why exactly since it is .ignoring_regions(), too, and lifetime subtyping is the only subtyping in Rust's type system meaning eq and sup should behave identical in this situation, right? I can definitely switch to eq but I wish I could understand for what deeper reason the selection code uses sup.

rust/compiler/rustc_hir_typeck/src/method/probe.rs

Line 1516 in 222d1ff

.sup(probe.xform_self_ty, self_ty)

You likely know the following, I am just making sure I understand correctly. As far as I know method probing ignores regions since implementation-wise it uses FnCtxt which stores Inherited whose only public ctor Inherited::build constructs an InferCtxt .ignoring_regions() and since specification-wise regions are erased before codegen and they should not matter for method dispatch.

If I replace .sup with .eq here, ~~it ices~~ (no longer does since I'm now using the provided InferCtxt). Since I'm no longer ignoring_regions()s though, I am still not sure if it's a good idea to switch.

backtrace

thread 'rustc' panicked at 'index out of bounds: the len is 0 but the index is 0', compiler/rustc_infer/src/infer/region_constraints/mod.rs:459:9 stack backtrace: 0: 0x7f6d278ebac1 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hef7c34ced48956ed 1: 0x7f6d27967fbe - core::fmt::write::hd6821393d61dbc78 2: 0x7f6d27910171 - std::io::Write::write_fmt::h0aae2c130bb43485 3: 0x7f6d278eb8ea - std::sys_common::backtrace::print::hefc1376ab8e30bd1 4: 0x7f6d278eca67 - std::panicking::default_hook::{{closure}}::h133538e0d38da3d4 5: 0x7f6d278ec895 - std::panicking::default_hook::h0f8bde92f9834261 6: 0x7f6d282873c5 - rustc_driver[748794e2e144500f]::DEFAULT_HOOK::{closure#0}::{closure#0} 7: 0x7f6d278ecf73 - std::panicking::rust_panic_with_hook::h6563697fa9ccd808 8: 0x7f6d278e3089 - std::panicking::begin_panic_handler::{{closure}}::ha90000309b42d4bb 9: 0x7f6d278e2fbc - std::sys_common::backtrace::__rust_end_short_backtrace::h13266d7b6e524bca 10: 0x7f6d278ecb3a - rust_begin_unwind 11: 0x7f6d278ca4e3 - core::panicking::panic_fmt::hee5867c2cd98c87f 12: 0x7f6d278ca652 - core::panicking::panic_bounds_check::h879d026d129b4b6a 13: 0x7f6d2a4d1938 - <rustc_infer[c33f17b4b4dc7e29]::infer::region_constraints::RegionConstraintCollector>::universe 14: 0x7f6d2a5032bb - <rustc_infer[c33f17b4b4dc7e29]::infer::InferCtxt>::universe_of_region 15: 0x7f6d2a43394a - <rustc_infer[c33f17b4b4dc7e29]::infer::combine::Generalizer as rustc_middle[e87e357ae1384561]::ty::relate::TypeRelation>::regions 16: 0x7f6d2a43bca2 - rustc_middle[e87e357ae1384561]::ty::relate::super_relate_tys::<rustc_infer[c33f17b4b4dc7e29]::infer::combine::Generalizer> 17: 0x7f6d2a4330cd - <rustc_infer[c33f17b4b4dc7e29]::infer::combine::Generalizer as rustc_middle[e87e357ae1384561]::ty::relate::TypeRelation>::tys 18: 0x7f6d2a434592 - <rustc_infer[c33f17b4b4dc7e29]::infer::combine::CombineFields>::instantiate 19: 0x7f6d2a47c80f - <rustc_infer[c33f17b4b4dc7e29]::infer::equate::Equate as rustc_middle[e87e357ae1384561]::ty::relate::TypeRelation>::tys 20: 0x7f6d2a5511f6 - <rustc_middle[e87e357ae1384561]::ty::subst::GenericArg as rustc_middle[e87e357ae1384561]::ty::relate::Relate>::relate::<rustc_infer[c33f17b4b4dc7e29]::infer::equate::Equate> 21: 0x7f6d2a57204c - <core[a759e9f1dec68147]::result::Result<rustc_middle[e87e357ae1384561]::ty::subst::GenericArg, rustc_middle[e87e357ae1384561]::ty::error::TypeError> as rustc_type_ir[98cc6c8502f90b82]::InternIteratorElement<rustc_middle[e87e357ae1384561]::ty::subst::GenericArg, &rustc_middle[e87e357ae1384561]::ty::list::List<rustc_middle[e87e357ae1384561]::ty::subst::GenericArg>>>::intern_with::<core[a759e9f1dec68147]::iter::adapters::map::Map<core[a759e9f1dec68147]::iter::adapters::zip::Zip<core[a759e9f1dec68147]::iter::adapters::copied::Copied<core[a759e9f1dec68147]::slice::iter::Iter<rustc_middle[e87e357ae1384561]::ty::subst::GenericArg>>, core[a759e9f1dec68147]::iter::adapters::copied::Copied<core[a759e9f1dec68147]::slice::iter::Iter<rustc_middle[e87e357ae1384561]::ty::subst::GenericArg>>>, rustc_middle[e87e357ae1384561]::ty::relate::relate_substs<rustc_infer[c33f17b4b4dc7e29]::infer::equate::Equate>::{closure#0}>, <rustc_middle[e87e357ae1384561]::ty::context::TyCtxt>::mk_substs<core[a759e9f1dec68147]::iter::adapters::map::Map<core[a759e9f1dec68147]::iter::adapters::zip::Zip<core[a759e9f1dec68147]::iter::adapters::copied::Copied<core[a759e9f1dec68147]::slice::iter::Iter<rustc_middle[e87e357ae1384561]::ty::subst::GenericArg>>, core[a759e9f1dec68147]::iter::adapters::copied::Copied<core[a759e9f1dec68147]::slice::iter::Iter<rustc_middle[e87e357ae1384561]::ty::subst::GenericArg>>>, rustc_middle[e87e357ae1384561]::ty::relate::relate_substs<rustc_infer[c33f17b4b4dc7e29]::infer::equate::Equate>::{closure#0}>>::{closure#0}> 22: 0x7f6d2a47bac6 - <rustc_infer[c33f17b4b4dc7e29]::infer::equate::Equate as rustc_middle[e87e357ae1384561]::ty::relate::TypeRelation>::relate_item_substs 23: 0x7f6d2a43af43 - rustc_middle[e87e357ae1384561]::ty::relate::super_relate_tys::<rustc_infer[c33f17b4b4dc7e29]::infer::equate::Equate> 24: 0x7f6d2a510fc6 - <rustc_infer[c33f17b4b4dc7e29]::infer::InferCtxt>::super_combine_tys::<rustc_infer[c33f17b4b4dc7e29]::infer::equate::Equate> 25: 0x7f6d2a47c5ce - <rustc_infer[c33f17b4b4dc7e29]::infer::equate::Equate as rustc_middle[e87e357ae1384561]::ty::relate::TypeRelation>::tys 26: 0x7f6d2889837a - <rustc_infer[c33f17b4b4dc7e29]::infer::InferCtxt>::commit_if_ok::<rustc_infer[c33f17b4b4dc7e29]::infer::InferOk<()>, rustc_middle[e87e357ae1384561]::ty::error::TypeError, <rustc_infer[c33f17b4b4dc7e29]::infer::at::Trace>::eq<rustc_middle[e87e357ae1384561]::ty::Ty>::{closure#0}> 27: 0x7f6d289d2db1 - <rustc_infer[c33f17b4b4dc7e29]::infer::at::At>::eq::<rustc_middle[e87e357ae1384561]::ty::Ty> 28: 0x7f6d2894a509 - <dyn rustc_hir_analysis[2dedc18b32354586]::astconv::AstConv>::associated_path_to_ty::{closure#0} 29: 0x7f6d289499fa - <dyn rustc_hir_analysis[2dedc18b32354586]::astconv::AstConv>::associated_path_to_ty 30: 0x7f6d2894e1e8 - <dyn rustc_hir_analysis[2dedc18b32354586]::astconv::AstConv>::ast_ty_to_ty_inner 31: 0x7f6d28657fcf - <rustc_hir_typeck[5384bad6713105a9]::fn_ctxt::FnCtxt>::to_ty 32: 0x7f6d285febcf - <rustc_hir_typeck[5384bad6713105a9]::gather_locals::GatherLocalsVisitor>::declare 33: 0x7f6d287c41b3 - rustc_hir[cf8f75e2b7a3e0eb]::intravisit::walk_block::<rustc_hir_typeck[5384bad6713105a9]::gather_locals::GatherLocalsVisitor> 34: 0x7f6d286ec69d - rustc_hir_typeck[5384bad6713105a9]::check::check_fn 35: 0x7f6d286d9dac - <rustc_hir_typeck[5384bad6713105a9]::inherited::InheritedBuilder>::enter::<rustc_hir_typeck[5384bad6713105a9]::typeck_with_fallback<rustc_hir_typeck[5384bad6713105a9]::typeck::{closure#0}>::{closure#0}::{closure#1}, &rustc_middle[e87e357ae1384561]::ty::typeck_results::TypeckResults> 36: 0x7f6d2881336a - rustc_hir_typeck[5384bad6713105a9]::typeck_with_fallback::<rustc_hir_typeck[5384bad6713105a9]::typeck::{closure#0}> 37: 0x7f6d287399a4 - rustc_hir_typeck[5384bad6713105a9]::typeck 38: 0x7f6d299a66f4 - rustc_query_system[a641d3afe2b8834a]::query::plumbing::try_execute_query::<rustc_query_impl[2d661b98ef5c64e8]::queries::typeck, rustc_query_impl[2d661b98ef5c64e8]::plumbing::QueryCtxt> 39: 0x7f6d29a41b20 - rustc_query_system[a641d3afe2b8834a]::query::plumbing::get_query::<rustc_query_impl[2d661b98ef5c64e8]::queries::typeck, rustc_query_impl[2d661b98ef5c64e8]::plumbing::QueryCtxt, rustc_middle[e87e357ae1384561]::dep_graph::dep_node::DepKind> 40: 0x7f6d297542c8 - <rustc_query_impl[2d661b98ef5c64e8]::Queries as rustc_middle[e87e357ae1384561]::ty::query::QueryEngine>::typeck 41: 0x7f6d2877c016 - std[6161937930c6e443]::panicking::try::<(), core[a759e9f1dec68147]::panic::unwind_safe::AssertUnwindSafe<rustc_data_structures[ad989cddaa07d642]::sync::par_for_each_in<&[rustc_span[67e7a4497f54abf5]::def_id::LocalDefId], <rustc_middle[e87e357ae1384561]::hir::map::Map>::par_body_owners<rustc_hir_typeck[5384bad6713105a9]::typeck_item_bodies::{closure#0}>::{closure#0}>::{closure#0}::{closure#0}>> 42: 0x7f6d28811395 - rustc_data_structures[ad989cddaa07d642]::sync::par_for_each_in::<&[rustc_span[67e7a4497f54abf5]::def_id::LocalDefId], <rustc_middle[e87e357ae1384561]::hir::map::Map>::par_body_owners<rustc_hir_typeck[5384bad6713105a9]::typeck_item_bodies::{closure#0}>::{closure#0}> 43: 0x7f6d28739865 - rustc_hir_typeck[5384bad6713105a9]::typeck_item_bodies 44: 0x7f6d2996323a - rustc_query_system[a641d3afe2b8834a]::query::plumbing::try_execute_query::<rustc_query_impl[2d661b98ef5c64e8]::queries::typeck_item_bodies, rustc_query_impl[2d661b98ef5c64e8]::plumbing::QueryCtxt> 45: 0x7f6d29a141ef - rustc_query_system[a641d3afe2b8834a]::query::plumbing::get_query::<rustc_query_impl[2d661b98ef5c64e8]::queries::typeck_item_bodies, rustc_query_impl[2d661b98ef5c64e8]::plumbing::QueryCtxt, rustc_middle[e87e357ae1384561]::dep_graph::dep_node::DepKind> 46: 0x7f6d29753d8e - <rustc_query_impl[2d661b98ef5c64e8]::Queries as rustc_middle[e87e357ae1384561]::ty::query::QueryEngine>::typeck_item_bodies 47: 0x7f6d2892e4e8 - <rustc_session[29c6e5af3d515c5e]::session::Session>::time::<(), rustc_hir_analysis[2dedc18b32354586]::check_crate::{closure#7}> 48: 0x7f6d2889bdae - rustc_hir_analysis[2dedc18b32354586]::check_crate 49: 0x7f6d283c6e52 - rustc_interface[e7497e78aff66ef4]::passes::analysis 50: 0x7f6d299a87ca - rustc_query_system[a641d3afe2b8834a]::query::plumbing::try_execute_query::<rustc_query_impl[2d661b98ef5c64e8]::queries::analysis, rustc_query_impl[2d661b98ef5c64e8]::plumbing::QueryCtxt> 51: 0x7f6d29a41db1 - rustc_query_system[a641d3afe2b8834a]::query::plumbing::get_query::<rustc_query_impl[2d661b98ef5c64e8]::queries::analysis, rustc_query_impl[2d661b98ef5c64e8]::plumbing::QueryCtxt, rustc_middle[e87e357ae1384561]::dep_graph::dep_node::DepKind> 52: 0x7f6d29735a5e - <rustc_query_impl[2d661b98ef5c64e8]::Queries as rustc_middle[e87e357ae1384561]::ty::query::QueryEngine>::analysis 53: 0x7f6d2828fb05 - <rustc_interface[e7497e78aff66ef4]::passes::QueryContext>::enter::<rustc_driver[748794e2e144500f]::run_compiler::{closure#1}::{closure#2}::{closure#3}, core[a759e9f1dec68147]::result::Result<(), rustc_errors[aafc41f3c6a3f30f]::ErrorGuaranteed>> 54: 0x7f6d2829103c - <rustc_interface[e7497e78aff66ef4]::interface::Compiler>::enter::<rustc_driver[748794e2e144500f]::run_compiler::{closure#1}::{closure#2}, core[a759e9f1dec68147]::result::Result<core[a759e9f1dec68147]::option::Option<rustc_interface[e7497e78aff66ef4]::queries::Linker>, rustc_errors[aafc41f3c6a3f30f]::ErrorGuaranteed>> 55: 0x7f6d2826f966 - rustc_span[67e7a4497f54abf5]::with_source_map::<core[a759e9f1dec68147]::result::Result<(), rustc_errors[aafc41f3c6a3f30f]::ErrorGuaranteed>, rustc_interface[e7497e78aff66ef4]::interface::run_compiler<core[a759e9f1dec68147]::result::Result<(), rustc_errors[aafc41f3c6a3f30f]::ErrorGuaranteed>, rustc_driver[748794e2e144500f]::run_compiler::{closure#1}>::{closure#0}::{closure#0}> 56: 0x7f6d282933fc - <scoped_tls[535ca24a62cbd8c4]::ScopedKey<rustc_span[67e7a4497f54abf5]::SessionGlobals>>::set::<rustc_interface[e7497e78aff66ef4]::interface::run_compiler<core[a759e9f1dec68147]::result::Result<(), rustc_errors[aafc41f3c6a3f30f]::ErrorGuaranteed>, rustc_driver[748794e2e144500f]::run_compiler::{closure#1}>::{closure#0}, core[a759e9f1dec68147]::result::Result<(), rustc_errors[aafc41f3c6a3f30f]::ErrorGuaranteed>> 57: 0x7f6d2827c1aa - std[6161937930c6e443]::sys_common::backtrace::__rust_begin_short_backtrace::<rustc_interface[e7497e78aff66ef4]::util::run_in_thread_pool_with_globals<rustc_interface[e7497e78aff66ef4]::interface::run_compiler<core[a759e9f1dec68147]::result::Result<(), rustc_errors[aafc41f3c6a3f30f]::ErrorGuaranteed>, rustc_driver[748794e2e144500f]::run_compiler::{closure#1}>::{closure#0}, core[a759e9f1dec68147]::result::Result<(), rustc_errors[aafc41f3c6a3f30f]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[a759e9f1dec68147]::result::Result<(), rustc_errors[aafc41f3c6a3f30f]::ErrorGuaranteed>> 58: 0x7f6d282976ee - std[6161937930c6e443]::panicking::try::<core[a759e9f1dec68147]::result::Result<(), rustc_errors[aafc41f3c6a3f30f]::ErrorGuaranteed>, core[a759e9f1dec68147]::panic::unwind_safe::AssertUnwindSafe<<std[6161937930c6e443]::thread::Builder>::spawn_unchecked_<rustc_interface[e7497e78aff66ef4]::util::run_in_thread_pool_with_globals<rustc_interface[e7497e78aff66ef4]::interface::run_compiler<core[a759e9f1dec68147]::result::Result<(), rustc_errors[aafc41f3c6a3f30f]::ErrorGuaranteed>, rustc_driver[748794e2e144500f]::run_compiler::{closure#1}>::{closure#0}, core[a759e9f1dec68147]::result::Result<(), rustc_errors[aafc41f3c6a3f30f]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[a759e9f1dec68147]::result::Result<(), rustc_errors[aafc41f3c6a3f30f]::ErrorGuaranteed>>::{closure#1}::{closure#0}>> 59: 0x7f6d2828c646 - <<std[6161937930c6e443]::thread::Builder>::spawn_unchecked_<rustc_interface[e7497e78aff66ef4]::util::run_in_thread_pool_with_globals<rustc_interface[e7497e78aff66ef4]::interface::run_compiler<core[a759e9f1dec68147]::result::Result<(), rustc_errors[aafc41f3c6a3f30f]::ErrorGuaranteed>, rustc_driver[748794e2e144500f]::run_compiler::{closure#1}>::{closure#0}, core[a759e9f1dec68147]::result::Result<(), rustc_errors[aafc41f3c6a3f30f]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[a759e9f1dec68147]::result::Result<(), rustc_errors[aafc41f3c6a3f30f]::ErrorGuaranteed>>::{closure#1} as core[a759e9f1dec68147]::ops::function::FnOnce<()>>::call_once::{shim:vtable#0} 60: 0x7f6d279038b8 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::hfe0a9abdbb746703 61: 0x7f6d278f1877 - std::sys::unix::thread::Thread::new::thread_start::h84bfc89499a70691 62: 0x7f6d276f78fd - <unknown> 63: 0x7f6d27779a60 - <unknown> 64: 0x0 - <unknown> error: internal compiler error: unexpected panic note: the compiler unexpectedly panicked. this is a bug. note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md note: rustc 1.69.0-dev running on x86_64-unknown-linux-gnu note: compiler flags: -Z threads=1 -C codegen-units=1 -Z ui-testing -Z simulate-remapped-rust-src-base=/rustc/FAKE_PREFIX -Z translate-remapped-path-to-local-path=no -Z deduplicate-diagnostics=no -C strip=debuginfo -C prefer-dynamic -C rpath -C debuginfo=0 query stack during panic: #0 [typeck] type-checking `main` #1 [typeck_item_bodies] type-checking all item bodies #2 [analysis] running analysis passes on this crate end of query stack

compiler-errors · 2023-01-11T23:11:10Z

compiler/rustc_hir_analysis/src/astconv/mod.rs

+                self.create_substs_for_associated_item(span, assoc_item, segment, adt_substs);
+            // FIXME(inherent_associated_types): Check if the obligations arising from the
+            // where-clause & the bounds on the associated type and its parameters hold.
+            let ty = tcx.bound_type_of(assoc_item).subst(tcx, item_substs);


I wonder if astconv should turn this into a ty::Projection with the impl item def id, then let normalization handle projecting it onto the real type and confirming the GAT substitutions. It would fix this comment:

Check if the obligations arising from the where-clause & the bounds on the associated type and its parameters hold.

That FIXME now has its own issue btw, namely #106722. I can try to implement your suggestion and return a type of kind TyKind::Alias(AliasKind::Projection, AliasTy { … }). I faintly remember trying this before and it lead to ICEs down the line where the compiler wasn't happy that inherent assoc ty projections obviously lacked a trait reference. The docs of AliasTy.def_id confirm this: “The DefId of the TraitItem […] if this is a projection”. Let's see if I can find another way. I wonder if rust-lang/compiler-team#504 is relevant here (to some extend I mean since we are talking about assoc tys here not top-level type aliases).

Should I attempt this in this PR though, I wonder? I want to keep things “minimal” to make life easier for reviewers and since I don't even know if the general disposition for this PR is merging or closing as the implementation is not quite clean (yet).

Do you think we are in need of a new AliasKind::InherentProjection (modulo naming)? I am not deeply familiar with the inner workings of the type system yet but as far as I understand at some point in the future we do need to map polymorphic inherent associated types to some form of placeholder projection types for lazy normalization & proper unification for them and to finally pave the way towards the long-awaited stabilization of IATs. Did I get that right?

Do you think we are in need of a new AliasKind::InherentProjection (modulo naming)?

I am currently investigating this approach.

I think this would be good as a followup.

bors · 2023-01-14T00:32:21Z

☔ The latest upstream changes (presumably #106822) made this pull request unmergeable. Please resolve the merge conflicts.

jackh726

Sorry that this has taken so long :/

I think my comments here could be added as FIXMEs and we could land this mostly as-is. But of course, address as much as you'd like/can.

jackh726 · 2023-01-25T23:04:29Z

tests/ui/associated-inherent-types/dispatch-on-self-type-0.rs

+    let _: Select<String>::Projection = false;
+    let _: Select<u8>::Projection = ();
+
+    let _: Choose<NonCopy>::Result = ();
+    let _: Choose<&str>::Result = vec!["..."];


For followup work: it would be nice to see tests where we don't have a concrete type, but instead something from the environment.

I've added two tests to make sure we use the correct ParamEnv. The first one is in fn parameterized in dispatch-on-self-type-0.rs which is check-pass and the second one is not-found-unsatisfied-bounds-1.rs which is check-fail. I did in fact not use the correct ParamEnv before. Now I use the environment of block.owner instead of the one of the impl block. Let me know if you'd like to see more ParamEnv tests (in a follow-up PR maybe).

compiler/rustc_hir_analysis/src/astconv/mod.rs

jackh726 · 2023-01-25T23:08:57Z

compiler/rustc_hir_analysis/src/astconv/mod.rs

+            // Evaluate those obligations to see if they might possibly hold.
+            for o in candidate_obligations {
+                let o = infcx.resolve_vars_if_possible(o);
+                if !infcx.predicate_may_hold(&o) {
+                    matches = false;
+                    unsatisfied_predicates.push(o.predicate);
+                }
+            }
+
+            // Evaluate those obligations to see if they might possibly hold.
+            for o in sub_obligations {
+                let o = infcx.resolve_vars_if_possible(o);
+                if !infcx.predicate_may_hold(&o) {
+                    matches = false;
+                    unsatisfied_predicates.push(o.predicate);
+                }
+            }


I imagine it might make more sense to use an ObligationCtxt here. Just register all the obligations and then call select_where_possible (maybe? would have to think a bit harder about it).

It also has helpful operations for the some of the above.

I've switched to using an ObligationCtxt as suggested.

jackh726 · 2023-01-25T23:11:14Z

compiler/rustc_hir_analysis/src/astconv/mod.rs

+        Err(self.complain_about_inherent_assoc_type_not_found(
+            name,
+            self_ty,
+            &candidates.into_iter().map(|(impl_, _)| impl_).collect::<Vec<_>>(),
+            unsatisfied_predicates,
+            span,
+        ))


Is it helpful to combine all unsatisfied predicates?

No, it is not. Not sure what I was thinking. Fixed in the latest commit.

jackh726 · 2023-01-25T23:11:42Z

compiler/rustc_hir_analysis/src/astconv/mod.rs

+                self.create_substs_for_associated_item(span, assoc_item, segment, adt_substs);
+            // FIXME(inherent_associated_types): Check if the obligations arising from the
+            // where-clause & the bounds on the associated type and its parameters hold.
+            let ty = tcx.bound_type_of(assoc_item).subst(tcx, item_substs);


I think this would be good as a followup.

bors · 2023-01-28T10:31:24Z

☔ The latest upstream changes (presumably #107400) made this pull request unmergeable. Please resolve the merge conflicts.

fmease · 2023-01-30T16:44:41Z

I've addressed the review comments either as a fix or as a FIXME for follow-up PRs. I've not squashed the commits for ease of review but I can do so after approval.

~~I am no longer sure if it's safe to ignore_regions here since I am conflating the probe and the confirmation phase. I have to think about this but it shouldn't be a blocker (?).~~ I am now using the InferCtxt provided by AstConv which does not ignore regions.

cjgillot · 2023-01-21T15:45:23Z

compiler/rustc_hir_analysis/src/astconv/mod.rs

+            &candidates.into_iter().map(|(impl_, _)| impl_).collect::<Vec<_>>(),
+            unsatisfied_predicates,
+            span,
+        ))


Should we error here? What happens if there is an associated type through a trait?

~~You are right, we should probably not eagerly error here.~~ Returning early yields a ~~better~~ different error message (compared to master). In terms of correctness, we wouldn't be able to shadow any associated types defined in trait impls, since they would be ambiguous anyway. Right? Since in such cases, the Self-type would always refer to a concrete type constructor (and not to a type parameter for example).

// pseudo ui test #![feature(inherent_associated_types)] #![allow(incomplete_features)] struct S<T>(T); trait Tr { type Pr; const PR: Self::Pr; } impl<T> Tr for S<T> { type Pr = (); const PR: () = (); } #[cfg(branch_is_fmease_iat_type_directed_probing)] impl S<()> { type Pr = i32; const PR: Self::Pr = 10; } fn main() { let _: () = S::<bool>::PR; // PASS (pass through) let _: S::<bool>::Pr = (); //[master]~^ ERROR[E0223] ambiguous associated type //[fmease:iat-type-directed-probing]~^^ ERROR associated type […] not found }

Do you consider this a diagnostic regression?

~~[@]rustbot author~~
@rustbot reviewer

jackh726

Just a few comments. I wouldn't mind leaving them as FIXMEs for now though.

jackh726 · 2023-02-19T20:46:47Z

tests/ui/associated-inherent-types/not-found-unsatisfied-bounds-in-multiple-impls.stderr

+error: the associated type `X` exists for `S<Featureless, Featureless>`, but its trait bounds were not satisfied
+  --> $DIR/not-found-unsatisfied-bounds-in-multiple-impls.rs:19:43
+   |
+LL | struct S<A, B>(A, B);
+   | -------------- associated item `X` not found for this struct
+LL | struct Featureless;
+   | ------------------
+   | |
+   | doesn't satisfy `Featureless: One`
+   | doesn't satisfy `Featureless: Two`
+...
+LL |     let _: S::<Featureless, Featureless>::X;
+   |                                           ^ associated type cannot be referenced on `S<Featureless, Featureless>` due to unsatisfied trait bounds
+   |
+   = note: the following trait bounds were not satisfied:
+           `Featureless: One`
+           `Featureless: Two`
+
+error: aborting due to previous error
+


So, this is a good test to showcase the current problem with fulfillment_errors being a Vec<FulfillmentError>.

Ideally, this error should point to this impl impl<T: One> S<Featureless, T>, then say that Featureless: One is not satisfied. Then point at this impl impl<T: Two> S<T, Featureless> and say that Featureless: Two is not satisfied.

Right now, you aren't pointing to the impls and are conflating what bounds must be satisfied for and individual impl to apply.

I see. However, this shouldn't be hard to fix. I basically have to copy over the changes made in #106702. When I opened my PR, I copied over the now outdated suggestion code. Before #106702, the equivalent code with assoc fns / consts had the same diagnostic. See for yourself: Playground (stable), playground (nightly).

I can update the diagnostic code once I've fixed the confirmation code.

yep, exactly the same thing actually!

jackh726 · 2023-02-19T20:52:01Z

tests/ui/associated-inherent-types/not-found-unsatisfied-bounds-in-multiple-impls.rs

+impl<T: Two> S<T, Featureless> {
+    type X = String;
+}
+


So, re coherence, if you added both impl One for Featureless and impl Two for Featureless, then both of these inherent impls could apply. Without the associated types, this is fine, but we do in fact complain in inherent_impls_overlap.

jackh726 · 2023-02-19T20:59:26Z

compiler/rustc_hir_analysis/src/astconv/errors.rs

+            }
+        };
+
+        if fulfillment_errors.is_empty() {


I think it would make more sense to pass a Vec<(DefId, (DefId, DefId), Option<Vec<FulfillmentError>>)>.

I see why checking if for is_empty() here is okay: basically, this is the case where all the Self types are different (which should be obvious by just enumerating the candidates). This could be the "simple path": if all the candidates have no fulfillment errors -> list (as is done now).

Otherwise, for each candidate, list the unsatisfied bounds.

jackh726 · 2023-02-19T21:04:07Z

compiler/rustc_hir_analysis/src/astconv/mod.rs

+            // FIXME(inherent_associated_types): To fully *confirm* the *probed* candidate,
+            // we still need to register region obligations for regionck to prove/disprove.


You should create new fresh item substs, then relate to the self ty (as done before).

But this can be left to another time.

Please add a FIXME though

jackh726 · 2023-02-19T22:39:27Z

There is obviously followup work for this, but given it's an unstable feature, I'm fine with these being FIXMEs for now.

Sorry for the review latency here. Thanks for addressing the reviews.

@bors r+

bors · 2023-02-19T22:39:29Z

📌 Commit f2253da has been approved by jackh726

It is now in the queue for this repository.

bors · 2023-02-20T00:37:24Z

⌛ Testing commit f2253da with merge 7b55296...

bors · 2023-02-20T03:37:36Z

☀️ Test successful - checks-actions
Approved by: jackh726
Pushing 7b55296 to master...

rust-timer · 2023-02-20T05:02:26Z

Finished benchmarking commit (7b55296): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

This benchmark run did not return any relevant results for this metric.

…compiler-errors Introduce `AliasKind::Inherent` for inherent associated types Allows us to check (possibly generic) inherent associated types for well-formedness. Type inference now also works properly. Follow-up to rust-lang#105961. Supersedes rust-lang#108430. Fixes rust-lang#106722. Fixes rust-lang#108957. Fixes rust-lang#109768. Fixes rust-lang#109789. Fixes rust-lang#109790. ~Not to be merged before rust-lang#108860 (`AliasKind::Weak`).~ CC `@jackh726` r? `@compiler-errors` `@rustbot` label T-types F-inherent_associated_types

rustbot assigned jackh726 Dec 20, 2022

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. F-inherent_associated_types `#![feature(inherent_associated_types)]` labels Dec 20, 2022

fmease force-pushed the iat-type-directed-probing branch 4 times, most recently from d784f44 to 1177252 Compare December 21, 2022 15:10

fmease force-pushed the iat-type-directed-probing branch 2 times, most recently from fb03d8e to 35f5315 Compare January 11, 2023 13:38

fmease mentioned this pull request Jan 11, 2023

Inherent associated types defined on non-ADTs don't resolve successfully #106719

Open

fmease mentioned this pull request Jan 11, 2023

Bounds on parameters of inherent GATs are not respected #106722

Closed

fmease force-pushed the iat-type-directed-probing branch from 35f5315 to d366229 Compare January 11, 2023 15:10

compiler-errors reviewed Jan 11, 2023

View reviewed changes

rustbot added the T-types Relevant to the types team, which will review and decide on the PR/issue. label Jan 13, 2023

fmease force-pushed the iat-type-directed-probing branch from d366229 to 5094a8e Compare January 14, 2023 01:26

jackh726 requested changes Jan 25, 2023

View reviewed changes

fmease force-pushed the iat-type-directed-probing branch from 5094a8e to 120e80b Compare January 30, 2023 16:29

fmease requested a review from jackh726 January 30, 2023 16:50

cjgillot reviewed Jan 30, 2023

View reviewed changes

fmease added 4 commits February 19, 2023 18:35

Use InferCtxt::probe to properly detect ambiguous candidates

6065867

Fix substitution bug

77ea90e

Add a test and several known bugs

6eb6455

Deduplicate fresh_item_substs

569ca2b

fmease force-pushed the iat-type-directed-probing branch from 4f72470 to 422b3fd Compare February 19, 2023 17:38

jackh726 reviewed Feb 19, 2023

View reviewed changes

fmease added 2 commits February 19, 2023 22:54

Collect fulfillment errors across impls

00b976a

Add some FIXMEs for follow-up PRs

f2253da

fmease force-pushed the iat-type-directed-probing branch from 422b3fd to f2253da Compare February 19, 2023 21:56

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 19, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 20, 2023

bors merged commit 7b55296 into rust-lang:master Feb 20, 2023

rustbot added this to the 1.69.0 milestone Feb 20, 2023

fmease deleted the iat-type-directed-probing branch February 20, 2023 12:27

This was referenced Feb 24, 2023

Inherent associated types: Better type inference #108430

Closed

Tracking issue for inherent associated types #8995

Open

fmease mentioned this pull request Mar 9, 2023

ICE: inherent_associated_types, IOOB, snapshot vec #108957

Closed

fmease mentioned this pull request Mar 20, 2023

Introduce AliasKind::Inherent for inherent associated types #109410

Merged

matthiaskrgr mentioned this pull request Mar 30, 2023

ICE: type variables should not be hashed: _#0t #109768

Closed

matthiaskrgr mentioned this pull request May 9, 2023

ICE: inherent_associated_types: Unexpected parameter Type(S<'a>) when substituting in region 'a (index=0) with -Zdump-mir=all #111390

Closed

matthiaskrgr mentioned this pull request May 23, 2023

ICE: inherent_associated_types: Unexpected parameter Type(..) ... when substituting in region.. #111879

Closed

matthiaskrgr mentioned this pull request Nov 17, 2023

ICE: cycle detected when computing function signature of #110106

Closed

		// FIXME(inherent_associated_types): To fully confirm the probed candidate,
		// we still need to register region obligations for regionck to prove/disprove.

Type-directed probing for inherent associated types #105961

Type-directed probing for inherent associated types #105961

Conversation

fmease commented Dec 20, 2022 • edited Loading

bors commented Jan 9, 2023

fmease commented Jan 11, 2023 • edited Loading

jackh726 commented Jan 11, 2023

fmease commented Jan 11, 2023

Choose a reason for hiding this comment

fmease Jan 12, 2023 • edited Loading

Choose a reason for hiding this comment

fmease Jan 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmease Jan 12, 2023 • edited Loading

Choose a reason for hiding this comment

fmease Jan 13, 2023 • edited Loading

Choose a reason for hiding this comment

fmease Jan 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Jan 14, 2023

jackh726 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmease Jan 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmease Jan 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Jan 28, 2023

fmease commented Jan 30, 2023 • edited Loading

Choose a reason for hiding this comment

fmease Jan 30, 2023 • edited Loading

Choose a reason for hiding this comment

jackh726 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jackh726 commented Feb 19, 2023

bors commented Feb 19, 2023

bors commented Feb 20, 2023

bors commented Feb 20, 2023

rust-timer commented Feb 20, 2023

Overall result: no relevant changes - no action needed

fmease commented Dec 20, 2022 •

edited

Loading

fmease commented Jan 11, 2023 •

edited

Loading

fmease Jan 12, 2023 •

edited

Loading

fmease Jan 30, 2023 •

edited

Loading

fmease Jan 12, 2023 •

edited

Loading

fmease Jan 13, 2023 •

edited

Loading

fmease Jan 25, 2023 •

edited

Loading

fmease Jan 30, 2023 •

edited

Loading

fmease Jan 30, 2023 •

edited

Loading

fmease commented Jan 30, 2023 •

edited

Loading

fmease Jan 30, 2023 •

edited

Loading