More suggestion for converting `Option<&Vec<T>>` to `Option<&[T]>` #127596

tesuji · 2024-07-11T05:24:17Z

Please review commit-by-commit.

rustbot · 2024-07-11T05:24:24Z

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

compiler-errors

This PR implements two basically unrelated suggestions. Could you please split it into two PRs?

My opinion about these two suggestions are split. I think the first suggestion is fine and we can land it (8acf2e4), but the second (7e719c9) seems somewhat invasive and also really specific, to the point that I don't think we should land it. Putting a new suggestion right into the middle of the argument checking logic seems like the wrong thing to do IMO, and I don't know if there's an easier way to do it.

compiler-errors · 2024-07-11T18:52:16Z

@rustbot author

tesuji · 2024-07-11T22:13:23Z

Alright, I'll split this PR into 2 parts.

About:

the second (7e719c9) seems somewhat invasive and also really specific, to the point that I don't think we should land it.

I mean people prefer using Option::unwrap_or over manually matching seem to be benefit from this suggestion.

Putting a new suggestion right into the middle of the argument checking logic seems like the wrong thing to do IMO

I don't understand what it means. That change add a new suggestion in fn report_arg_errors

rust/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

Line 482 in bcf1f6d

fn report_arg_errors(

and it is laid after suggest_ptr_null_mut, which is also a suggestion:

rust/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

Lines 944 to 949 in bcf1f6d

    
           self.suggest_ptr_null_mut( 
        
               expected_ty, 
        
               provided_ty, 
        
               provided_args[*provided_idx], 
        
               &mut err, 
        
           );

@rustbot ready
r? compiler-errors

tesuji · 2024-07-11T22:25:00Z

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

-                });
-                return true;
-            } else if let ty::Adt(adt, _) = found_ty_inner.peel_refs().kind()
+            } else if let ty::Ref(_, peeled_found_ty, _) = found_ty_inner.kind()


This change is necessary. Because suggesting Option<String>.map(|v| v.as_str()) violates ownership rules.

compiler-errors · 2024-07-12T00:02:19Z

@bors r+ rollup

bors · 2024-07-12T00:02:22Z

📌 Commit a776e5f has been approved by compiler-errors

It is now in the queue for this repository.

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#127164 (match lowering: Clarify the main loop of the algorithm) - rust-lang#127422 (as_simd: fix doc comment to be in line with align_to) - rust-lang#127596 (More suggestion for converting `Option<&Vec<T>>` to `Option<&[T]>`) - rust-lang#127607 (compiletest: Better error message for bad `normalize-*` headers) - rust-lang#127622 (Mark `builtin_syntax` as internal) - rust-lang#127625 (Revert accidental comment deletion) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#127596 - tesuji:help-unwrap-or, r=compiler-errors More suggestion for converting `Option<&Vec<T>>` to `Option<&[T]>` Please review commit-by-commit.

…r, r=estebank Suggest using `map_or` when `Option<&T>::unwrap_or where T: Deref` fails Fix rust-lang#127545 Split from rust-lang#127596 (review)

… r=estebank Suggest using `map_or` when `Option<&T>::unwrap_or where T: Deref` fails Fix rust-lang#127545 Split from rust-lang#127596 (review)

Suggest using `map_or` when `Option<&T>::unwrap_or where T: Deref` fails Fix #127545 Split from rust-lang/rust#127596 (review)

Update Rust toolchain from nightly-2024-07-12 to nightly-2024-07-13 without any other source changes. This is an automatically generated pull request. If any of the CI checks fail, manual intervention is required. In such a case, review the changes at https://github.com/rust-lang/rust from rust-lang@5315cbe up to rust-lang@c6727fc. The log for this commit range is: rust-lang@c6727fc9b5 Auto merge of rust-lang#123351 - beetrees:x86-ret-snan-rust, r=nikic,workingjubilee rust-lang@62c068feea Auto merge of rust-lang#127636 - nnethercote:fix-Parser-look_ahead, r=oli-obk rust-lang@5d76a13bbe Auto merge of rust-lang#127653 - matthiaskrgr:rollup-72bqgvp, r=matthiaskrgr rust-lang@f11c2c8e95 Rollup merge of rust-lang#127648 - Kobzol:ci-lower-timeout, r=pietroalbini rust-lang@526da2366a Rollup merge of rust-lang#127627 - lcnr:rustc_search_graph, r=compiler-errors rust-lang@f5fa6fb602 Rollup merge of rust-lang#127613 - nikic:riscv-update, r=cuviper rust-lang@b4f002d2e5 Rollup merge of rust-lang#127552 - onur-ozkan:unnecessary-git-usage, r=Kobzol rust-lang@8ceb4e49ff Rollup merge of rust-lang#127433 - dtolnay:conststrlen, r=workingjubilee rust-lang@f9b3e8b387 Rollup merge of rust-lang#126827 - the8472:pidfd-spawn, r=workingjubilee rust-lang@18152d72a4 Rollup merge of rust-lang#126639 - sayantn:amx, r=Amanieu rust-lang@65ea92d4a1 Rollup merge of rust-lang#124980 - zachs18:rc-allocator, r=Amanieu rust-lang@05eac57ef6 Auto merge of rust-lang#127479 - Urgau:rustc-stable-hash, r=michaelwoerister rust-lang@15f770b143 enable fuzzing of `SearchGraph` rust-lang@cae9d480bf Adjust tests for x86 "Rust" ABI changes rust-lang@3f4b9dd463 Lower timeout of CI jobs to 4 hours rust-lang@7f1518bddd Add instability attribute on private const_strlen function rust-lang@b286722878 Auto merge of rust-lang#127635 - matthiaskrgr:rollup-foopajr, r=matthiaskrgr rust-lang@100f3fd133 Add a new special case to `Parser::look_ahead`. rust-lang@ebe1305b1e Remove the bogus special case from `Parser::look_ahead`. rust-lang@dad95578b0 Add unit tests for `Parser::look_ahead`. rust-lang@ec05c4ea3f Add the feature gate and target-features rust-lang@c2b7842555 Rollup merge of rust-lang#127625 - SkiFire13:revert-comment-deletion, r=workingjubilee rust-lang@ca576eae4e Rollup merge of rust-lang#127622 - compiler-errors:builtin-internal, r=lqd rust-lang@fe564c10ab Rollup merge of rust-lang#127607 - Zalathar:normalize-hint, r=wesleywiser rust-lang@83d1a1b252 Rollup merge of rust-lang#127596 - tesuji:help-unwrap-or, r=compiler-errors rust-lang@1e7ad4c3ed Rollup merge of rust-lang#127422 - greaka:master, r=workingjubilee rust-lang@58fe37f2c3 Rollup merge of rust-lang#127164 - Nadrieril:clean-lowering-loop, r=matthewjasper rust-lang@4a31a6c32a Auto merge of rust-lang#127382 - estebank:const-let, r=compiler-errors rust-lang@5e311f933d Auto merge of rust-lang#127614 - matthiaskrgr:rollup-8geziwi, r=matthiaskrgr rust-lang@a776e5f922 Add doc for deconstruct_option_or_result rust-lang@872d7b82e1 Add suggestion for `Option<&Vec<T>> -> Option<&[T]` rust-lang@d9170dc666 Add regression test for issue 127545 rust-lang@4df75140dd Fix aarch64 test rust-lang@cbe75486f7 Account for `let foo = expr`; to suggest `const foo: Ty = expr;` rust-lang@b56dc8ee90 Use verbose style when suggesting changing `const` with `let` rust-lang@d9021791eb Revert accidental comment deletion rust-lang@b77d3ef7c4 Mark builtin syntax as internal rust-lang@fa3ce50f0b Rollup merge of rust-lang#127605 - nikic:remove-extern-wasm, r=oli-obk rust-lang@d433f176ef Rollup merge of rust-lang#127601 - trevyn:issue-127600, r=compiler-errors rust-lang@47ab86653e Rollup merge of rust-lang#127599 - tgross35:lazy_cell_consume-rename, r=workingjubilee rust-lang@a10b4d1463 Rollup merge of rust-lang#127598 - weiznich:diagnostic_do_not_recommend_also_skips_help, r=compiler-errors rust-lang@73c500b3a7 Rollup merge of rust-lang#127591 - compiler-errors:label-after-primary, r=lcnr rust-lang@380c78741e Rollup merge of rust-lang#127588 - uweigand:s390x-f16-doctests, r=tgross35 rust-lang@6fd955549a Rollup merge of rust-lang#127572 - tbu-:pr_debug_event_nonpacked, r=jhpratt rust-lang@8de487fdbd Rollup merge of rust-lang#124599 - estebank:issue-41708, r=wesleywiser rust-lang@55256c5a18 Update dist-riscv64-linux to binutils 2.40 rust-lang@977439d9b8 Use uplifted `rustc-stable-hash` crate in `rustc_data_structures` rust-lang@f56b2074c6 solve -> solve/mod rust-lang@08a2992d6b compiletest: Better error message for bad `normalize-*` headers rust-lang@8a50bcbdce Remove extern "wasm" ABI rust-lang@a01f49e7f3 check is_ident before parse_ident rust-lang@ab56fe2053 Rename `lazy_cell_consume` to `lazy_cell_into_inner` rust-lang@27d5db166e Allows `#[diagnostic::do_not_recommend]` to supress trait impls in suggestions as well rust-lang@12ae282987 Fix diagnostic and add a test for it rust-lang@df72e478b0 Make sure that labels are defined after the primary span in diagnostics rust-lang@0065763950 core: Limit remaining f16 doctests to x86_64 linux rust-lang@45ad522e87 Don't mark `DEBUG_EVENT` struct as `repr(packed)` rust-lang@0134bd2e67 remove unnecessary `git` usages rust-lang@42772e98e0 Address review comments rust-lang@3e030b38ef Return the `otherwise_block` instead of passing it as argument rust-lang@fc40247c6b Factor out the "process remaining candidates" cases rust-lang@8a222ffd6b Don't try to save an extra block rust-lang@c5062f7318 Move or-pattern expansion inside the main part of the algorithm rust-lang@bff4d213fa Factor out the special handling of or-patterns rust-lang@5bf50e66f9 Move a function rust-lang@53d3e6217b Stabilize const_cstr_from_ptr (CStr::from_ptr, CStr::count_bytes) rust-lang@585ca16e0b as_simd: fix comment to be in line with 507583a (rust-lang#121201) rust-lang@0f643c449a Ensure tests don't fail on i586 in CI rust-lang@ec0c755704 Check that we get somewhat sane PIDs when spawning with pidfds rust-lang@3e4e31b7bf more fine-grained feature-detection for pidfd spawning rust-lang@0ce361938e document safety properties of the internal Process::new constructor rust-lang@6687a3f7da use pidfd_spawn for faster process creation when pidfds are requested rust-lang@5c46acac04 document the cvt methods rust-lang@0e1c832dbd Update `platform-support.md` to reflect improvements in returning floats on 32-bit x86 rust-lang@952becc0bd Ensure floats are returned losslessly by the Rust ABI on 32-bit x86 rust-lang@a1ad6346d6 Add fn allocator method to rc/sync::Weak. Relax Rc<T>/Arc<T>::allocator to allow unsized T. rust-lang@2df4f7dd8c Suggest borrowing on fn argument that is `impl AsRef` Co-authored-by: celinval <35149715+celinval@users.noreply.github.com>

rustbot assigned petrochenkov Jul 11, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 11, 2024

This comment has been minimized.

Sign in to view

tesuji force-pushed the help-unwrap-or branch from b3964c0 to 7e719c9 Compare July 11, 2024 05:35

This comment has been minimized.

Sign in to view

compiler-errors requested changes Jul 11, 2024

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 11, 2024

tesuji force-pushed the help-unwrap-or branch from 1f36334 to a40ade5 Compare July 11, 2024 22:21

tesuji changed the title ~~More suggestion for transforming Option<&Vec<T>> to Option<&[T]> or &[T]~~ More suggestion for converting Option<&Vec<T>> to Option<&[T]> Jul 11, 2024

tesuji commented Jul 11, 2024

View reviewed changes

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 11, 2024

rustbot assigned compiler-errors and unassigned petrochenkov Jul 11, 2024

tesuji mentioned this pull request Jul 11, 2024

Suggest using map_or when Option<&T>::unwrap_or where T: Deref fails #127629

Merged

This comment has been minimized.

Sign in to view

tesuji added 3 commits July 11, 2024 22:50

Add regression test for issue 127545

d9170dc

Add suggestion for Option<&Vec<T>> -> Option<&[T]

872d7b8

Add doc for deconstruct_option_or_result

a776e5f

tesuji force-pushed the help-unwrap-or branch from a40ade5 to a776e5f Compare July 11, 2024 22:50

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 12, 2024

matthiaskrgr mentioned this pull request Jul 12, 2024

Rollup of 6 pull requests #127635

Merged

bors merged commit 83d1a1b into rust-lang:master Jul 12, 2024
6 checks passed

rustbot added this to the 1.81.0 milestone Jul 12, 2024

tesuji deleted the help-unwrap-or branch July 12, 2024 06:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More suggestion for converting `Option<&Vec<T>>` to `Option<&[T]>` #127596

More suggestion for converting `Option<&Vec<T>>` to `Option<&[T]>` #127596

tesuji commented Jul 11, 2024 •

edited

Loading

rustbot commented Jul 11, 2024

This comment has been minimized.

This comment has been minimized.

compiler-errors left a comment

compiler-errors commented Jul 11, 2024

tesuji commented Jul 11, 2024 •

edited

Loading

tesuji Jul 11, 2024 •

edited

Loading

This comment has been minimized.

compiler-errors commented Jul 12, 2024

bors commented Jul 12, 2024

More suggestion for converting Option<&Vec<T>> to Option<&[T]> #127596

More suggestion for converting Option<&Vec<T>> to Option<&[T]> #127596

Conversation

tesuji commented Jul 11, 2024 • edited Loading

rustbot commented Jul 11, 2024

This comment has been minimized.

This comment has been minimized.

compiler-errors left a comment

Choose a reason for hiding this comment

compiler-errors commented Jul 11, 2024

tesuji commented Jul 11, 2024 • edited Loading

tesuji Jul 11, 2024 • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

compiler-errors commented Jul 12, 2024

bors commented Jul 12, 2024

More suggestion for converting `Option<&Vec<T>>` to `Option<&[T]>` #127596

More suggestion for converting `Option<&Vec<T>>` to `Option<&[T]>` #127596

tesuji commented Jul 11, 2024 •

edited

Loading

tesuji commented Jul 11, 2024 •

edited

Loading

tesuji Jul 11, 2024 •

edited

Loading