Fix non-existent-field ICE for generic fields.#81716
Conversation
Co-authored-by: eddyb <eddyb@lyken.rs>
|
r? @estebank (rust-highfive has picked a reviewer for you, use r? to override) |
| field_path.push(candidate_field.ident.normalize_to_macros_2_0()); | ||
| let field_ty = candidate_field.ty(self.tcx, subst); | ||
| if let Some((nested_fields, _)) = self.get_field_candidates(span, &field_ty) { | ||
| if let Some((nested_fields, subst)) = self.get_field_candidates(span, &field_ty) { |
There was a problem hiding this comment.
Random note about something I noticed, that I don't think is worth touching in this PR: this function used subst instead of substs, and while we do use the name subst, it's for the method (where it means "substitute") - the variables of type Substs ("substitutions") should be called substs.
|
@bors r+ (what are the chances this gets into nightly?) |
|
📌 Commit 68cc12a has been approved by |
|
@eddyb you can do |
|
Right now, p=1 would not do much, looking at the bors queue:
|
|
Since we've already gone several reports of this, I'll just do it myself :) @bors p=1 |
Well, it means that it will definitely get in by tomorrow's nightly :) I think it has about 6 hours to get merged, so might not get in. |
Yeah but I didn't even look at the queue, not know enough about what's going on right now to risk messing with priorities. |
|
bors seems to prioritize within a priority level based on when it was approved, so it shouldn't mess with anything that's already prioritized. But, if you want to you can undo my priority bump, I'm fine either way. |
Rollup of 5 pull requests Successful merges: - rust-lang#80394 (make const_err a future incompat lint) - rust-lang#81532 (Remove incorrect `delay_span_bug`) - rust-lang#81692 (Update clippy) - rust-lang#81715 (Reduce tab formatting assertions to debug only) - rust-lang#81716 (Fix non-existent-field ICE for generic fields.) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
|
Thanks for fixing this and sorry for introducing the bug. I still don't understand how this change fixes the bug, can anybody explain this, please? |
That's okay, everyone introduces them :)
My understanding (though I may be totally off-base) is that previously the code was checking for nested fields but in the |
|
Oh yes, I see. Thanks. |
I mentioned this ICE in a chat and it took about 3 milliseconds before @eddyb found the problem and said this change would fix it. :)
This also changes one the field types in the related test to one that triggered the ICE.
Fixes #81627.
Fixes #81672.
Fixes #81709.
Cc #81480 @b-naber @estebank.