Make sure that predicates with unmentioned bound vars are still considered global in the old solver

[compiler-errors]

In the old solver, we consider predicates with late-bound vars to not be "global":

rust/compiler/rustc_trait_selection/src/traits/select/mod.rs

Lines 1840 to 1844 in 9c8a269

	// Check if a bound would previously have been removed when normalizing
	// the param_env so that it can be given the lowest priority. See
	// #50825 for the motivation for this.
	let is_global =
	|cand: &ty::PolyTraitPredicate<'tcx>| cand.is_global() && !cand.has_late_bound_vars();

The implementation of has_late_bound_vars was modified in #115834 so that we'd properly anonymize binders that had late-bound vars but didn't reference them. This fixed an ICE.

However, this also led to a behavioral change in #117056 (comment) for a couple of crates, which now consider for<'a> GL33: Shader (note the binder var that is not used in the predicate) to not be "global". This forces associated types to not be normalizable due to the old trait solver being dumb.

This PR distinguishes types which reference late-bound vars and binders which have late-bound vars. The latter is represented with the new type flag TypeFlags::HAS_BINDER_VARS, which is used when we only care about knowing whether binders have vars in their bound var list (even if they're not used, like for binder anonymization).

This should fix (after beta backport) the luminance-gl and luminance-webgl crates in #117056.

r? types
(priority is kinda high on a review here given beta becomes stable on November 16.)

[compiler-errors]

@bors try

[bors]

⌛ Trying commit 32294fc with merge 1e981f3...

[jackh726]

r=me with or without crater (maybe we want to land and then do a crater run?)

[bors]

☀️ Try build successful - checks-actions
Build commit: 1e981f3 (1e981f329e2ae73beb121ad9b30e8013bc4ead78)

[compiler-errors]

@craterbot check

[craterbot]

👌 Experiment pr-117589 created and queued.
🤖 Automatically detected try build 1e981f3
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[compiler-errors]

@bors r=jackh726 rollup=never

[bors]

📌 Commit 32294fc has been approved by jackh726

It is now in the queue for this repository.

[craterbot]

🚧 Experiment pr-117589 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[bors]

⌛ Testing commit 32294fc with merge f64d028...

[bors]

☀️ Test successful - checks-actions
Approved by: jackh726
Pushing f64d028 to master...

[rust-timer]

Finished benchmarking commit (f64d028): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.4%	[0.8%, 2.6%]	4
Regressions ❌ (secondary)	2.7%	[2.7%, 2.7%]	1
Improvements ✅ (primary)	-0.9%	[-2.3%, -0.4%]	4
Improvements ✅ (secondary)	-1.0%	[-1.0%, -1.0%]	1
All ❌✅ (primary)	0.3%	[-2.3%, 2.6%]	8

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.5%	[0.5%, 0.6%]	2
Regressions ❌ (secondary)	0.5%	[0.5%, 0.5%]	1
Improvements ✅ (primary)	-0.5%	[-0.7%, -0.4%]	4
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.2%	[-0.7%, 0.6%]	6

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 638.32s -> 638.639s (0.05%)
Artifact size: 304.42 MiB -> 304.53 MiB (0.04%)

[craterbot]

🎉 Experiment pr-117589 is completed!
📊 2 regressed and 12 fixed (385369 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[lqd]

No need to re-run crater on the regressions imho, out of the two:

• the behavior change on soa_derive-0.13.0 is probably real
• graphql-client-302-repro however is unrelated and non-deterministic

[jackh726]

We should open a PR to soa_derive I think

(At the very least- it does seem like a bit of a rough regression)

[jackh726]

To revert to the old behavior for this, we probably need to change here:

rust/compiler/rustc_hir_analysis/src/check/wfcheck.rs

Line 1880 in 6bf2fb3

if pred.is_global() && !pred.has_late_bound_vars() {

!pred.has_late_bound_vars probably needs to be replaced with !pred.has_type_flags(TypeFlags::HAS_BINDER_VARS | TypeFlags::HAS_LATE_BOUND)

[compiler-errors]

HAS_LATE_BOUND necessarily implies HAS_BINDER_VARS, but yeah, I will prepare a PR for that soon.

[lqd]

Maybe even just !pred.has_type_flags(TypeFlags::HAS_BINDER_VARS) ? 🤔

In any case, to save 2 minutes for a test, here's a trivial bound that's not using the lifetime and that now trips:

struct S;

fn repro()
    where
        for<'a> S: Clone,
    {
    }

(If the above and Jack's comment sounds right, Michael, I can open a PR)

[compiler-errors]

open a pr @lqd since I am still in bed!!

[lqd]

@compiler-errors working on rust while in Bed, Bath, and Beyond.

[compiler-errors]

Oh right, this needs to be backported to fix a beta regression. This should be backported together with #117637.

[jackh726]

Good call on the crater run here. Nice catch.

[apiraino]

Beta backport accepted as per compiler team on Zulip

@rustbot label +beta-accepted