-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
arbitrary_self_types: Split the Autoderef chain #146095
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
This PR changes a file inside |
This comment has been minimized.
This comment has been minimized.
bef5ba5 to
f3e8785
Compare
This comment has been minimized.
This comment has been minimized.
f3e8785 to
31788b7
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@rustbot label +F-arbitrary_self_types |
31788b7 to
1a2dbf8
Compare
|
The Miri subtree was changed cc @rust-lang/miri Some changes occurred in compiler/rustc_codegen_gcc |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Is this waiting on discussion and a decision from the lang team? |
|
@jackh726 Sorry I was not very clear. Indeed it would need a lang team discussion. To support the discussion, I am writing a report to describe what changes this would bring to the feature. |
|
☔ The latest upstream changes (presumably #145993) made this pull request unmergeable. Please resolve the merge conflicts. |
|
The report is a bit overdue. I would like to just publish a partial text while I need more time to restore some diagnostics. Acceptable receiversLanding #146095 means that types implementing
Library changesThis patch demonstrates that we now explicitly mark types like This is particularly important for ShadowingAfter careful consideration and testing, I believe that splitting the two traits do not change the fundamental reasoning behind method shadowing diagnostics. Before this change, concerns about shadowing items through |
32433c2 to
2553a45
Compare
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #144420) made this pull request unmergeable. Please resolve the merge conflicts. |
2553a45 to
2304ec9
Compare
This comment has been minimized.
This comment has been minimized.
2304ec9 to
b980142
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
b980142 to
3ea0cdf
Compare
|
Quick update:
|
|
@bors2 try |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
arbitrary_self_types: Split the Autoderef chain
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (7757954): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -2.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 476.46s -> 476.713s (0.05%) |
|
The results seem neutral to me. Do we need deeper look into the performance? |
|
Perf. seems like a wash, so I don't think so. |
Receiver chain has been an extension of Deref chain, but the consequence has been questioned as this would admit method signatures that are deemed dubious. It is also debatable that Receiver trait which determines applicable `Self` type should have anything to do with dereference operation in general. This patch separate the two traits and isolate their use in the language. This means that in method probing, we will chase down a Deref chain for an applicable type for the variable `self`, from which we additionally chase down a Receiver chain for an applicable `Self` type from a list of methods that accepts one of these possible types of `self`. This patch includes rewording of diagnostics.
To allow use of Receiver with Target diverging from Deref::Target, one must now enable an unstable feature flag arbitrary-self-types-split-chains. Signed-off-by: Xiangfei Ding <[email protected]>
3ea0cdf to
63a3a7d
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Previously we have been picking methods for receiver types even though we are inspecting other receiver types that are several levels of dereferences, which is wasted effort because there will never be matches with these methods. Signed-off-by: Xiangfei Ding <[email protected]>
63a3a7d to
02f464d
Compare
|
@rustbot label: +perf-regression-triaged |
Receiver chain has been an extension of Deref chain, but the consequence has been questioned as this would admit method signatures that are deemed dubious.
It is also debatable that Receiver trait which determines applicable
Selftype should have anything to do with dereference operation in general.This patch separate the two traits and isolate their use in the language. This means that in method probing, we will chase down a Deref chain for an applicable type for the variable
self, from which we additionally chase down a Receiver chain for an applicableSelftype from a list of methods that accepts one of these possible types ofself.To facilitate discussion, we can use the Zulip thread #t-lang > Consequences of making Deref a subtrait of Receiver.
Pending items
arbitrary_self_typesgate again.