Don't normalize xform_ret_ty during method candidate assembly #86506

b-naber · 2021-06-21T08:01:11Z

Normalizing the return type of a method candidate together with the expected receiver type of the method can lead to valid method candidates being rejected during probing. Specifically in the example of the fixed issue we have a self_ty of the form &A<&[Coef]> whereas the impl_ty of the method would be &A<_>, if we normalize the projection in the return type we unify the inference variable with Cont, which will lead us to reject the candidate in the sup type check in consider_probe. Since we don't actually need the normalized return type during candidate assembly, we postpone the normalization until we consider candidates in consider_probe.

rust-highfive · 2021-06-21T08:01:14Z

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

jackh726 · 2021-06-22T21:21:39Z

So, I haven't looked into this issue or this exact fix, but I'm not sure if we should be moving to "normalize less" instead of "normalize more"

b-naber · 2021-06-22T21:26:07Z

Hm well maybe you should at least look at the PR :)

As I wrote above we do normalize the return type, just not during candidate assembly.

jackh726 · 2021-06-22T21:37:36Z

I mean, I've looked at the PR, but don't know exactly why this particular change fixes things.

Does this issue get fixed by #85499 as-is.

b-naber · 2021-06-22T21:58:59Z

Does this issue get fixed by #85499 as-is.

If that is a question, I don't know. You have a build, can you test whether this issue is fixed by that PR?

jackh726 · 2021-09-09T12:50:48Z

r? @jackh726

bors · 2021-10-02T13:48:38Z

☔ The latest upstream changes (presumably #89405) made this pull request unmergeable. Please resolve the merge conflicts.

jackh726 · 2021-10-05T19:25:02Z

Okay, so finally getting a chance to look at this. Sorry this has sat so long @b-naber

Now that I'm looking at this, I actually understand the problem here. It's a bit weird. Because of the Self: AsSlice<Element = Coef> bound, we end up with a predicate <A<Cont> as AsSlice>::Element = Coef in the param_env when type checking failing. Then, later, we try to normalize <A<_#3t> as AsSlice>::Element. We then use that predicate in the param_env as a candidate, unifying _#3t with Cont and getting a result of Coef.

That also causes our self_ty to gets inferred to be &ReErased A<Cont>. This doesn't match &ReErased A<&'_#1r [Coef]>.

Delaying normalization of the return type does work, but I think that it hides a deeper bug. Ideally, I think the problem is the param_env itself. The clauses here probably shouldn't have Params, but instead either fresh vars or maybe just be canonicalized.

For this, I suppose I'm okay with merging as-is. But with a more explicit comment that this problem stems from the param_env having Params instead of inference/canonical vars. I'd also like an issue filed and a FIXME linking to that.

r=me with that

b-naber · 2021-10-07T21:32:13Z

@jackh726 Thanks for the review. Opened the issue and added a FIXME comment.

Kind of stumped by the CI fail though.

jackh726 · 2021-10-07T21:48:43Z

You checked in submodule changes. That's probably it.

b-naber · 2021-10-07T23:40:34Z

You checked in submodule changes. That's probably it.

Thanks, hope this is fixed now.

jackh726 · 2021-10-08T00:24:21Z

Looks better. Can you edit the FIXME to include issue (#89650)? r=me with that

@bors delegate+

bors · 2021-10-08T00:24:23Z

✌️ @b-naber can now approve this pull request

b-naber · 2021-10-08T09:54:24Z

@bors r=jackh726 rollup

bors · 2021-10-08T09:54:25Z

📌 Commit 3215403 has been approved by jackh726

…, r=jackh726 Don't normalize xform_ret_ty during method candidate assembly Fixes rust-lang#85671 Normalizing the return type of a method candidate together with the expected receiver type of the method can lead to valid method candidates being rejected during probing. Specifically in the example of the fixed issue we have a `self_ty` of the form `&A<&[Coef]>` whereas the `impl_ty` of the method would be `&A<_>`, if we normalize the projection in the return type we unify the inference variable with `Cont`, which will lead us to reject the candidate in the sup type check in `consider_probe`. Since we don't actually need the normalized return type during candidate assembly, we postpone the normalization until we consider candidates in `consider_probe`.

…laumeGomez Rollup of 6 pull requests Successful merges: - rust-lang#86506 (Don't normalize xform_ret_ty during method candidate assembly ) - rust-lang#89538 (Make rustdoc not highlight `->` and `=>` as operators) - rust-lang#89649 (clippy::complexity fixes) - rust-lang#89668 (Cfg hide more conditions for core and alloc) - rust-lang#89669 (Remove special-casing of never primitive in rustdoc-json-types) - rust-lang#89672 (Remove unwrap_or! macro) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned matthewjasper Jun 21, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 21, 2021

b-naber changed the title ~~Don't normalize xform_ret_ty during method probing~~ Don't normalize xform_ret_ty during method candidate assembly Jun 21, 2021

crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 9, 2021

JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 25, 2021

JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 15, 2021

inquisitivecrystal added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 24, 2021

rust-highfive assigned jackh726 and unassigned matthewjasper Sep 9, 2021

JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 27, 2021

jackh726 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 Oct 5, 2021

b-naber force-pushed the gen_trait_impl_inconsistent branch from 4e8bb79 to fcf55a0 Compare October 7, 2021 21:21

This comment has been minimized.

Sign in to view

b-naber mentioned this pull request Oct 7, 2021

Type inference problems during method probing #89650

Open

b-naber force-pushed the gen_trait_impl_inconsistent branch from fcf55a0 to 2d7ed7a Compare October 7, 2021 23:39

b-naber added 2 commits October 8, 2021 10:55

dont normalize return type during candidate assembly in method probing

cbf9153

tests

3215403

b-naber force-pushed the gen_trait_impl_inconsistent branch from 2d7ed7a to 3215403 Compare October 8, 2021 08:56

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 8, 2021

GuillaumeGomez mentioned this pull request Oct 8, 2021

Rollup of 6 pull requests #89671

Closed

GuillaumeGomez mentioned this pull request Oct 8, 2021

Rollup of 6 pull requests #89683

Merged

bors merged commit 3250240 into rust-lang:master Oct 9, 2021

rustbot added this to the 1.57.0 milestone Oct 9, 2021

This was referenced Oct 10, 2021

Where clause Binder(<...>) was applicable to Obligation(predicate=Binder(TraitPredicate(<...>)),depth=1) but now is not #52893

Closed

ICE in librustc::traits::select::confirm_param_candidate #68295

Closed

b-naber deleted the gen_trait_impl_inconsistent branch October 6, 2022 15:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't normalize xform_ret_ty during method candidate assembly #86506

Don't normalize xform_ret_ty during method candidate assembly #86506

b-naber commented Jun 21, 2021 •

edited

rust-highfive commented Jun 21, 2021

jackh726 commented Jun 22, 2021

b-naber commented Jun 22, 2021

jackh726 commented Jun 22, 2021 •

edited

b-naber commented Jun 22, 2021

jackh726 commented Sep 9, 2021

bors commented Oct 2, 2021

jackh726 commented Oct 5, 2021

This comment has been minimized.

b-naber commented Oct 7, 2021 •

edited

jackh726 commented Oct 7, 2021

b-naber commented Oct 7, 2021

jackh726 commented Oct 8, 2021

bors commented Oct 8, 2021

b-naber commented Oct 8, 2021

bors commented Oct 8, 2021

Don't normalize xform_ret_ty during method candidate assembly #86506

Don't normalize xform_ret_ty during method candidate assembly #86506

Conversation

b-naber commented Jun 21, 2021 • edited

rust-highfive commented Jun 21, 2021

jackh726 commented Jun 22, 2021

b-naber commented Jun 22, 2021

jackh726 commented Jun 22, 2021 • edited

b-naber commented Jun 22, 2021

jackh726 commented Sep 9, 2021

bors commented Oct 2, 2021

jackh726 commented Oct 5, 2021

This comment has been minimized.

b-naber commented Oct 7, 2021 • edited

jackh726 commented Oct 7, 2021

b-naber commented Oct 7, 2021

jackh726 commented Oct 8, 2021

bors commented Oct 8, 2021

b-naber commented Oct 8, 2021

bors commented Oct 8, 2021

b-naber commented Jun 21, 2021 •

edited

jackh726 commented Jun 22, 2021 •

edited

b-naber commented Oct 7, 2021 •

edited