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

GAT/const_generics: Allow with_opt_const_param to return GAT param def_id #81911

Merged
merged 5 commits into from
Feb 13, 2021
Merged
Show file tree
Hide file tree
Changes from 4 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
58 changes: 58 additions & 0 deletions compiler/rustc_typeck/src/collect/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,64 @@ pub(super) fn opt_const_param_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<
let parent_node = tcx.hir().get(parent_node_id);

match parent_node {
// This matches on types who's paths couldn't be resolved without typeck'ing e.g.
Copy link
Contributor

Choose a reason for hiding this comment

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

I was reading the rustdoc for the opt_const_param_of query and I think this isn't quite right.

This query would, I think, be invoked on the 3 in your example, and it would be returning the const N1: usize node.

It doesn't have much to do with Self::Assoc in particular, from what I can tell, except that this is the context in which the 3 appears.

Copy link
Contributor

Choose a reason for hiding this comment

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

In other words, I think if you wrote <Self as Foo>::Assoc<3> the query could still be invoked

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I'm wrong, I see your point. The query is invoked on 3, but this match arm is specific to the case where the 3 appears as part of a Self::Assoc<3> type thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

(I am leaving my stream of consciousness to help guide you in how to improve the comment)

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 rewrote that comment to put more emphasis on what some of the variables correspond to and what exactly the match arm matches on :)

//
// trait Foo {
// type Assoc<const N1: usize>;
// fn foo() -> Self::Assoc<3>;
// // note: if the def_id argument is the 3 then in this example
// // parent_node would be the node for Self::Assoc<_>
// }
// We didnt write <Self as Foo>::Assoc so the Self::Assoc<_> is lowered to QPath::TypeRelative.
// I believe this match arm is only needed for GAT but I am not 100% sure - BoxyUwU
Node::Ty(hir_ty @ Ty { kind: TyKind::Path(QPath::TypeRelative(_, segment)), .. }) => {
nikomatsakis marked this conversation as resolved.
Show resolved Hide resolved
// Walk up from the parent_node to find an item so that
// we can resolve the relative path to an actual associated type.
// For the code example above, this item would be the Foo trait.
Copy link
Contributor

Choose a reason for hiding this comment

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

I found this comment confusing. I think what's going on is this:

  • Find the item I that contains the anonymous constant so that we can create the context for it
  • Using that context, we will convert the HIR for Self::Assoc<3> into a type, which will be a fully resolved projection like <Self as Foo>::Assoc<3>

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea I wasn't great at this comment as I'm not all that familiar with the exact details of this Item stuff. What you said sounds correct to me so I'll just uh take that and put it in the comment x]

let item_hir_id = tcx
.hir()
.parent_iter(hir_id)
.filter(|(_, node)| matches!(node, Node::Item(_)))
.map(|(id, _)| id)
.next()
.unwrap();
let item_did = tcx.hir().local_def_id(item_hir_id).to_def_id();
let item_ctxt = &ItemCtxt::new(tcx, item_did) as &dyn crate::astconv::AstConv<'_>;

// This ty will be the actual associated type so that we can
// go through its generics to find which param our def_id corresponds to.
// For the code example above, this ty would be the Assoc<const N1: usize>.
let ty = item_ctxt.ast_ty_to_ty(hir_ty);
if let ty::Projection(projection) = ty.kind() {
let generics = tcx.generics_of(projection.item_def_id);

let arg_index = segment
nikomatsakis marked this conversation as resolved.
Show resolved Hide resolved
.args
.and_then(|args| {
args.args
.iter()
.filter(|arg| arg.is_const())
.position(|arg| arg.id() == hir_id)
})
.unwrap_or_else(|| {
bug!("no arg matching AnonConst in segment");
});

return generics
.params
.iter()
.filter(|param| matches!(param.kind, ty::GenericParamDefKind::Const))
.nth(arg_index)
.map(|param| param.def_id);
}

// I dont think it's possible to reach this but I'm not 100% sure - BoxyUwU
tcx.sess.delay_span_bug(
tcx.def_span(def_id),
"unexpected non-GAT usage of an anon const",
);
return None;
}
Node::Expr(&Expr {
kind:
ExprKind::MethodCall(segment, ..) | ExprKind::Path(QPath::TypeRelative(_, segment)),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_typeck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ This API is completely unstable and subject to change.
*/

#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
#![feature(bindings_after_at)]
#![feature(bool_to_option)]
#![feature(box_syntax)]
#![feature(crate_visibility_modifier)]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// run-pass
#![feature(generic_associated_types)]
#![allow(incomplete_features)]

// This test unsures that with_opt_const_param returns the
// def_id of the N param in the Foo::Assoc GAT.

trait Foo {
type Assoc<const N: usize>;
fn foo(&self) -> Self::Assoc<3>;
}

impl Foo for () {
type Assoc<const N: usize> = [(); N];
fn foo(&self) -> Self::Assoc<3> {
[(); 3]
}
}

fn main() {
assert_eq!(().foo(), [(); 3]);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// run-pass
#![feature(generic_associated_types)]
#![allow(incomplete_features)]

// This test unsures that with_opt_const_param returns the
// def_id of the N param in the Foo::Assoc GAT.

trait Foo {
type Assoc<const N: usize>;
fn foo<const N: usize>(&self) -> Self::Assoc<N>;
}

impl Foo for () {
type Assoc<const N: usize> = [(); N];
fn foo<const N: usize>(&self) -> Self::Assoc<N> {
[(); N]
}
}

fn main() {
assert_eq!(().foo::<10>(), [(); 10]);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// run-pass
#![feature(generic_associated_types)]
#![allow(incomplete_features)]

// This test unsures that with_opt_const_param returns the
// def_id of the N param in the Bar::Assoc GAT.

trait Bar {
type Assoc<const N: usize>;
}
trait Foo: Bar {
fn foo(&self) -> Self::Assoc<3>;
}

impl Bar for () {
type Assoc<const N: usize> = [(); N];
}

impl Foo for () {
fn foo(&self) -> Self::Assoc<3> {
[(); 3]
}
}

fn main() {
assert_eq!(().foo(), [(); 3]);
}