Make extern_mod_stmt_cnum into a regular function

[Aaron1011]

cc #83126

It's very short, so there's no need for it to go through the query
system.

[rust-highfive]

r? @lcnr

(rust-highfive has picked a reviewer for you, use r? to override)

[Mark-Simulacrum]

@bors r+ rollup=never (may be perf sensitive)

[bors]

📌 Commit 0e9be63 has been approved by Mark-Simulacrum

[lcnr]

do we clear the query cache if the extern_crate_map changes between compilation sessions?

[Mark-Simulacrum]

I don't immediately see how it'd be different inside the query (i.e., why that would not lead to just as many problems as with this impl) -- but happy to r- this for now if you think that's warranted

[lcnr]

no, I also don't see how having a query helps here, so this PR seems good.

The existence of this query just feels kind of weird to me, maybe it was intended to improve incremental reuse in case extern_crate_map changes? But that wasn't actually the case afaict

[Aaron1011]

I think the important thing is that a CrateId doesn't actually store any information by itself. A query can only produce it as part of a result (which would lead to the same ICE if the StableCrateId changed), or call a query using it as an input (which will cause it to get re-run if the crate changes).

[michaelwoerister]

Are you all sure this is the right fix? Making something not be a query will certainly avoid triggering errors emitted by the query system's sanity checking -- but it might just turn this into a silent error. This not being a query will mean there's no dependency tracking for it.

[michaelwoerister]

@bors r-

Sorry for barging in here, but this feels kind of fishy to me. @Aaron1011's previous suggestion of turning this into an eval-always query seems like the safer option to me. I'll take a closer look right now.

[michaelwoerister]

Yes, I'm confident now that this should be an eval_always query. Like all other queries that directly read pre-populated data from a tcx field, this is an outside input into the query system (like module_exports, in_scope_traits_map, and crate_name for example). Turning this into a regular method will mean that we don't do any dependency tracking for it.

The fact that this has not been an eval_always query is the actual bug. Since the query just reads some data from the tcx, it will never have any dependencies. When try_mark_green tries to mark it green, it will not find any red dependencies, so it will assume that nothing has changed without ever fingerprinting the new value.

So, to sum up: just make the query eval_always and the bug should be fixed.

[Aaron1011]

Closing in favor of #83153