Move the query list into a new rustc_middle::queries module

[Zoxc]

This moves the query list from rustc_middle::query into a new rustc_middle::queries module. This splits up the use of the query system from the remaining implementation of it in rustc_middle::query, which conceptually belong to rustc_query_system.

The goal is to let rustc crates define queries with their own queries module, and this makes rustc_middle also fit this pattern.

The inner queries module used by the macros are renamed to query_info, so it doesn't conflict with the new outer name.

[rustbot]

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[nnethercote]

This appears to be the first step of a much larger change that would definitely need an MCP.

[Zoxc]

Just to clarify, I think this makes sense whether or not we want to let other crates define queries. It would just be additional motivation to do so.

[nnethercote]

I don't like query_info as a name. info (or data) in a name is usuall a bad sign, because it's so generic.

Is the query_info module providing any real value? Could we eliminate it, and reduce queries::query_info::$name to just queries::$name?

[Zoxc]

Is the query_info module providing any real value? Could we eliminate it, and reduce queries::query_info::$name to just queries::$name?

It's quite useful. We'd want to keep it in an fresh submodule to avoid naming conflicts. Maybe just query would be a better name?

[nnethercote]

What naming conflicts could occur? Isn't the point of the queries submodule to isolate all the queries (which have unique names)?

[Zoxc]

Yeah, I guess we could just keep the new top-level rustc_middle::queries module clean to avoid conflicts.

[Zoxc]

Actually the proc macro generates some modules which may conflict.

[nnethercote]

What are the conflicting modules?

[petrochenkov]

r? @nnethercote

[Zoxc]

We have 3 modules each defining a $name item per query: query_info, descs, cached. The module wasn't as clear as I though, but we can rename the others if needed (they look like unlikely query names currently).

[nnethercote]

The queries::$name/queries::#name stuff is good.

The queries::desc/queries::cached is less good, but probably ok.

There's also all of these:

use crate::queries::{
    ExternProviders, PerQueryVTables, Providers, QueryArenas, QueryCaches, QueryEngine, QueryStates,
};

Is that avoidable? Ideally the queries module would only contain the individual query names...

[Zoxc]

Is that avoidable?

The macro could probably just use full paths for these instead.

[nnethercote]

In case it wasn't clear: I would prefer ExternProviders, PerQueryVTables, etc., to not be in the queries module, if possible.

[Zoxc]

Ah, I though at first you were referring to the extra imports in the queries module (I think those should go). The types you refer to are specific to the queries defined and do belong alongside them. Not sure why you'd want them moved?

[rustbot]

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.

[nnethercote]

I guess the argument is that ExternProviders et al. are types that have one field per query, i.e. their structure is entirely dependent on the queries list, and therefore they belong in the same module as the query functions themselves. I can live with that.

@bors r+

[rust-bors]

📌 Commit 9de16dd has been approved by nnethercote

It is now in the queue for this repository.

[Zoxc]

Conceptually the types would be defined in rustc_middle::query and be generic over the query list, which we can't practically do in Rust.

[Zalathar]

In general I think it's premature to make sweeping changes based on the ambition of splitting query declarations across multiple crates, because that's an MCP-level change that I still have a lot of reservations about.

However, in this particular case pulling the query declarations out of rustc_middle::query is a good idea anyway, because it was awkward to have those declarations in a mod.rs file mixed in with submodules full of plumbing.

So I guess this is fine?

[Zalathar]

As a matter of commit style, I generally don't like to see moves and fixups in separate commits, unless it's absolutely necessary to get Git rename-tracking to still work by default.

EDIT: Though looking at the squashed PR diff, the separation does seem to be necessary to properly preserve the move, so I guess this is one of those exceptional cases after all.

Similarly, I don't like to see unnecessary indentation changes in the fixups. While Git tools can sometimes deal with that, it still adds an unwelcome layer of noise when trying to make sense of history.

[Zalathar]

We have 3 modules each defining a $name item per query: query_info, descs, cached. The module wasn't as clear as I though, but we can rename the others if needed (they look like unlikely query names currently).

In #151943 I intend to rename cached to _cache_on_disk_if_fns, and I'll probably do something similar with descs in a follow-up.

Regarding the flattened queries module, in some ways I did like the explicitness of a separate submodule for the macro-generated helper items, because I think it makes the use-sites a bit clearer. But we'll see how this flatter scheme works out in practice; it could be fine.