-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Add HashStable_NoContext
to simplify HashStable
implementations in rustc_type_ir
#117580
Add HashStable_NoContext
to simplify HashStable
implementations in rustc_type_ir
#117580
Conversation
r? @wesleywiser (rustbot has picked a reviewer for you, use r? to override) |
Should we use this implementation everywhere as |
@cjgillot: I tried it, and it breaks stuff, I think where generating |
☔ The latest upstream changes (presumably #117578) made this pull request unmergeable. Please resolve the merge conflicts. |
b94f779
to
740635f
Compare
r? @jackh726 |
☔ The latest upstream changes (presumably #117876) made this pull request unmergeable. Please resolve the merge conflicts. |
740635f
to
003e1ee
Compare
This comment has been minimized.
This comment has been minimized.
003e1ee
to
0cfdedb
Compare
I'll get to this soon (and the other) |
☔ The latest upstream changes (presumably #117731) made this pull request unmergeable. Please resolve the merge conflicts. |
0cfdedb
to
543d9d3
Compare
let body = s.each(|bi| { | ||
let attrs = parse_attributes(bi.ast()); | ||
if attrs.ignore { | ||
quote! {} | ||
} else if let Some(project) = attrs.project { | ||
quote! { | ||
(&#bi.#project).hash_stable(__hcx, __hasher); | ||
} | ||
} else { | ||
quote! { | ||
#bi.hash_stable(__hcx, __hasher); | ||
} | ||
} | ||
}); | ||
|
||
let discriminant = match s.ast().data { | ||
syn::Data::Enum(_) => quote! { | ||
::std::mem::discriminant(self).hash_stable(__hcx, __hasher); | ||
}, | ||
syn::Data::Struct(_) => quote! {}, | ||
syn::Data::Union(_) => panic!("cannot derive on union"), | ||
}; |
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.
Can these use the functions below? I don't see how they're different?
@@ -64,6 +64,50 @@ pub(crate) fn hash_stable_generic_derive( | |||
) | |||
} | |||
|
|||
pub(crate) fn hash_stable_no_context_derive( |
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.
Can we have an inner function with a bool to indicate whether to add the where clause or not, then just delegate these two derives to that?
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.
Sure
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.
Cool, r=me with that change.
b9eab51
to
efee297
Compare
@bors r=jackh726 Rebased, unified all the HashStable implementations, and made sure it continues to build with/without |
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message
|
☔ The latest upstream changes (presumably #118107) made this pull request unmergeable. Please resolve the merge conflicts. |
efee297
to
c9143ea
Compare
@bors r=jackh726 |
☀️ Test successful - checks-actions |
…, r=jackh726 Uplift `CanonicalVarInfo` and friends into `rustc_type_ir` Depends on rust-lang#117580 and rust-lang#117578 Uplift `CanonicalVarInfo` and friends into `rustc_type_ir` so they can be consumed by an interner-agnostic `Canonicalizer` implementation for the new trait solver ❤️ r? `@ghost`
Finished benchmarking commit (7bd385d): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 674.399s -> 674.8s (0.06%) |
adds
derive(HashStable_NoContext)
which is a derivedHashStable
implementation that has noHashStableContext
bound, and which addswhere
bounds forHashStable
based off of fields and not generics.This means we can
derive(HashStable_NoContext)
in more places inrustc_type_ir
rather than having to hand-roll implementations.