On method chain expression failure, look for missing method in earlier segments of the chain #115229

iSwapna · 2023-08-25T22:20:46Z

This PR tries to fix the issue: #115222

As suggested by @estebank , I did the following:

Add new test tests/ui/structs/method-chain-expression-failure.rs
In compiler/rusct_hir_tycheck/src/method/suggest.rs
walking up the method chain and calling probe_for_name with the method name. But the call fails to return Ok.

rustbot · 2023-08-25T22:20:53Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @wesleywiser (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

compiler/rustc_hir_typeck/src/method/suggest.rs

oli-obk · 2023-09-11T07:31:14Z

r? @oli-obk

Please include some high level information about this PR in the main message (e.g. link to the issue and state what new message you are adding to which existing message)

oli-obk · 2023-09-11T07:32:20Z

it looks like there are tests other than yours that are affected by this change. If you bless all ui tests, you should see some changes in your git status view

compiler/rustc_hir_typeck/src/method/suggest.rs

iSwapna · 2023-09-11T15:59:48Z

@rustbot review

tests/ui/typeck/issue-31173.stderr

compiler/rustc_hir_typeck/src/method/suggest.rs

oli-obk · 2023-09-13T15:26:01Z

r? @estebank

iSwapna · 2023-09-16T06:47:45Z

@estebank Contents of tests/ui/structs/method-chain-expression-failure.stdout:

Applicable candidates: [(Candidate { xform_self_ty: &A, xform_ret_ty: Some(B), item: AssocItem { def_id: DefId(0:12 ~ method_chain_expression_failure[2e39]::{impl#0}::b), name:\
 "b", kind: Fn, container: ImplContainer, trait_item_def_id: None, fn_has_self_parameter: true, opt_rpitit_info: None }, kind: InherentImplCandidate([], []), import_ids: [] }, \
Match)]
Applicable candidates: [(Candidate { xform_self_ty: &B, xform_ret_ty: Some(C), item: AssocItem { def_id: DefId(0:14 ~ method_chain_expression_failure[2e39]::{impl#1}::c), name:\
 "c", kind: Fn, container: ImplContainer, trait_item_def_id: None, fn_has_self_parameter: true, opt_rpitit_info: None }, kind: InherentImplCandidate([], []), import_ids: [] }, \
Match)]
Applicable candidates: [(Candidate { xform_self_ty: &C, xform_ret_ty: Some(D), item: AssocItem { def_id: DefId(0:17 ~ method_chain_expression_failure[2e39]::{impl#2}::d), name:\
 "d", kind: Fn, container: ImplContainer, trait_item_def_id: None, fn_has_self_parameter: true, opt_rpitit_info: None }, kind: InherentImplCandidate([], []), import_ids: [] }, \
Match)]
prev_ty: &C
item_name: foo
No Match
prev_ty: &B
item_name: foo
self_ty=&B probe=Candidate { xform_self_ty: &B, xform_ret_ty: Some(()), item: AssocItem { def_id: DefId(0:15 ~ method_chain_expression_failure[2e39]::{impl#1}::foo), name: "foo\
", kind: Fn, container: ImplContainer, trait_item_def_id: None, fn_has_self_parameter: true, opt_rpitit_info: None }, kind: InherentImplCandidate([], []), import_ids: [] }
1705 :: Consider_probe: comparing return_ty &B with xform ret ty ()
Error Type is Err(Sorts(ExpectedFound { expected: &B, found: () }))
Applicable candidates: [(Candidate { xform_self_ty: &B, xform_ret_ty: Some(()), item: AssocItem { def_id: DefId(0:15 ~ method_chain_expression_failure[2e39]::{impl#1}::foo), na\
me: "foo", kind: Fn, container: ImplContainer, trait_item_def_id: None, fn_has_self_parameter: true, opt_rpitit_info: None }, kind: InherentImplCandidate([], []), import_ids: [\
] }, BadReturnType)]
BadReturnType
prev_ty: &A
item_name: foo
No Match

iSwapna · 2023-09-16T06:51:41Z

@estebank Contents of tests/ui/structs/method-chain-expression-failure.stdout:

Applicable candidates: [(Candidate { xform_self_ty: &A, xform_ret_ty: Some(B), item: AssocItem { def_id: DefId(0:12 ~ method_chain_expression_failure[2e39]::{impl#0}::b), name:\
 "b", kind: Fn, container: ImplContainer, trait_item_def_id: None, fn_has_self_parameter: true, opt_rpitit_info: None }, kind: InherentImplCandidate([], []), import_ids: [] }, \
Match)]
Applicable candidates: [(Candidate { xform_self_ty: &B, xform_ret_ty: Some(C), item: AssocItem { def_id: DefId(0:14 ~ method_chain_expression_failure[2e39]::{impl#1}::c), name:\
 "c", kind: Fn, container: ImplContainer, trait_item_def_id: None, fn_has_self_parameter: true, opt_rpitit_info: None }, kind: InherentImplCandidate([], []), import_ids: [] }, \
Match)]
Applicable candidates: [(Candidate { xform_self_ty: &C, xform_ret_ty: Some(D), item: AssocItem { def_id: DefId(0:17 ~ method_chain_expression_failure[2e39]::{impl#2}::d), name:\
 "d", kind: Fn, container: ImplContainer, trait_item_def_id: None, fn_has_self_parameter: true, opt_rpitit_info: None }, kind: InherentImplCandidate([], []), import_ids: [] }, \
Match)]
prev_ty: &C
item_name: foo
No Match
prev_ty: &B
item_name: foo
self_ty=&B probe=Candidate { xform_self_ty: &B, xform_ret_ty: Some(()), item: AssocItem { def_id: DefId(0:15 ~ method_chain_expression_failure[2e39]::{impl#1}::foo), name: "foo\
", kind: Fn, container: ImplContainer, trait_item_def_id: None, fn_has_self_parameter: true, opt_rpitit_info: None }, kind: InherentImplCandidate([], []), import_ids: [] }
1705 :: Consider_probe: comparing return_ty &B with xform ret ty ()
Error Type is Err(Sorts(ExpectedFound { expected: &B, found: () }))
Applicable candidates: [(Candidate { xform_self_ty: &B, xform_ret_ty: Some(()), item: AssocItem { def_id: DefId(0:15 ~ method_chain_expression_failure[2e39]::{impl#1}::foo), na\
me: "foo", kind: Fn, container: ImplContainer, trait_item_def_id: None, fn_has_self_parameter: true, opt_rpitit_info: None }, kind: InherentImplCandidate([], []), import_ids: [\
] }, BadReturnType)]
BadReturnType
prev_ty: &A
item_name: foo
No Match

In this case, should we check for return type of foo() to match the one implemented by B ? If so, we should first check if there is a variable the return value is getting assigned to? But I think, since this is a hint to the user, just matching by name and returning the first (closest to D) one as a possible match would be enough to point the user in the right direction?

estebank · 2023-09-19T01:00:50Z

But I think, since this is a hint to the user, just matching by name and returning the first (closest to D) one as a possible match would be enough to point the user in the right direction?

If we can filter the method with the right return type, I would prefer to suggest it, otherwise I would mention all of them. Give me a minute to check the rest of the PR and correlate with the logs you have here.

estebank · 2023-09-19T01:09:42Z

Ah! I see what's happening. A.b().c().d().foo(); as written of course fails, but the solution in this case is to write A.b().foo();, but the logic as written is (somewhat incorrectly) trying to figure out if A.b().foo().c().d(); is valid. The former is a case of additional elements in the chain, while the later is a case of misplaced method call. I think we can support both with high accuracy. For the latter the current logic we need to make a change: if you look at return_ty being &B it means that it's looking for a method that will return prev_ty, not just one that has that as Self. I'll point it out inline.

compiler/rustc_hir_typeck/src/method/suggest.rs

estebank · 2023-11-06T18:16:56Z

compiler/rustc_hir_typeck/src/method/suggest.rs

+                    prev_ty,
+                    rcvr_expr.hir_id,
+                    ProbeScope::TraitsInScope,) {
+                        err.span_label(method_span, format!("{item_kind} `{item_name}` is available on `{prev_ty}`"));


If you use parent_expr.span, it would make the error be correct, but when you have more than one element in the chain you would end up with overlapping spans which won't look great.

An option we have is to iterate once using the while let as you have it now, but collect the messages and spans in a vec and emit the diagnostics afterwards by iterating over the vec and peeking at the next span.

estebank · 2023-11-06T18:17:37Z

compiler/rustc_hir_typeck/src/method/suggest.rs

+        // Only if an appropriate error source is not found, check method chain for possible candiates
+        let error_source_not_found = unsatisfied_predicates.is_empty();
+        if error_source_not_found && let Mode::MethodCall = mode && let SelfSource::MethodCall(mut rcvr_expr) = source {
+        while let hir::ExprKind::MethodCall(_path_segment, parent_expr, _args, method_span) =


I would prefer we called parent_expr to something that includes rcvr/receiver in the name, because the parent concept is highly overloaded :)

estebank · 2023-11-06T18:18:37Z

tests/ui/structs/method-chain-expression-failure.stderr

+LL |     A.b().c().d().foo();
+   |           ---     ^^^ method not found in `D`
+   |           |
+   |           method `foo` is available on `&B`


To drive the point home, if you look at the impl the span label should be pointing at the .b() which returns B and not at the method call .c() which returns C.

estebank

I love the results so far! On your next change, please squash your commits as I don't think we need the prior attempts anymore and expect us to land this shortly.

estebank · 2023-11-07T19:49:24Z

You'll need to run x.py test tests/ui --bless again. Let's also squash the remaining commits and I think we can merge the PR after that.

compiler/rustc_hir_typeck/src/method/suggest.rs

iSwapna · 2023-11-10T06:51:13Z

You'll need to run x.py test tests/ui --bless again. Let's also squash the remaining commits and I think we can merge the PR after that.

Had found an issue. Done now.

…s receiver to align spans

estebank · 2023-11-10T21:09:20Z

@bors r+

bors · 2023-11-10T21:09:23Z

📌 Commit 56a109d has been approved by estebank

It is now in the queue for this repository.

bors · 2023-11-10T21:10:32Z

⌛ Testing commit 56a109d with merge edf0b1d...

bors · 2023-11-10T23:06:13Z

☀️ Test successful - checks-actions
Approved by: estebank
Pushing edf0b1d to master...

rust-timer · 2023-11-11T04:29:16Z

Finished benchmarking commit (edf0b1d): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This 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.

	mean	range	count
Regressions ❌ (primary)	0.9%	[0.5%, 1.3%]	4
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.7%	[-1.0%, -0.4%]	8
Improvements ✅ (secondary)	-4.2%	[-4.2%, -4.2%]	1
All ❌✅ (primary)	-0.1%	[-1.0%, 1.3%]	12

Cycles

Results

This 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.

	mean	range	count
Regressions ❌ (primary)	0.5%	[0.4%, 0.5%]	3
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.5%	[0.4%, 0.5%]	3

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 674.395s -> 675.48s (0.16%)
Artifact size: 311.13 MiB -> 311.11 MiB (-0.01%)

Update Rust toolchain from nightly-2023-11-10 to nightly-2023-11-11 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@0f44eb3 up to rust-lang@edf0b1d. The log for this commit range is: rust-lang@edf0b1db0a Auto merge of rust-lang#115229 - iSwapna:issue-115222-fix, r=estebank rust-lang@56a109d15b Recurse over the method chain and maintain a stack to peek at previous receiver to align spans rust-lang@d4c86cfc49 Auto merge of rust-lang#117779 - bjorn3:sync_cg_clif-2023-11-10, r=bjorn3 rust-lang@d186b49460 Merge commit 'c84d1871dc4456539b7b578830268ab3539915d0' into sync_cg_clif-2023-11-10 rust-lang@c84d1871dc Rustup to rustc 1.75.0-nightly (0f44eb3 2023-11-09) rust-lang@6e7961ac5d Sync from rust 0f44eb3 rust-lang@3d0e99d632 Auto merge of rust-lang#117765 - onur-ozkan:fix-117762, r=clubby789 rust-lang@17d0a45f5d Auto merge of rust-lang#117572 - RalfJung:addr_of, r=cuviper rust-lang@e30f8ae867 mention null explicitly rust-lang@0a1e5598b0 Auto merge of rust-lang#117750 - klensy:icu-followup, r=Nilstrieb rust-lang@d42d73b144 Auto merge of rust-lang#117769 - matthiaskrgr:rollup-4efjlg3, r=matthiaskrgr rust-lang@186a3c8c61 Rollup merge of rust-lang#117751 - aDotInTheVoid:unkind, r=GuillaumeGomez rust-lang@7607597d3a Rollup merge of rust-lang#117743 - sjwang05:issue-117720, r=estebank rust-lang@7fd7719ca1 Rollup merge of rust-lang#117741 - eltociear:patch-23, r=compiler-errors rust-lang@0f1da7e682 Rollup merge of rust-lang#117730 - jmillikin:fmt-debug-helper-fns, r=cuviper rust-lang@7096ec3e00 Rollup merge of rust-lang#117039 - scottmcm:clarify-get-unchecked, r=cuviper rust-lang@9dc022dd80 Rollup merge of rust-lang#114191 - rcvalle:rust-exploit-mitigations, r=cuviper rust-lang@82a9f94de5 Closure-consuming helper functions for `fmt::Debug` helpers rust-lang@fdb72795d1 enable unstable feature on `x clean [PATH]` rust-lang@22e1576a12 rustdoc-json: Fix test so it actuall checks things rust-lang@7142c8d83c bump few ICU4X leftover deps rust-lang@5693a34db2 Suggest fix for ; within let-chains rust-lang@b8648216a5 Fix typo in internal.rs rust-lang@7c385f5a03 Update exploit mitigations documentation rust-lang@545175ce87 Fix addition formatting rust-lang@82487a9447 Merge pull request rust-lang#1417 from rust-lang/implement_xgetbv rust-lang@864973135a Implement all vendor intrinsics used by the simd-json crate rust-lang@9f426cef38 Merge pull request rust-lang#1416 from afonso360/aarch64-intrinsics-1 rust-lang@ecf79a304a Implement all vendor intrinsics used by the fimg crate rust-lang@0a35232c85 Implement all vendor intrinsics used by the httparse crate rust-lang@61e38ceea7 Implement all SSE intrinsics used by the jpeg-decoder crate rust-lang@438194980b Implement all avx2 intrinsics used by the image crate rust-lang@6a53acefd8 Implement _mm256_permute2f128_ps and _mm256_permute2f128_pd intrinsics rust-lang@81af5b5031 update and clarify addr_of docs rust-lang@209476e33a Only import aarch64 intrinsics on aarch64 rust-lang@f824da66c6 Make neon example build in all arches rust-lang@70a6abfd29 Add unsigned saturating add/sub intrinsics for aarch64 rust-lang@88c2e7896b Implement aarch64 addp intrinsics rust-lang@1f09bae6a8 Implement min/max neon intrisics rust-lang@8eca01f4b6 Remove support for compiler plugins. rust-lang@f6a8c3afb5 Add real implementation of _xgetbv() rust-lang@909513ef74 Use Value instead of CValue in CInlineAsmOperand rust-lang@ef3703694f Disable a couple of rustc tests which are broken due to a rustc bug rust-lang@c04ceb4342 Fix workaround for the `int $$0x29` issue to not crash on empty inline asm rust-lang@04f1024ecb Rustup to rustc 1.75.0-nightly (75b064d 2023-11-01) rust-lang@361585e06d Sync from rust 75b064d rust-lang@03c9acdd8f Support enum variants in offset_of! rust-lang@48ca2d9703 Implement llvm.fma.v* intrinsics rust-lang@aed0ed2875 Rollup merge of rust-lang#117317 - RalfJung:track-caller, r=oli-obk rust-lang@9a33f82140 Remove inline asm support from the list of limitations rust-lang@51f6ac7bfc Merge branch 'sync_from_rust' rust-lang@41dcb52153 Merge commit 'dde58803fd6cbb270c7a437f36a8a3a29fbef679' into sync_cg_clif-2023-10-29 rust-lang@c6f5090294 share the track_caller handling within a mir::Body rust-lang@bad4be6e29 interpret: call caller_location logic the same way codegen does, and share some code rust-lang@01ca7a0cb0 Add the missing word rust-lang@2c13ee8970 Clarify UB in `get_unchecked(_mut)` rust-lang@40a83be6eb Format exploit mitigations documentation Co-authored-by: celinval <celinval@users.noreply.github.com>

rustbot assigned wesleywiser Aug 25, 2023

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 Aug 25, 2023

This comment has been minimized.

Sign in to view

iSwapna commented Sep 10, 2023

View reviewed changes

compiler/rustc_hir_typeck/src/method/suggest.rs Outdated Show resolved Hide resolved

iSwapna marked this pull request as ready for review September 10, 2023 17:48

This comment has been minimized.

Sign in to view

iSwapna mentioned this pull request Sep 11, 2023

On method chain expression failure, look for missing method in earlier segments of the chain #115222

Closed

rustbot assigned oli-obk and unassigned wesleywiser Sep 11, 2023

This comment has been minimized.

Sign in to view

estebank reviewed Sep 11, 2023

View reviewed changes

compiler/rustc_hir_typeck/src/method/suggest.rs Outdated Show resolved Hide resolved

estebank reviewed Sep 11, 2023

View reviewed changes

compiler/rustc_hir_typeck/src/method/suggest.rs Outdated Show resolved Hide resolved

estebank reviewed Sep 11, 2023

View reviewed changes

tests/ui/typeck/issue-31173.stderr Outdated Show resolved Hide resolved

estebank changed the title ~~Issue 115222 fix~~ On method chain expression failure, look for missing method in earlier segments of the chain Sep 11, 2023

This comment has been minimized.

Sign in to view

iSwapna commented Sep 12, 2023

View reviewed changes

compiler/rustc_hir_typeck/src/method/suggest.rs Outdated Show resolved Hide resolved

rustbot assigned estebank and unassigned oli-obk Sep 13, 2023

estebank reviewed Sep 19, 2023

View reviewed changes

compiler/rustc_hir_typeck/src/method/suggest.rs Outdated Show resolved Hide resolved

estebank reviewed Nov 6, 2023

View reviewed changes

iSwapna force-pushed the issue-115222-fix branch from d7214df to 39c4312 Compare November 7, 2023 07:20

This comment has been minimized.

Sign in to view

estebank reviewed Nov 7, 2023

View reviewed changes

compiler/rustc_hir_typeck/src/method/suggest.rs Show resolved Hide resolved

This comment has been minimized.

Sign in to view

estebank 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 Nov 9, 2023

iSwapna force-pushed the issue-115222-fix branch 2 times, most recently from 763194d to 7441d1e Compare November 10, 2023 06:49

iSwapna requested a review from estebank November 10, 2023 06:53

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 Nov 10, 2023

Recurse over the method chain and maintain a stack to peek at previou…

56a109d

…s receiver to align spans

iSwapna force-pushed the issue-115222-fix branch from 7441d1e to 56a109d Compare November 10, 2023 21:01

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 Nov 10, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 10, 2023

bors merged commit edf0b1d into rust-lang:master Nov 10, 2023
11 checks passed

rustbot added this to the 1.75.0 milestone Nov 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

On method chain expression failure, look for missing method in earlier segments of the chain #115229

On method chain expression failure, look for missing method in earlier segments of the chain #115229

iSwapna commented Aug 25, 2023 •

edited

Loading

rustbot commented Aug 25, 2023

This comment has been minimized.

This comment has been minimized.

oli-obk commented Sep 11, 2023

oli-obk commented Sep 11, 2023

This comment has been minimized.

iSwapna commented Sep 11, 2023

This comment has been minimized.

This comment has been minimized.

oli-obk commented Sep 13, 2023

iSwapna commented Sep 16, 2023 •

edited

Loading

iSwapna commented Sep 16, 2023

estebank commented Sep 19, 2023

estebank commented Sep 19, 2023

estebank Nov 6, 2023

estebank Nov 6, 2023

estebank Nov 6, 2023

estebank left a comment

This comment has been minimized.

estebank commented Nov 7, 2023

This comment has been minimized.

iSwapna commented Nov 10, 2023

estebank commented Nov 10, 2023

bors commented Nov 10, 2023

bors commented Nov 10, 2023

bors commented Nov 10, 2023

rust-timer commented Nov 11, 2023

On method chain expression failure, look for missing method in earlier segments of the chain #115229

On method chain expression failure, look for missing method in earlier segments of the chain #115229

Conversation

iSwapna commented Aug 25, 2023 • edited Loading

rustbot commented Aug 25, 2023

This comment has been minimized.

This comment has been minimized.

oli-obk commented Sep 11, 2023

oli-obk commented Sep 11, 2023

This comment has been minimized.

iSwapna commented Sep 11, 2023

This comment has been minimized.

This comment has been minimized.

oli-obk commented Sep 13, 2023

iSwapna commented Sep 16, 2023 • edited Loading

iSwapna commented Sep 16, 2023

estebank commented Sep 19, 2023

estebank commented Sep 19, 2023

estebank Nov 6, 2023

Choose a reason for hiding this comment

estebank Nov 6, 2023

Choose a reason for hiding this comment

estebank Nov 6, 2023

Choose a reason for hiding this comment

estebank left a comment

Choose a reason for hiding this comment

This comment has been minimized.

estebank commented Nov 7, 2023

This comment has been minimized.

iSwapna commented Nov 10, 2023

estebank commented Nov 10, 2023

bors commented Nov 10, 2023

bors commented Nov 10, 2023

bors commented Nov 10, 2023

rust-timer commented Nov 11, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

iSwapna commented Aug 25, 2023 •

edited

Loading

iSwapna commented Sep 16, 2023 •

edited

Loading