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.

Sign up for GitHub

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

Jump to bottom

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

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

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1d9ac3c
Fix const generics in GAT
BoxyUwU Feb 9, 2021
2c4337a
Comments :3
BoxyUwU Feb 10, 2021
0422745
Fix comment smol mistakes
BoxyUwU Feb 10, 2021
0ffa2da
comma...
BoxyUwU Feb 10, 2021
7ca96ed
rewrite the comments
BoxyUwU Feb 10, 2021
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
Prev Previous commit
Next Next commit
Fix comment smol mistakes
  • Loading branch information
@BoxyUwU
BoxyUwU committed Feb 10, 2021
commit 042274558563623b7656df0a413bd3ccaf83678d
8 changes: 5 additions & 3 deletions compiler/rustc_typeck/src/collect/type_of.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub(super) fn opt_const_param_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<
// 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>;;
// 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<_>
Expand All @@ -41,7 +41,8 @@ pub(super) fn opt_const_param_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<
// 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
// we can resolve the relative path to an actual associated type.
// For the code example above this item would be the Foo trait.
let item_hir_id = tcx
.hir()
.parent_iter(hir_id)
Expand All @@ -53,7 +54,8 @@ pub(super) fn opt_const_param_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<
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
// 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);
Expand Down