Skip to content
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

rustc_hir_analysis cleanups #136281

Merged
merged 14 commits into from
Jan 31, 2025
Merged

Conversation

nnethercote
Copy link
Contributor

Just some improvements I found while looking through this code.

r? @lcnr

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 30, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 30, 2025

HIR ty lowering was modified

cc @fmease

@lcnr
Copy link
Contributor

lcnr commented Jan 30, 2025

r=me after nit for everything outside of df90ba0

I personally don't think reflowing comments is worth it as it results in merge conflicts for little benefit, e.g. the changes to

// FIXME(#132279): Once PostBorrowckAnalysis is supported in the old solver, this branch
// should be removed.

will conflict with #135899.

@bors
Copy link
Contributor

bors commented Jan 30, 2025

☔ The latest upstream changes (presumably #134824) made this pull request unmergeable. Please resolve the merge conflicts.

I was confused here for a bit.
Also rewrite the merged arm slightly to more closely match the arm above
it.
This comment made sense when this crate was called `rustc_typeck`, but
makes less sense now that it's called `rustc_hir_analysis`. Especially
given that `check_drop_impl` is only called within the crate.
Note: `inherit_predicates_for_delegation_item` already has these cases
merged.
There is a comment `Delegation to inherent methods is not yet
supported.` that appears three times mid-pattern and somehow inhibits
rustfmt from formatting the enclosing `match` statement. This commit
moves them to the top of the pattern, which enables more formatting.
`delegation.rs` has three builders: `GenericsBuilder`,
`PredicatesBuilder`, and `GenericArgsBuilder`. The first two builders
have just two optional parameters, and the third one has zero. Each
builder is used within a single function. The code is over-engineered.

This commit removes the builders, replacing each with with a single
`build_*` function. This makes the code shorter and simpler.
It used to be bigger, with an `Xform` trait, but now it has just a
single function.
Instead re-export `rustc_hir_analysis::collect::suggest_impl_trait`,
which is the only thing from the module used in another crate. This
fixes a `FIXME` comment. Also adjust some visibilities to satisfy the
`unreachable_pub` lint.

This changes requires downgrading a link in a comment on `FnCtxt`
because `collect` is no longer public and rustdoc complains otherwise.
This is annoying but I can't see how to avoid it.
@nnethercote
Copy link
Contributor Author

I removed the comment reflows.

@bors r=lcnr

@bors
Copy link
Contributor

bors commented Jan 31, 2025

📌 Commit 483307f has been approved by lcnr

It is now in the queue for this repository.

@bors 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 Jan 31, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 31, 2025
Rollup of 9 pull requests

Successful merges:

 - rust-lang#132156 (When encountering unexpected closure return type, point at return type/expression)
 - rust-lang#133429 (Autodiff Upstreaming - rustc_codegen_ssa, rustc_middle)
 - rust-lang#136281 (`rustc_hir_analysis` cleanups)
 - rust-lang#136297 (Fix a typo in profile-guided-optimization.md)
 - rust-lang#136300 (atomic: extend compare_and_swap migration docs)
 - rust-lang#136310 (normalize `*.long-type.txt` paths for compare-mode tests)
 - rust-lang#136312 (Disable `overflow_delimited_expr` in edition 2024)
 - rust-lang#136313 (Filter out RPITITs when suggesting unconstrained assoc type on too many generics)
 - rust-lang#136323 (Fix a typo in conventions.md)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 940b45f into rust-lang:master Jan 31, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Jan 31, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 31, 2025
Rollup merge of rust-lang#136281 - nnethercote:rustc_hir_analysis, r=lcnr

`rustc_hir_analysis` cleanups

Just some improvements I found while looking through this code.

r? `@lcnr`
@nnethercote nnethercote deleted the rustc_hir_analysis branch February 1, 2025 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants