Stream: t-compiler/help

Topic: reason for `TyCtxtEnsure`


Bastian Kauschke (Apr 25 2020 at 09:47, on Zulip):

We use tcx.ensure().query_name(...) in a few places in the compiler.
Looking at https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TyCtxt.html#method.ensure

Returns a transparent wrapper for TyCtxt, which ensures queries are executed instead of just returning their results.

I do not quite understand where this is desired (afaict it might reemit some warnings between incremental builds, which might be useful :shrug: ).

E.g. in optimized_mir we ensure mir_borrowck https://github.com/rust-lang/rust/blob/master/src/librustc_mir/transform/mod.rs#L338

With the comment

// (Mir-)Borrowck uses `mir_validated`, so we have to force it to
// execute before we can steal.
tcx.ensure().mir_borrowck(def_id);

I don't see why ensure is actually needed here. ensure only changes the behavior in case mir_borrowck is already cached. In this case it recomputes it and stores the same result in the cache, changing no observable behavior afaict. Why would we want to rerun mir_borrowck if we already did so before?

What am I missing here :sweat_smile:

bjorn3 (Apr 25 2020 at 12:11, on Zulip):

If optimized_mir for a function were to run before mir_borrowck, the steal of the mir at https://github.com/rust-lang/rust/blob/a58b1ed44f5e06976de2bdc4d7dc81c36a96934f/src/librustc_mir/transform/mod.rs#L341 would mean that mir_borrowck can no longer access the mir_validated output.

bjorn3 (Apr 25 2020 at 12:12, on Zulip):

mir_validated returns Steal<mir::Body>, not mir::Body. This makes it possible to steal the inner mir::Body, after which any attempt to access it will panic. This is an optimization to prevent a clone of mir::Body, which is very costly.

Bastian Kauschke (Apr 25 2020 at 12:15, on Zulip):

jup, what I don't understand is why we need ensure there.

oli (Apr 25 2020 at 12:15, on Zulip):

before stealing, you want to ensure that mir_borrowck has already been run

oli (Apr 25 2020 at 12:16, on Zulip):

because then the query is cached

oli (Apr 25 2020 at 12:16, on Zulip):

and future invocations of it will just yield the cached result instead of running the body which would try to access mir_validated

Bastian Kauschke (Apr 25 2020 at 12:16, on Zulip):

But isn't this also a given if we call mir_borrowck without ensure

oli (Apr 25 2020 at 12:16, on Zulip):

yes

Bastian Kauschke (Apr 25 2020 at 12:17, on Zulip):

Afaik ensure actually reruns mir_borrowck there even if it was already called

Bastian Kauschke (Apr 25 2020 at 12:17, on Zulip):

which seems like it's unnecessary

oli (Apr 25 2020 at 12:17, on Zulip):

ensure is just a scheme for calling a query without using its result afaik, but that's just from superficial knowledge

Bastian Kauschke (Apr 25 2020 at 12:19, on Zulip):

Afaik ensure actually reruns mir_borrowck there even if it was already called

Aaah, so this was where I was wrong. TyCtxtEnsure is pretty much

fn query_name(..) {
    self.tcx.query_name(...);
}
Bastian Kauschke (Apr 25 2020 at 12:22, on Zulip):

Thank you

Jonas Schievink (Apr 25 2020 at 12:24, on Zulip):

The documentation on fn ensure is somewhat misleading there

Bastian Kauschke (Apr 25 2020 at 13:18, on Zulip):

Looking somewhat more into this, we sometimes use let _ = tcx.query_name(...); instead of ensure. https://github.com/rust-lang/rust/blob/a58b1ed44f5e06976de2bdc4d7dc81c36a96934f/src/librustc_mir/transform/mod.rs#L240

As I can fully get what this does without having to read and understand the docs of ensure I am kind of in favor of just removing ensure entirely :laughing: I still don't know too much about the query system yet, so there might be some advantage to using ensure I don't know of

bjorn3 (Apr 25 2020 at 13:22, on Zulip):

When the query is already cached, tcx.ensure() doesn't create a clone of the query result, which saves time for big values.

Bastian Kauschke (Apr 25 2020 at 13:24, on Zulip):

true :sparkles: do we have any queries which return a big struct directly instead of a reference

eddyb (Apr 27 2020 at 06:40, on Zulip):

maybe we should require that the result is Copy :P

eddyb (Apr 27 2020 at 06:40, on Zulip):

and aggressively intern everything

Last update: Sep 27 2020 at 13:15UTC