Document query modifiers

[Mark-Simulacrum]

We should document what the query modifiers mean in a more involved form than currently, ideally in rustc-guide or so, with a comment in src/librustc/query/mod.rs pointing to there.

[Mark-Simulacrum]

I've nominated this for compiler team so that we can discuss and hopefully assign someone to this, since I think it is likely an "easy" task for someone with the knowledge (perhaps @eddyb or @michaelwoerister) to give a quick summary which can then be put into docs by someone from @rust-lang/wg-learning perhaps.

Some specific questions:

• What does forcing a query mean? Why is it desirable to not allow that?
• Does eval_always mean the query is always run regardless of incremental? Does that mean we don't serialize it to disk? Why would we want this?
• What are anonymous queries for? What gain is there to not having a dep node for the query itself?

[eddyb]

I think the names and defaults are suboptimal, so we might also want to clean that up first.
no_force is required on any query that is not keyed on DefId or CrateNum, so maybe it should be type-based instead?
eval_always should be seldom used and merits a more descriptive name (it seems to be used for some "input" queries AFAICT?)

[Mark-Simulacrum]

Perhaps, although I think figuring out what they mean is also worthwhile -- does no_force then have something to do with not being able to "pull out" the query key from the hash we store in the serialized data? (So, if I have a DefId query, should I never mark it as no_force?)

[eddyb]

I just checked and no queries with a key type implementing RecoverKey have no_force.
Those types are DefId, CrateNum and DefIndex:

rust/src/librustc/dep_graph/dep_node.rs

Lines 434 to 454 in eb5ef81

	pub trait RecoverKey<'tcx>: Sized {
	fn recover(tcx: TyCtxt<'tcx>, dep_node: &DepNode) -> Option<Self>;
	}
	impl RecoverKey<'tcx> for CrateNum {
	fn recover(tcx: TyCtxt<'tcx>, dep_node: &DepNode) -> Option<Self> {
	dep_node.extract_def_id(tcx).map(|id| id.krate)
	}
	}
	impl RecoverKey<'tcx> for DefId {
	fn recover(tcx: TyCtxt<'tcx>, dep_node: &DepNode) -> Option<Self> {
	dep_node.extract_def_id(tcx)
	}
	}
	impl RecoverKey<'tcx> for DefIndex {
	fn recover(tcx: TyCtxt<'tcx>, dep_node: &DepNode) -> Option<Self> {
	dep_node.extract_def_id(tcx).map(|id| id.index)
	}
	}

So presumably this method could be on the more general Key trait and have a default implementation that always returns None, or maybe something slightly fancier to avoid doing work? (specialization is also an option, I guess, if we can't avoid using it)

Then we wouldn't need no_force, I would think? cc @Zoxc

[Zoxc]

@eddyb Sounds like something on my todo list. The default implementation of recover should panic though, as in, the type can't even try to recover.

[Zoxc]

@eddyb Is always_runs a better name for eval_always? =P

Input queries isn't really a thing. All inputs must be available when the query system is started. We have queries which use an input and only return part of it though, but that is a more general pattern.

[michaelwoerister]

What does forcing a query mean? Why is it desirable to not allow that?

"Forcing" basically means executing the query (i.e. we are forcing the result to be in the cache). In the context of incremental compilation, we force a query if we cannot guarantee that it has the same result during the current compilation session. In particular:

• During incremental compilation, before executing a query provider, we first try to mark the corresponding DepNode as green, because if it is green, we can use the result from the on-disk-cache. Or we might not need the result at all because we are just checking that there was no change.
• However, if we encounter a DepNode that cannot be marked as green (i.e. one of the query inputs has changed) then we need to "force" the corresponding query in order to find out if the result has changed or not.
• In such a case we take the DepNode and try to reconstruct the corresponding query key from it. Unfortunately, in the current implementation, this is only possible if the query key is of type DefId. If the query key has a different type, we cannot reconstruct the query key and thus also cannot force the query. All such queries have to be marked as no_force (otherwise you'll probably get compilation errors).
• So no_force really means "this query cannot be re-run from just the DepNode it corresponds to because we don't have enough information to reconstruct the query key".

[Mark-Simulacrum]

So, if I have a DefId query, should I never mark it as no_force?

So the answer here is yes, right? From what I understand based on what you've said, a DefId query can always be rerun.

And no_force is as such not really an additional marker, as @eddyb mentions, but more so just a way to bypass some "smart" logic based on the query inputs being DefId or not. i.e., you should only add it if you have to, never for actual "reasons" so to speak.

[michaelwoerister]

@Mark-Simulacrum Yes, that sounds correct.

[eddyb]

So no_force really means "this query cannot be re-run from just the DepNode it corresponds to because we don't have enough information to reconstruct the query key".

This makes me even more confident in the belief that no_force should be type-based and automatic.

@eddyb Is always_runs a better name for eval_always? =P

That still doesn't explain why you'd ever want that on a query, from my quick skimming it looks like it only exists on queries that read pre-filled data from TyCtxt (hence me calling them "input queries").

Input queries isn't really a thing. All inputs must be available when the query system is started. We have queries which use an input and only return part of it though, but that is a more general pattern.

What about the queries that expose the inputs available when the query system is started? My guess was that eval_always was specifically used for them.

I would prefer a name that only made sense for whatever eval_always happens to be used for today, not "always run this query", which would seem to apply to anything if not further qualified.

[Zoxc]

What about the queries that expose the inputs available when the query system is started?

These are only eval_always because that's slightly more efficient somewhere.

eval_always is used for 2 main reasons:

1. There some side effects that needs to be executed that incremental doesn't track (like writing to files or marking attributes a used). This is needed for correctness.
2. A query depends on pretty much the entire crate in which case we can skip recording its dependencies and just assume it changes. This is good for performance.

[eddyb]

What if we used two different names, to make it clearer?

1. has_side_effects
2. depends_on_everything

Bonus, you'd only rerun has_side_effects always, but depends_on_everything only if anything changed at all in whatever the dep graph tracks as "inputs" (unless it's those queries themselves?).

EDIT: if we're not thinking of performance at all, but just "side-effects":

1. output_side_effects
2. input_side_effects

[Zoxc]

Another reason to use eval_always is that running the query might be faster than skipping it. If the query is a simple parallel for loop which just runs other queries, it will only be parallel if we actually run it, not if the query system has to serially check if dependencies are up to date.