Strictly proof not tainted by errors in TypeckResults

[Luv-Ray]

I get here from #125888.

For now there is a hack when calling tcx.typeck():

rust/compiler/rustc_hir_typeck/src/writeback.rs

Lines 125 to 130 in 76c7382

	// HACK: We specifically don't want the (opaque) error from tainting our
	// inference context. That'll prevent us from doing opaque type inference
	// later on in borrowck, which affects diagnostic spans pretty negatively.
	if let Some(e) = fcx.tainted_by_errors() {
	wbcx.typeck_results.tainted_by_errors = Some(e);
	}

so that in returned TypeckResults, the tainted_by_errors may be None even though there is actually some errors. And this could cause many ICEs, because some functions trust that there is no type errors, and whenever it encounters an error, it won't throw an error report and just panic (#125888 is an example).

---

In this tainted_by_errors function, I try to change the condition to if self.dcx().err_count_excluding_lint_errs() > 0, then return the ErrorGuaranteed, and find that it could resolve 62 ICEs.

Resolved ICE list

    [crashes] tests/crashes/110630.rs
    [crashes] tests/crashes/111709.rs
    [crashes] tests/crashes/111709-2.rs
    [crashes] tests/crashes/112623.rs
    [crashes] tests/crashes/113379.rs
    [crashes] tests/crashes/114663.rs
    [crashes] tests/crashes/115808.rs
    [crashes] tests/crashes/117795.rs
    [crashes] tests/crashes/118545.rs
    [crashes] tests/crashes/119701.rs
    [crashes] tests/crashes/119729.rs
    [crashes] tests/crashes/119786.rs
    [crashes] tests/crashes/120793-2.rs
    [crashes] tests/crashes/120793.rs
    [crashes] tests/crashes/120792.rs
    [crashes] tests/crashes/120873.rs
    [crashes] tests/crashes/121097.rs
    [crashes] tests/crashes/121127.rs
    [crashes] tests/crashes/121063.rs
    [crashes] tests/crashes/121052.rs
    [crashes] tests/crashes/121411.rs
    [crashes] tests/crashes/121429.rs
    [crashes] tests/crashes/121613-2.rs
    [crashes] tests/crashes/121613.rs
    [crashes] tests/crashes/121623.rs
    [crashes] tests/crashes/121816.rs
    [crashes] tests/crashes/121957-1.rs
    [crashes] tests/crashes/121957-2.rs
    [crashes] tests/crashes/122044.rs
    [crashes] tests/crashes/122587-1.rs
    [crashes] tests/crashes/121858.rs
    [crashes] tests/crashes/122259.rs
    [crashes] tests/crashes/122630.rs
    [crashes] tests/crashes/122681.rs
    [crashes] tests/crashes/122904-2.rs
    [crashes] tests/crashes/122904.rs
    [crashes] tests/crashes/122909.rs
    [crashes] tests/crashes/123255.rs
    [crashes] tests/crashes/123276.rs
    [crashes] tests/crashes/123690.rs
    [crashes] tests/crashes/124004.rs
    [crashes] tests/crashes/124021.rs
    [crashes] tests/crashes/124083.rs
    [crashes] tests/crashes/124164.rs
    [crashes] tests/crashes/124262.rs
    [crashes] tests/crashes/124182.rs
    [crashes] tests/crashes/124340.rs
    [crashes] tests/crashes/124563.rs
    [crashes] tests/crashes/124583.rs
    [crashes] tests/crashes/124436.rs
    [crashes] tests/crashes/124894.rs
    [crashes] tests/crashes/125155.rs
    [crashes] tests/crashes/125185.rs
    [crashes] tests/crashes/125553.rs
    [crashes] tests/crashes/125758.rs
    [crashes] tests/crashes/125801.rs
    [crashes] tests/crashes/125768.rs
    [crashes] tests/crashes/125769.rs
    [crashes] tests/crashes/125874.rs
    [crashes] tests/crashes/125888.rs
    [crashes] tests/crashes/125914.rs
    [crashes] tests/crashes/125992.rs

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

Lines 1152 to 1172 in f6b4b71

	/// 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
	}
	}

---

But it will also break many ui tests, because as the hack said, it will affect diagnostic spans pretty negatively. So I think a proper way to resolve that is return a new defined enum rather than Option, such as

rust/compiler/rustc_middle/src/ty/typeck_results.rs

Lines 161 to 163 in f6b4b71

	/// If any errors occurred while type-checking this body,
	/// this field will be set to `Some(ErrorGuaranteed)`.
	pub tainted_by_errors: Option<ErrorGuaranteed>,

enum TaintedByErrors {
    Normal(ErrorGuaranteed),
    Opaque(ErrorGuaranteed),
    StrictlyNoError,
}

So that we can choose to either early return or do more diagnostics.

@rustbot label +I-ICE +T-compiler

[fmease]

cc @oli-obk

[oli-obk]

self.dcx().err_count_excluding_lint_errs()

This is global information, and while it could fix many ICEs, it will cause us to hide errors if there were errors anywhere, even totally unrelated to the current item being type checked.

Instead we should find out why no error tainting happens in your program B in #125888

Wrt the hack: I'm not sure anymore why that comment is there. We're tainting after all, and no opaque type logic is involved. Maybe we used to taint after all of typeck is done? Will need some investigation

[veera-sivarajan]

@rustbot label -needs-triage +T-types

[Luv-Ray]

self.err_count_on_creation and self.dcx().err_count_excluding_lint_errs() excludes delayed bugs, so if the current item only raise delayed bugs, it will not set self as tained by errors.

I think it's more reasonable to check all kinds of error count not changed when checking if self is tained by errors, but directly count in delayed bugs here will cause many ui tests fail too. Maybe it needs a more subtle way to fix.

[oli-obk]

These global error counts include errors from other items, so an unrelated error from another item will silence errors in this item.