Clean up AstConv by fmease · Pull Request #122527 · 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

Merged

Clean up AstConv #122527

Conversation3 Commits1 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 }})

Split off from #120926 to make it only contain the renaming & (doc) comment updates.
Any changes other than that which have accumulated over time are now part of this PR.
Let's be disciplined ;) Inspired by #120926 (comment).

• Remove hir_trait_to_predicates
  • Unused since Make privacy visitor use types more (instead of HIR) #113671
• Inline create_args_for_ast_trait_ref
  • Only had a single call site
  • Having it as a separate method didn't gain us anything
• Use an if-let guard somewhere to avoid unwrapping
• Avoid explicit trait object lifetimes
  • More legible, stylistic-only (the updated code is 100% semantically identical)
  • Use explicitly elided lifetimes in impl headers, they get elaborated to distinct lifetimes
  • Make use of object lifetime defaulting for a trait object type inside of a reference type somewhere
• Use preexisting dedicated method ItemCtxt::to_ty over <dyn AstConv<'_>>::ast_ty_to_ty
• Use preexisting dedicated method AstConv::astconv over explicit coercions
• Simplify the function signature of create_args_for_ast_path and of check_generic_arg_count
  • In both cases redundant information was passed rendering the call sites verbose and confusing
  • No perf impact (tested in #120926)
• Move diagnostic method report_ambiguous_associated_type from astconv to astconv::errors
  • The submodule errors exists specifically for that purpose
  • Use it to keep the main module clean & short

fmease added C-cleanup

Category: PRs that clean code up or issues documenting cleanup.

T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

S-blocked

Status: Blocked on something else such as an RFC or other implementation work.

labels

Mar 15, 2024

fmease changed the titleClean up HIR ty lowering Clean up AstConv

Mar 15, 2024

fmease removed S-blocked

Status: Blocked on something else such as an RFC or other implementation work.

A-meta

Area: Issues & PRs about the rust-lang/rust repository itself

WG-trait-system-refactor

The Rustc Trait System Refactor Initiative (-Znext-solver)

labels

Mar 15, 2024

fmease marked this pull request as ready for review

March 15, 2024 04:08

Yeah I don't expect any of these changes to have a perf impact lol

@bors r+ rollup

📌 Commit 5beda81 has been approved by compiler-errors

It is now in the queue for this repository.

bors added S-waiting-on-bors

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

Mar 15, 2024

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Mar 15, 2024

…=compiler-errors

Clean up AstConv

Split off from rust-lang#120926 to make it only contain the renaming & (doc) comment updates. Any changes other than that which have accumulated over time are now part of this PR. Let's be disciplined ;) Inspired by rust-lang#120926 (comment).

• Remove hir_trait_to_predicates
  • Unused since rust-lang#113671
• Inline create_args_for_ast_trait_ref
  • Only had a single call site
  • Having it as a separate method didn't gain us anything
• Use an if-let guard somewhere to avoid unwrapping
• Avoid explicit trait object lifetimes
  • More legible, stylistic-only (the updated code is 100% semantically identical)
  • Use explicitly elided lifetimes in impl headers, they get elaborated to distinct lifetimes
  • Make use of object lifetime defaulting for a trait object type inside of a reference type somewhere
• Use preexisting dedicated method ItemCtxt::to_ty over <dyn AstConv<'_>>::ast_ty_to_ty
• Use preexisting dedicated method AstConv::astconv over explicit coercions
• Simplify the function signature of create_args_for_ast_path and of check_generic_arg_count
  • In both cases redundant information was passed rendering the call sites verbose and confusing
  • No perf impact (tested in rust-lang#120926)
• Move diagnostic method report_ambiguous_associated_type from astconv to astconv::errors
  • The submodule errors exists specifically for that purpose
  • Use it to keep the main module clean & short

bors added a commit to rust-lang-ci/rust that referenced this pull request

Mar 15, 2024

…iaskrgr

Rollup of 5 pull requests

Successful merges:

• rust-lang#121885 (Move generic NonZero rustc_layout_scalar_valid_range_start attribute to inner type.)
• rust-lang#122471 (preserve span when evaluating mir::ConstOperand)
• rust-lang#122515 (Pass the correct DefId when suggesting writing the aliased Self type out)
• rust-lang#122523 (Ensure RPITITs are created before def-id freezing)
• rust-lang#122527 (Clean up AstConv)

r? @ghost @rustbot modify labels: rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request

Mar 15, 2024

…iaskrgr

Rollup of 7 pull requests

Successful merges:

• rust-lang#121207 (Add -Z external-clangrt)
• rust-lang#122174 (diagnostics: suggest Clone bounds when noop clone())
• rust-lang#122471 (preserve span when evaluating mir::ConstOperand)
• rust-lang#122515 (Pass the correct DefId when suggesting writing the aliased Self type out)
• rust-lang#122523 (Ensure RPITITs are created before def-id freezing)
• rust-lang#122526 (Docs for thir::ExprKind::Use and thir::ExprKind::Let)
• rust-lang#122527 (Clean up AstConv)

r? @ghost @rustbot modify labels: rollup

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Mar 15, 2024

Rollup merge of rust-lang#122527 - fmease:clean-up-hir-ty-lowering, r=compiler-errors

Clean up AstConv

Split off from rust-lang#120926 to make it only contain the renaming & (doc) comment updates. Any changes other than that which have accumulated over time are now part of this PR. Let's be disciplined ;) Inspired by rust-lang#120926 (comment).

• Remove hir_trait_to_predicates
  • Unused since rust-lang#113671
• Inline create_args_for_ast_trait_ref
  • Only had a single call site
  • Having it as a separate method didn't gain us anything
• Use an if-let guard somewhere to avoid unwrapping
• Avoid explicit trait object lifetimes
  • More legible, stylistic-only (the updated code is 100% semantically identical)
  • Use explicitly elided lifetimes in impl headers, they get elaborated to distinct lifetimes
  • Make use of object lifetime defaulting for a trait object type inside of a reference type somewhere
• Use preexisting dedicated method ItemCtxt::to_ty over <dyn AstConv<'_>>::ast_ty_to_ty
• Use preexisting dedicated method AstConv::astconv over explicit coercions
• Simplify the function signature of create_args_for_ast_path and of check_generic_arg_count
  • In both cases redundant information was passed rendering the call sites verbose and confusing
  • No perf impact (tested in rust-lang#120926)
• Move diagnostic method report_ambiguous_associated_type from astconv to astconv::errors
  • The submodule errors exists specifically for that purpose
  • Use it to keep the main module clean & short

fmease deleted the clean-up-hir-ty-lowering branch

March 15, 2024 12:29

fmease added a commit to fmease/rust that referenced this pull request

Mar 16, 2024

…piler-errors

Remove obsolete parameter speculative from instantiate_poly_trait_ref

In rust-lang#122527 I totally missed that speculative has become obsolete with the removal of hir_trait_to_predicates / due to rust-lang#113671.

Fixes rust-lang#114635.

r? @compiler-errors

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Mar 17, 2024

Rollup merge of rust-lang#122577 - fmease:speculative-say-what, r=compiler-errors

Remove obsolete parameter speculative from instantiate_poly_trait_ref

In rust-lang#122527 I totally missed that speculative has become obsolete with the removal of hir_trait_to_predicates / due to rust-lang#113671.

Fixes rust-lang#114635.

r? @compiler-errors

Labels

C-cleanup

Category: PRs that clean code up or issues documenting cleanup.

S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.