Cycle error through variance computation when using -> _.

[eddyb]

Reduced example (playground):

struct Wrapper<T>(T);

fn foo<T>(x: Wrapper<T>) -> _ {
    drop(x);
}

error[E0391]: cycle detected when processing `foo`
 --> src/lib.rs:3:1
  |
3 | fn foo<T>(x: Wrapper<T>) -> _ {
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
note: ...which requires type-checking `foo`...
 --> src/lib.rs:3:1
  |
3 | fn foo<T>(x: Wrapper<T>) -> _ {
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires processing `Wrapper`...
 --> src/lib.rs:1:1
  |
1 | struct Wrapper<T>(T);
  | ^^^^^^^^^^^^^^^^^^^^^
  = note: ...which requires computing the variances for items in this crate...
  = note: ...which again requires processing `foo`, completing the cycle
note: cycle used when collecting item types in top-level module
 --> src/lib.rs:1:1
  |
1 | / struct Wrapper<T>(T);
2 | |
3 | | fn foo<T>(x: Wrapper<T>) -> _ {
4 | |     drop(x);
5 | | }
  | |_^

You can see right away that it's hard to figure out what "processing" means and we should probably start a cleanup effort to add proper descriptions to queries (or at least name the query in the default message, even if it might not look pretty at all). cc @rust-lang/compiler

---

Using -Ztreat-err-as-bug + RUST_BACKTRACE=1 I can turn the error into an ICE and get this:

#0 [crate_variances] computing the variances for items in this crate
#1 [variances_of] processing `Wrapper`
#2 [typeck_tables_of] type-checking `foo`
#3 [fn_sig] processing `foo`
#4 [collect_mod_item_types] collecting item types in top-level module
#5 [analysis] running analysis passes on this crate

So variance computation uses fn_sig (not sure why it would need to), which is normally harmless, as function signatures are fully explicit.

But we nowadays "allow" _ in the signature, guaranteeing an error still, while showing the user the inferred type (this should be useful during prototyping but I keep forgetting it exists).

As a result of that, instead of typeck_tables_of depending on fn_sig, fn_sig depends on typeck_tables_of (since it needs the results of type inference from the fn body), so an otherwise harmless fn_sig turns into a cycle as soon as type-checking needs any variances.

[matthewjasper]

we should probably start a cleanup effort to add proper descriptions to queries

#72707 had to do this to because I couldn't get the default impl to work with min_specialization

[eddyb]

@matthewjasper Can you split that part out and PM me on Discord/Zulip when you do?
Seems easier to review than specialization as a whole.

[eddyb]

I think one of the things we could also do to improve this kind of query cycle error is to replace:

 --> src/lib.rs:3:1
  |
3 | fn foo<T>(x: Wrapper<T>) -> _ {
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

with

 --> src/lib.rs:3:29
  |
3 | fn foo<T>(x: Wrapper<T>) -> _ {
  |                             ^

That's because we know why we queried typeck_tables_of(foo), it's the _.

So we can just change this line (i.e. by adding .at(ty.span)):

rust/src/librustc_typeck/collect.rs

Line 1474 in 4b1f86a

let fn_sig = tcx.typeck_tables_of(def_id).liberated_fn_sigs()[hir_id];

         let fn_sig = tcx.at(ty.span).typeck_tables_of(def_id).liberated_fn_sigs()[hir_id];

[eddyb]

Since I'm looking through typeck, thought I'd take a look at the underlying cause:

rust/src/librustc_typeck/variance/constraints.rs

Lines 160 to 162 in 4b1f86a

	ty::FnDef(..) => {
	self.add_constraints_from_sig(current_item, tcx.fn_sig(def_id), self.covariant);
	}

I thought that was unused/vestigial, but it does seem to be used when relating "function item" types (relate_item_substs uses variance):

rust/src/librustc_middle/ty/relate.rs

Lines 471 to 476 in 4b1f86a

	(&ty::FnDef(a_def_id, a_substs), &ty::FnDef(b_def_id, b_substs))
	if a_def_id == b_def_id =>
	{
	let substs = relation.relate_item_substs(a_def_id, a_substs, b_substs)?;
	Ok(tcx.mk_fn_def(a_def_id, substs))
	}

---

So if Wrapper<T> is invariant wrt T, then typeof(foo::<&'a T>) is invariant over 'a, huh.
(Note that this isn't about the function pointer which you can get through casting/coercion, and which has its own variance rules, but the zero-sized unit type that represents the function itself, statically)

That entire behavior seems vestigial, I wonder what we can do backwards-compatibly.

The easiest thing to do would be to make it invariant. It might even work out.
The most compatible thing to do would be to make it bivariant (since the generic parameters only matter for call anyway), but I'm not sure we support that sort of thing anymore.

Nominating for discussion (on the topic of variance of "function item" types).

[nikomatsakis]

I sort of remember that you added in the variance to enable backwards compatibility of FnDef.

I'm not sure what you mean by the "most compatible thing would be bivariant", in part because "bivariant" is perhaps not that well-defined.

I think we want to maintain the invariant (no pun intended) that if you have a given fn-def type F, no matter how you upcast it, it will always produce the same function pointer, and of course that function pointer should never be invoked with arguments it doesn't expect. Clearly the current rules achieve that, as would invariance, but I don't think that bivariance would do so, unless I'm missing something (in particular it would fail the second criteria, right?).

[spastorino]

Unnominating this as it was already discussed in our last T-compiler meeting.