Assert redundant incremental check

[zetanumbers]

So to incrementally execute tcx.ensure_done().query() we first do query graph coloring to determine if nothing has changed and maybe color our query green. In that case for ensure_ok we skip further execution and return. However for ensure_done we do this check:

rust/compiler/rustc_query_impl/src/execution.rs

Lines 601 to 608 in 7c3c88f

	// In ensure-done mode, we can only skip execution for this key
	// if there's a disk-cached value available to load later if
	// needed, which guarantees the query provider will never run
	// for this key.
	EnsureMode::Done => {
	(query.will_cache_on_disk_for_key_fn)(key)
	&& loadable_from_disk(tcx, serialized_dep_node_index)
	}

Which in turn looks up a serialized query value:

rust/compiler/rustc_middle/src/query/on_disk_cache.rs

Lines 338 to 343 in 7c3c88f

	/// Returns true if there is a disk-cached query return value for the given node.
	#[inline]
	pub fn loadable_from_disk(&self, dep_node_index: SerializedDepNodeIndex) -> bool {
	self.query_values_index.contains_key(&dep_node_index)
	// with_decoder is infallible, so we can stop here
	}

However, we only serialize query values if the pure (I've checked) function will_cache_on_disk_for_key_fn from above returns true:

rust/compiler/rustc_query_impl/src/plumbing.rs

Lines 95 to 99 in 7c3c88f

	query.cache.for_each(&mut |key, value, dep_node| {
	if (query.will_cache_on_disk_for_key_fn)(*key) {
	encoder.encode_query_value::<V>(dep_node, &erase::restore_val::<V>(*value));
	}
	});

As such loadable_from_disk should be able to simply check if tcx.query_system.on_disk_cache is loaded and assume query value is present, assuming that will_cache_on_disk_for_key_fn returned true.

[rustbot]

r? @chenyukang

rustbot has assigned @chenyukang.
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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler, query-system
• compiler, query-system expanded to 73 candidates
• Random selection from 17 candidates

[zetanumbers]

I think we can clarify this change with a comment or by outright inlining that function. Not sure what to pick tho.

[zetanumbers]

Also that check is covered by tests, so I making it unconditionally panic would trigger following incremental tests failures:

• change_crate_dep_kind.rs
• const-generics/change-const-param-type.rs
• const-generics/change-const-param-gat.rs
• hashes/consts.rs
• define-opaques.rs
• hashes/closure_expressions.rs
• feature_gate.rs
• commandline-args.rs
• hashes/call_expressions.rs
• hashes/enum_constructors.rs
• hashes/if_expressions.rs
• hashes/loop_expressions.rs
• hashes/function_interfaces.rs
• hashes/for_loops.rs
• hashes/inherent_impls.rs
• hashes/struct_constructors.rs
• hashes/while_loops.rs
• hashes/statics.rs
• hashes/while_let_loops.rs
• hashes/trait_impls.rs
• ich_nested_items.rs
• issue-100521-change-struct-name-assocty.rs
• issue-49595/issue-49595.rs
• reorder_vtable.rs
• spans_significant_w_panic.rs
• spans_significant_w_debuginfo.rs
• issue-110457-same-span-closures/main.rs
• static_cycle/b.rs
• static_refering_to_other_static2/issue.rs
• static_refering_to_other_static3/issue.rs
• thinlto/cgu_keeps_identical_fn.rs
• static_stable_hash/issue-49301.rs
• thinlto/cgu_invalidated_when_export_added.rs
• unrecoverable_query.rs

[zetanumbers]

I think we could change it to debug_assert and check if perf is affected.