Fix some simple associated const issues. #25065
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
|
This all seems good to me modulo that one error message (which also happens to be pretty frequent). I think we should ensure it's phrased in as comprehensible a way as possible. I'm now thinking:
or, when not doing a method lookup,
|
|
@nikomatsakis Better? |
|
@quantheory much. Sorry for being slow, was on partial vacation last week. @bors r+ |
|
📌 Commit a5e6075 has been approved by |
|
⌛ Testing commit a5e6075 with merge 1eb21ca... |
|
💔 Test failed - auto-linux-64-x-android-t |
|
Perhaps a rebase for this PR is in order? Failures like that generally don't turn out to be false negatives unfortunately. |
|
i tried it locally, even with the rebase that line has to go. |
quantheory
force-pushed
the
fix_associated_const_ambiguity_message
branch
2 times, most recently
from
94c3e8a to
5beaeab
Compare
|
I haven't had the chance to do a full |
|
The version here passed |
|
⌛ Testing commit 5beaeab with merge b5277ba... |
|
⛄ The build was interrupted to prioritize another pull request. |
|
⌛ Testing commit 5beaeab with merge 543ef45... |
|
⛄ The build was interrupted to prioritize another pull request. |
|
⌛ Testing commit 5beaeab with merge 4a5e4cd... |
|
⛄ The build was interrupted to prioritize another pull request. |
|
⌛ Testing commit 5beaeab with merge 65ea478... |
|
⛄ The build was interrupted to prioritize another pull request. |
|
⌛ Testing commit 5beaeab with merge bcc76d4... |
|
⛄ The build was interrupted to prioritize another pull request. |
|
☔ The latest upstream changes (presumably #24619) made this pull request unmergeable. Please resolve the merge conflicts. |
quantheory
force-pushed
the
fix_associated_const_ambiguity_message
branch
from
5beaeab to
29b31d0
Compare
Also change several error messages to refer to "items" rather than "methods", since associated items that require resolution during type checking are not always methods.
quantheory
force-pushed
the
fix_associated_const_ambiguity_message
branch
from
29b31d0 to
b4bbf3a
Compare
|
Rebased, passes |
|
⌛ Testing commit b4bbf3a with merge 1b5b639... |
…age, r=nikomatsakis This fixes #24922 and #25017, and reduces the number of error messages that talk about "methods" when associated constants rather than methods are involved. I will admit that I haven't thought very carefully about the error messages. My goal has been to make more of the messages technically correct in all situations, and to avoid ICEs. But in some cases we could probably talk specifically about "methods" rather than "items".
…omatsakis Closes #25046 (by rejecting the code that causes the ICE) and #24946. I haven't been able to deal with the array size or recursion issues yet for associated consts, though my hope was that the change I made for range match patterns might help with array sizes, too. This PR is pretty much orthogonal to #25065.
This fixes #24922 and #25017, and reduces the number of error messages that talk about "methods" when associated constants rather than methods are involved.
I will admit that I haven't thought very carefully about the error messages. My goal has been to make more of the messages technically correct in all situations, and to avoid ICEs. But in some cases we could probably talk specifically about "methods" rather than "items".