[MCP 723] Rename astconv::AstConv and related items by fmease · Pull Request #120926 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation36 Commits6 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Status: This is awaiting some action (such as code changes or more information) from the author.
Status: PR has a compiler MCP and is waiting for the compiler MCP to complete.
labels
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
Relevant to the rustdoc team, which will review and decide on the PR/issue.
The Rustc Trait System Refactor Initiative (-Znext-solver)
labels
Relevant to the rustdoc team, which will review and decide on the PR/issue.
and removed T-rustdoc
Relevant to the rustdoc team, which will review and decide on the PR/issue.
Status: Awaiting review from the assignee but also interested parties.
The Rustc Trait System Refactor Initiative (-Znext-solver)
labels
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment has been minimized.
This comment was marked as resolved.
Status: PR has a compiler MCP and is waiting for the compiler MCP to complete.
label
[WIP] [waiting on MCP] Rename [WIP] [MCP 723] Rename astconv::AstConv and related itemsastconv::AstConv and related items
This makes it easier to read the trait definition for newcomers: Sorted from least “complex” to most “complex” followed by trivial “plumbing” and grouped by area.
- Move
allow_inferabove all*_infermethods- It's the least complex method of those
- Allows the
*_inferto be placed right next to each other
- Move
probe_ty_param_boundsfurther down right next tolower_assoc_tyandprobe_adt- It's more complex than the
infermethods, it should come “later” - Now all required lowering functions are grouped together
- It's more complex than the
- Move the “plumbing” function
set_tainted_by_errorsfurther down below any actual lowering methods. - Provided method should come last
Some changes occurred in src/tools/clippy
cc @rust-lang/clippy
Some changes occurred to the core trait solver
cc @rust-lang/initiative-trait-system-refactor
triagebot.toml has been modified, there may have been changes to the review queue.
Alrighty, the renaming of items and the updating of all relevant (doc) comments has been completed at last!
Best reviewed commit by commit. Some commits contain a short explainer outlining the motivation for the respective changes if not entirely obvious.
Please consult the following exhaustive & normative list of changes here:
https://fmease.dev/rustc-dev/astconv-no-mo.html (2024-03-22 archive link).
The MCP doesn't contain every single change but it does contain the majority of it.
Feel free to suggest better names for individual items but keep in mind that most of the name changes are well motivated and are part of a cohesive naming scheme. If you do please double-check first if there's an explainer for the name change in question over at the linked web page.
Ping @oli-obk @compiler-errors
[WIP] [MCP 723] Rename [MCP 723] Rename astconv::AstConv and related itemsastconv::AstConv and related items
Status: Awaiting review from the assignee but also interested parties.
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
<3 Thanks for doing this. The html doc is amazing. Still reading the doc, then will give this PR a review
@bors p=10 bitrotty
| /// **A note on binders:** there is an implied binder around |
|---|
| /// `param_ty` and `ast_bounds`. See `instantiate_poly_trait_ref` |
| /// for more details. |
| /// There is an implied binder around `param_ty` and `ast_bounds`. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does "implied binder" mean here? If no binder is specified, you get an empty binder?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It just means that param_ty: Ty<'tcx> may contain late-bound vars even though it's not wrapped in a Binder<'tcx, ·> (rendering them escaping I guess). Neither is ast_bounds/hir_bounds but well that param is of a HIR data type, so obviously it can't be wrapped in a Binder.
Wording is a bit poor, I guess I could've adjusted it.
| @@ -37,6 +37,7 @@ pub use crate::traits::{MethodViolationCode, ObjectSafetyViolation}; |
|---|
| /// Currently that is `Self` in supertraits. This is needed |
| /// because `object_safety_violations` can't be used during |
| /// type collection. |
| #[instrument(level = "debug", skip(tcx))] |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could also use ret here
📌 Commit 5e73a8b has been approved by oli-obk
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
This was referenced
Mar 22, 2024
Finished benchmarking commit (eff958c): comparison URL.
Overall result: no relevant changes - no action needed
@rustbot label: -perf-regression
Instruction count
This benchmark run did not return any relevant results for this metric.
Max RSS (memory usage)
This benchmark run did not return any relevant results for this metric.
Cycles
Results
This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) | - | - | 0 |
| Regressions ❌ (secondary) | 2.2% | [2.2%, 2.2%] | 1 |
| Improvements ✅ (primary) | - | - | 0 |
| Improvements ✅ (secondary) | - | - | 0 |
| All ❌✅ (primary) | - | - | 0 |
Binary size
This benchmark run did not return any relevant results for this metric.
Bootstrap: 669.305s -> 669.088s (-0.03%)
Artifact size: 315.01 MiB -> 314.99 MiB (-0.01%)
Labels
Area: Issues & PRs about the rust-lang/rust repository itself
This PR was explicitly merged by bors.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the compiler team, which will review and decide on the PR/issue.
Relevant to the rustdoc team, which will review and decide on the PR/issue.
The Rustc Trait System Refactor Initiative (-Znext-solver)