Error tainting in parallel compiler

[olafes]

Emitting diagnostics in one thread taints every infcx out there, but other threads don't expect their infcx to become tainted if they themselves aren't emitting anything. This leads to skipped diagnostics, early bailing out of queries that could've run to completion or painful to debug ICEs. For example, take #120601: snippet below run with -Zthreads=3 --edition=2021 works, but crashes if you uncomment the last function.

struct Tuple(i32);

async fn tuple() -> Tuple {
    Tuple(1i32)
}

async fn xyz() {
    match tuple() {
        Tuple(_) => {},
    }
}

/* uncomment this function to observe crashes
async fn fail() {
    Fail(())
}
*/

In parallel compiler, code that does different things depending on the result of InferCtxt::tainted_by_errors() can introduce spurious failures or even replace previously resolved Ty's with Ty::new_error():

rust/compiler/rustc_borrowck/src/nll.rs

Line 202 in f9515fd

let remapped_opaque_tys = regioncx.infer_opaque_types(infcx, opaque_type_values);

in regioncx.infer_opaque_types:

rust/compiler/rustc_borrowck/src/region_infer/opaque_types.rs

Lines 133 to 134 in f9515fd

	let ty =
	infcx.infer_opaque_definition_from_instantiation(opaque_type_key, concrete_type);

rust/compiler/rustc_borrowck/src/region_infer/opaque_types.rs

Lines 278 to 285 in f9515fd

	fn infer_opaque_definition_from_instantiation(
	&self,
	opaque_type_key: OpaqueTypeKey<'tcx>,
	instantiated_ty: OpaqueHiddenType<'tcx>,
	) -> Ty<'tcx> {
	if let Some(e) = self.tainted_by_errors() {
	return Ty::new_error(self.tcx, e);
	}

relevant part of InferCtxt:

rust/compiler/rustc_infer/src/infer/mod.rs

Lines 1219 to 1239 in f9515fd

	/// Returns `true` if errors have been reported since this infcx was
	/// created. This is sometimes used as a heuristic to skip
	/// reporting errors that often occur as a result of earlier
	/// errors, but where it's hard to be 100% sure (e.g., unresolved
	/// inference variables, regionck errors).
	#[must_use = "this method does not have any side effects"]
	pub fn tainted_by_errors(&self) -> Option<ErrorGuaranteed> {
	if let Some(guar) = self.tainted_by_errors.get() {
	Some(guar)
	} else if self.dcx().err_count_excluding_lint_errs() > self.err_count_on_creation {
	// Errors reported since this infcx was made. Lint errors are
	// excluded to avoid some being swallowed in the presence of
	// non-lint errors. (It's arguable whether or not this exclusion is
	// important.)
	let guar = self.dcx().has_errors().unwrap();
	self.set_tainted_by_errors(guar);
	Some(guar)
	} else {
	None
	}
	}

rustc 1.81.0-nightly (f1586001a 2024-06-13)
binary: rustc
commit-hash: f1586001ace26df7cafeb6534eaf76fb2c5513e5
commit-date: 2024-06-13
host: x86_64-unknown-linux-gnu
release: 1.81.0-nightly
LLVM version: 18.1.7

[olafes]

@rustbot label T-compiler I-ICE A-diagnostics WG-compiler-parallel

[oli-obk]

yea, the reason this exists is that it wants to detect when queries invoked by the current InferCtxt cause errors. The way to fix this is to make more queries return Result<CurrentOutput, ErrorGuaranteed> or taint their output in case the output supports that.

Next steps include some data gathering: remove this check and look at the failing ui tests with -Ztreat-err-as-bug to see which queries are reporting errors, but not tainting their result.