Properly deal with GATs when looking for method chains to point at #121912
Conversation
rustbot
added
S-waiting-on-review
| | ^ `Base::Base` is `<T as Base>::Base<_>` here | ||
| ... | ||
| LL | input.fmap(f1).fmap(f2) | ||
| | -------- `Base::Base` remains `_` here |
There was a problem hiding this comment.
We can likely just check for _ and avoid the label then, as a quick change.
There was a problem hiding this comment.
I can either change the select_where_possible a few lines below my changes to select_all_or_error which would make this note go away or instead add an is_ty_var check.
There was a problem hiding this comment.
Can you check if select_all_or_error will result in any currently "good" note going away too?
There was a problem hiding this comment.
I'll make CI do the dirty work...
There was a problem hiding this comment.
I can either change the
select_where_possiblea few lines below my changes toselect_all_or_errorwhich would make this note go away or instead add anis_ty_varcheck.
Went with is_ty_var since select_all_or_error turned out to be too strict.
| --> $DIR/method-chain-gats.rs:13:29 | ||
| | | ||
| LL | fn fmap2<T, A, B, C>(input: T, f1: impl Fn(A) -> B, f2: impl Fn(B) -> C) -> T::Base<C> | ||
| | ^ `Base::Base` is `<T as Base>::Base<_>` here |
There was a problem hiding this comment.
wrong location I suppose?
There was a problem hiding this comment.
I think the location is "right", because the logic is looking for the root element of the method chain (even accounting for let bindings and fn arguments), so this is trying to say that <Base>::Base<_> = <T as Base>::Base<_>, which is somewhat useless information.
I do have to say that the test code makes my head spin, even without accounting for the error message 😬
There was a problem hiding this comment.
I think the only thing left to do is to improve this label to better account for GATs, but that shouldn't happen in this PR.
fmease
added
S-waiting-on-author
fmease
force-pushed
the
diag-method-chains-gat
branch
from
819d395 to
5b43cfa
Compare
This comment has been minimized.
This comment has been minimized.
fmease
force-pushed
the
diag-method-chains-gat
branch
from
5b43cfa to
6035e87
Compare
fmease
added
S-waiting-on-review
|
@bors r=compiler-errors,estebank |
bors
added
S-waiting-on-bors
…piler-errors,estebank Properly deal with GATs when looking for method chains to point at Fixes rust-lang#121898. ~~While it prevents an ICE and the structured suggestion is correct, the method chain diagnostic notes are weird / useless / incorrect judging by a quick look. I guess I should improve that in this PR.~~ Sufficiently taken care of. r? estebank or compiler-errors (rust-lang#105332, rust-lang#105674).
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
Network failure |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 3 pull requests Successful merges: - rust-lang#121130 (Suggest moving definition if non-found macro_rules! is defined later) - rust-lang#121912 (Properly deal with GATs when looking for method chains to point at) - rust-lang#121927 (Add a proper `with_no_queries` to printing) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#121912 - fmease:diag-method-chains-gat, r=compiler-errors,estebank Properly deal with GATs when looking for method chains to point at Fixes rust-lang#121898. ~~While it prevents an ICE and the structured suggestion is correct, the method chain diagnostic notes are weird / useless / incorrect judging by a quick look. I guess I should improve that in this PR.~~ Sufficiently taken care of. r? estebank or compiler-errors (rust-lang#105332, rust-lang#105674).
6 participants
Fixes #121898.
While it prevents an ICE and the structured suggestion is correct, the method chain diagnostic notes are weird / useless / incorrect judging by a quick look. I guess I should improve that in this PR.Sufficiently taken care of.r? estebank or compiler-errors (#105332, #105674).