update error E0451 to new format #36054
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jonathandturner (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
Thanks for the PR! @bors r+ rollup |
|
📌 Commit a24ea73 has been approved by |
|
I broke compile-fail/functional-struct-update-respects-privacy.rs, trying to fix it |
| self.check_field(expr.span, adt, field); | ||
| for (field, field_span) in variant.fields.iter() | ||
| .zip(fields.iter().map(|f| f.span)) { | ||
| self.check_field(field_span, adt, field); |
There was a problem hiding this comment.
@mikhail-m1: The problem, I believe, is that fields contains less fields than variant.fields, which means that not all fields are iterated over.
Also I don't think it's guaranteed that the fields in fields and variant.fields occur in the same order - my intuition would be that variant.fields is in the order the fields were defined and fields is in the order the fields were written in the struct literal.
|
librustc_llvm fails |
| } else { | ||
| expr.span | ||
| }; | ||
| self.check_field(span, adt, field); |
There was a problem hiding this comment.
I think this should work now, however I'm a bit concerned that the complexity here went from linear to quadratic (for all struct literals). I'm not sure how much of an impact this will have on real world code however there are crates with structs with 1500+ fields in which case this may not be ideal.
There was a problem hiding this comment.
I don't expect so big structs, but this code works only on matching expressions, does any one match more then tens fields?
There was a problem hiding this comment.
If I understand the situation correctly, this code runs for every struct literal, ie. let _ = Foo { bar: "hi!" }. And initialising such large structs does happen.
There was a problem hiding this comment.
ok, I'll rework it
|
I did some (absolutely non-scientific) performance measurements between local stage 1 builds (with and without this pr) and found no real differences in compile time, so the current implementation is probably fine. |
|
📌 Commit 46fc80c has been approved by |
|
@bors r- I missed the "I broke compile-fail/functional-struct-update-respects-privacy.rs, trying to fix it" comment. Let me know when this gets fixed and I'll put it back through review. |
|
@jonathandturner: That should habe been fixed by the updated commit as well. |
|
@TimNN - Oh okay. With Travis down for the moment, we're having to guess what will pass based on what failed in previous builds. If you think it's ready, I can re:r+ it |
|
@jonathandturner - I'm confident that the previous failure of As such I think this pr is good to go. |
|
📌 Commit 46fc80c has been approved by |
update error E0451 to new format Fixes rust-lang#35928 as part of rust-lang#35233. r? @jonathandturner
update error E0451 to new format Fixes rust-lang#35928 as part of rust-lang#35233. r? @jonathandturner
Fixes #35928 as part of #35233.
r? @jonathandturner