Remove use of unwrap() from save-analysis#69422
Conversation
|
r? @davidtwco (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
r? @Xanewok |
Xanewok
left a comment
Xanewok
left a comment
There was a problem hiding this comment.
This brings it in line with other usages and is safe to do, left some further comments
src/librustc_save_analysis/lib.rs
Outdated
| span, | ||
| ref_id: id_from_def_id(variant.fields[index].did), | ||
| })); | ||
| if let Some(index) = self.tcx.find_field_index(ident, variant) { |
There was a problem hiding this comment.
Could we use map here? Since we indent by a level anyway and to avoid trailing None
src/test/ui/assign-to-method.rs
Outdated
| @@ -1,3 +1,5 @@ | |||
| // compile-flags: -Zsave-analysis | |||
There was a problem hiding this comment.
I think we should have a dedicated regression test in src/test/ui/save-analysis and not rely on this flag in general src/test/ui directory
cc @Centril on test layout
There was a problem hiding this comment.
My general sentiment is that we should not have files at all in src/test/ui, only directories. I have a question though... is the purpose of this test just for save-analysis? If not, then it doesn't seem fit for a save-analysis/ folder, which I would expect to be targeted specifically at testing save-analysis as opposed to some other aspect (and save analysis being incidentally also tested).
There was a problem hiding this comment.
Left comments to clarify and moved assign-to-method.rs to ui/methods.
| @@ -1,3 +1,5 @@ | |||
| // compile-flags: -Zsave-analysis | |||
JohnTitor
force-pushed
the
remove-unwrap
branch
from
2aeaba3 to
5307edc
Compare
|
Friendly ping @Xanewok, addressed your reviews, how about? |
|
Sorry, forgot about this! Thanks =) @bors r+ rollup |
|
📌 Commit 5307edc has been approved by |
|
🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened |
bors
added
S-waiting-on-bors
Remove use of `unwrap()` from save-analysis Fix rust-lang#69409, fix rust-lang#69416
Rollup of 8 pull requests Successful merges: - #69422 (Remove use of `unwrap()` from save-analysis) - #69548 (Turn trailing tokens in `assert!()` into hard errors) - #69561 (Clean up unstable book) - #69599 (check_binding_alt_eq_ty: improve precision wrt. `if let`) - #69641 (Update books) - #69776 (Fix & test leak of some BTreeMap nodes on panic during `into_iter`) - #69805 (resolve: Modernize some naming) - #69810 (test(bindings_after_at): add dynamic drop tests for bindings_after_at) Failed merges: r? @ghost
6 participants
Fix #69409, fix #69416