- 
                Notifications
    You must be signed in to change notification settings 
- Fork 13.9k
          Add HashStable_NoContext to simplify HashStable implementations in rustc_type_ir
          #117580
        
          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
              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?
| ) | ||
| } | ||
|  | ||
| 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 derivedHashStableimplementation that has noHashStableContextbound, and which addswherebounds forHashStablebased off of fields and not generics.This means we can
derive(HashStable_NoContext)in more places inrustc_type_irrather than having to hand-roll implementations.