-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
[Step 2] Implement "small substs optimization" for substs of length 1 #59312
Conversation
r? @oli-obk (rust_highfive has picked a reviewer for you, use r? to override) |
} else { | ||
folder.tcx().intern_substs(¶ms) | ||
folder.tcx().intern_substs(¶ms).into() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
intern_substs
should directly return a SubstsRef
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oli-obk
As _intern_substs
comes from slice_interners!, which returns a List
, then should I implement a specific one for SubstsRef
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to just modify that method, too. I don't think it's used elsewhere.
☔ The latest upstream changes (presumably #59382) made this pull request unmergeable. Please resolve the merge conflicts. |
Ping from triage, @csmoe, you have some rebasing + review comments to address. |
5943ac9
to
8a62a32
Compare
641a29a
to
e52b4c7
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Change-Id: Idc024631af3ecbe352fb0d18c9184ab7f2d55a19
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@@ -199,7 +199,7 @@ pub trait Printer<'gcx: 'tcx, 'tcx>: Sized { | |||
} | |||
}).count(); | |||
|
|||
&substs[own_params] | |||
SubstsRef::from_slice(self.tcx(), &substs[own_params]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why does this take TyCtxt
as an argument? Is there a method that will just pass through the reference if it already has the appropriate lifetime?
let generics = tcx.generics_of(def_id); | ||
let parent_len = generics.parent_count; | ||
SplitClosureSubsts { | ||
closure_kind_ty: self.substs.type_at(parent_len), | ||
closure_sig_ty: self.substs.type_at(parent_len + 1), | ||
upvar_kinds: &self.substs[parent_len + 2..], | ||
upvar_kinds: SubstsRef::from_slice(tcx, &self.substs[parent_len + 2..]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here, tcx
isn't really necessary, the lifetimes should already fit
☔ The latest upstream changes (presumably #59008) made this pull request unmergeable. Please resolve the merge conflicts. |
Triage ping @csmoe, there's merge conflicts waiting to be resolved, and also a few review comments |
Triage pinging again @csmoe |
Replace ClosureSubsts with SubstsRef Addresses rust-lang#42340 part 3 rust-lang#59312 might benefit from this clean up. r? @nikomatsakis
Address #58310