Avoid committing to autoderef in object method probing#57885
Avoid committing to autoderef in object method probing#57885bors merged 3 commits intorust-lang:masterfrom
Conversation
|
@arielb1 I've been wondering about trying to move all of method selection to a canonical query. Have you thought about that at all? |
|
I'll rather talk a bit about it on Zulip. |
|
So I think the simpler approach to things I have found is actually sound, and I added a test for a few of the edge-cases that should pass (it passes today, and should continue passing). I'll rewrite this PR when I have the time for that. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@arielb1 is the "simpler approach" reflected in this PR now, or is that a new thing that you have yet to push? |
These aren't fixed by this PR, but were broken in a few older attempts at it. Make sure they don't regress.
|
PR rewritten to use new and much-simpler approach. That's not what I had in mind at first, but it's even better. |
|
@arielb1 in your earlier version of this PR, you said you didn't want to beta-nominate the first version you posted. From skimming this (simpler) version, it seems like a reasonable thing to consider backporting this to beta. Do you still dislike the idea of backporting this? Or do you agree it would be worthwhile to backport this? |
|
I'm not quite sure. This looks simple enough, but it's subtle enough I'm not sure I want it to land on beta. |
|
I'm actually ok with both options - the bug this PR fixes is quite a corner case. |
|
beta-nominating to ensure we discuss at weekly meeting (either today or at some point after this patch lands.) |
|
So @nikomatsakis told me I should r+ this myself. @bors r=nikomatsakis |
|
📌 Commit 927d01f has been approved by |
Avoid committing to autoderef in object method probing This fixes the "leak" introduced in #57835 (see test for details, also apparently #54252 had no tests for the "leaks" that were fixed in it, so go ahead and add one). Maybe beta-nominating because regression, but I'm against landing things on beta we don't have to. r? @nikomatsakis
|
☀️ Test successful - checks-travis, status-appveyor |
[beta] Rollup backports Cherry-picked: * #58207: Make `intern_lazy_const` actually intern its argument. * #58161: Lower constant patterns with ascribed types. * #57908: resolve: Fix span arithmetics in the import conflict error * #57835: typeck: remove leaky nested probe during trait object method resolution * #57885: Avoid committing to autoderef in object method probing * #57646: Fixes text becoming invisible when element targetted Rolled up: * #58522: [BETA] Update cargo r? @ghost
[beta] Rollup backports Cherry-picked: * #58207: Make `intern_lazy_const` actually intern its argument. * #58161: Lower constant patterns with ascribed types. * #57908: resolve: Fix span arithmetics in the import conflict error * #57835: typeck: remove leaky nested probe during trait object method resolution * #57885: Avoid committing to autoderef in object method probing * #57646: Fixes text becoming invisible when element targetted Rolled up: * #58522: [BETA] Update cargo r? @ghost
This fixes the "leak" introduced in #57835 (see test for details, also apparently #54252 had no tests for the "leaks" that were fixed in it, so go ahead and add one).
Maybe beta-nominating because regression, but I'm against landing things on beta we don't have to.
r? @nikomatsakis