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

Start removing rustc_middle::hir::map::Map #136466

Merged
merged 7 commits into from
Feb 17, 2025

Conversation

nnethercote
Copy link
Contributor

rustc_middle::hir::map::Map is now just a low-value wrapper around TyCtxt. This PR starts removing it.

r? @cjgillot

@rustbot rustbot added A-rustc-dev-guide Area: rustc-dev-guide A-rustdoc-json Area: Rustdoc JSON backend 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Feb 3, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 3, 2025

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

changes to the core type system

cc @compiler-errors, @lcnr

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes.

cc @BoxyUwU, @jieyouxu, @Kobzol

Some changes occurred in coverage instrumentation.

cc @Zalathar

Some changes occurred in match checking

cc @Nadrieril

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

HIR ty lowering was modified

cc @fmease

Some changes occurred in need_type_info.rs

cc @lcnr

@nnethercote
Copy link
Contributor Author

There are a lot more methods to move from Map to TyCtxt, but I thought I would file this to see if people are happy with the general approach before sinking more time into it. It's good to see the changes to the intravisit::Map trait worked out.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Feb 5, 2025

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

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@nnethercote
Copy link
Contributor Author

Note: this comment is the one that identified that Map is now just a wrapper around TyCtxt.

@bors
Copy link
Contributor

bors commented Feb 6, 2025

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

@rustbot rustbot added the A-attributes Area: Attributes (`#[…]`, `#![…]`) label Feb 11, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 11, 2025

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann

@nnethercote
Copy link
Contributor Author

I rebased. This PR is conflict-prone.

@bors
Copy link
Contributor

bors commented Feb 12, 2025

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

@nnethercote
Copy link
Contributor Author

The conflicts are trivial and won't affect review, so I will wait before fixing them.

The `Map` trait is there for cases where `tcx` isn't available. This
isn't one of those cases, so it's simpler to just call through `tcx`
directly.
@rustbot rustbot added the T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. label Feb 16, 2025
@nnethercote
Copy link
Contributor Author

It's been two weeks, let's try a different reviewer. I have rebased to fix the conflicts.

r? Zalathar

@rustbot rustbot assigned Zalathar and unassigned cjgillot Feb 16, 2025
Copy link
Contributor

@cjgillot cjgillot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for cleaning this up.
2 nits and r=me

The end goal is to eliminate `Map` altogether.

I added a `hir_` prefix to all of them, that seemed simplest. The
exceptions are `module_items` which became `hir_module_free_items` because
there was already a `hir_module_items`, and `items` which became
`hir_free_items` for consistency with `hir_module_free_items`.
First of all, note that `Map` has three different relevant meanings.
- The `intravisit::Map` trait.
- The `map::Map` struct.
- The `NestedFilter::Map` associated type.

The `intravisit::Map` trait is impl'd twice.
- For `!`, where the methods are all unreachable.
- For `map::Map`, which gets HIR stuff from the `TyCtxt`.

As part of getting rid of `map::Map`, this commit changes `impl
intravisit::Map for map::Map` to `impl intravisit::Map for TyCtxt`. It's
fairly straightforward except various things are renamed, because the
existing names would no longer have made sense.

- `trait intravisit::Map` becomes `trait intravisit::HirTyCtxt`, so named
  because it gets some HIR stuff from a `TyCtxt`.
- `NestedFilter::Map` assoc type becomes `NestedFilter::MaybeTyCtxt`,
  because it's always `!` or `TyCtxt`.
- `Visitor::nested_visit_map` becomes `Visitor::maybe_tcx`.

I deliberately made the new trait and associated type names different to
avoid the old `type Map: Map` situation, which I found confusing. We now
have `type MaybeTyCtxt: HirTyCtxt`.
It's a trivial wrapper around the `hir_crate` query with a small number
of uses.
@nnethercote
Copy link
Contributor Author

I fixed the nits.

@bors r=cjgillot

@bors
Copy link
Contributor

bors commented Feb 17, 2025

📌 Commit f666361 has been approved by cjgillot

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 Feb 17, 2025
nnethercote added a commit to nnethercote/rust that referenced this pull request Feb 17, 2025
Continuing the work started in rust-lang#136466.

Every method gains a `hir_` prefix, though for the ones that already
have a `par_` or `try_par_` prefix I added the `hir_` after that.
nnethercote added a commit to nnethercote/rust that referenced this pull request Feb 17, 2025
Continuing the work started in rust-lang#136466.

Every method gains a `hir_` prefix, though for the ones that already
have a `par_` or `try_par_` prefix I added the `hir_` after that.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 17, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#136466 (Start removing `rustc_middle::hir::map::Map`)
 - rust-lang#136671 (Overhaul `rustc_middle::limits`)
 - rust-lang#136817 (Pattern Migration 2024: clean up and comment)
 - rust-lang#136844 (Use `const_error!` when possible)
 - rust-lang#137080 (bootstrap: add more tracing to compiler/std/llvm flows)
 - rust-lang#137101 (`invalid_from_utf8[_unchecked]`: also lint inherent methods)
 - rust-lang#137140 (Fix const items not being allowed to be called `r#move` or `r#static`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f3a4f1a into rust-lang:master Feb 17, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Feb 17, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 17, 2025
Rollup merge of rust-lang#136466 - nnethercote:start-removing-Map, r=cjgillot

Start removing `rustc_middle::hir::map::Map`

`rustc_middle::hir::map::Map` is now just a low-value wrapper around `TyCtxt`. This PR starts removing it.

r? `@cjgillot`
@nnethercote nnethercote deleted the start-removing-Map branch February 17, 2025 21:43
nnethercote added a commit to nnethercote/rust that referenced this pull request Feb 17, 2025
Continuing the work started in rust-lang#136466.

Every method gains a `hir_` prefix, though for the ones that already
have a `par_` or `try_par_` prefix I added the `hir_` after that.
Zalathar added a commit to Zalathar/rust that referenced this pull request Feb 18, 2025
Move methods from `Map` to `TyCtxt`, part 2.

Continuing the work started in rust-lang#136466.

Every method gains a `hir_` prefix, though for the ones that already have a `par_` or `try_par_` prefix I added the `hir_` after that.

r? Zalathar
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 18, 2025
Move methods from `Map` to `TyCtxt`, part 2.

Continuing the work started in rust-lang#136466.

Every method gains a `hir_` prefix, though for the ones that already have a `par_` or `try_par_` prefix I added the `hir_` after that.

r? Zalathar
flip1995 pushed a commit to flip1995/rust that referenced this pull request Feb 20, 2025
Continuing the work started in rust-lang#136466.

Every method gains a `hir_` prefix, though for the ones that already
have a `par_` or `try_par_` prefix I added the `hir_` after that.
github-merge-queue bot pushed a commit to model-checking/kani that referenced this pull request Feb 22, 2025
Upstream PRs requiring changes:

rust-lang/rust#135994
rust-lang/rust#136466

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 and MIT licenses.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-rustc-dev-guide Area: rustc-dev-guide A-rustdoc-json Area: Rustdoc JSON backend 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants