[ty] Return a tuple spec from the iterator protocol

[dcreager]

This PR updates our iterator protocol machinery to return a tuple spec describing the elements that are returned, instead of a type. That allows us to track heterogeneous iterators more precisely, and consolidates the logic in unpacking and splatting, which are the two places where we can take advantage of that more precise information. (Other iterator consumers, like for loops, have to collapse the iterated elements down to a single type regardless, and we provide a new helper method on TupleSpec to perform that summarization.)

[github-actions]

mypy_primer results

No ecosystem changes detected ✅
No memory usage changes detected ✅

[dcreager]

I moved this logic over into Type::try_iterate, but it seems inconsistent with how we interpret iterating over Never in e.g. a for loop. Before, this would reveal Never:

def _(never: Never):
    for x in never:
        reveal_type(x)  # revealed: Never

i.e., the body is never executed, just like when we iterate over the empty tuple:

for x in ():
    reveal_type(x)  # revealed: Never

This is consistent with simplifying tuple[Never, ...] to tuple[()].

The interpretation described here is that iterating over Never might produce an arbitrary number of elements. That requires changing the for loop test to reveal Unknown.

To me, the interpretation without this special case seems more intuitive, especially when you consider it in terms of the for loop example. @AlexWaygood, you added this special case in #19469. Were there examples in the ecosystem check of indexing into this kind of tuple, which necessitated this special case?

[AlexWaygood]

I think I added the special case in #18987, not #19469 — it was to tackle false positives I saw in the ecosystem report for sympy on early versions of that PR. If you look at the revisions to the primer report in #18987 (comment), the false positives for sympy are present in the penultimate version of the comment

[carljm]

To me the thing that seems wrong in this comment is that we simplify tuple[Never, ...] to tuple[()]. Shouldn't it simplify to Never instead, and then we wouldn't get false positives indexing into it?

[AlexWaygood]

Shouldn't it simplify to Never instead

No because that would imply the type is uninhabited. tuple[Never, ...] is inhabited by the empty tuple (), the same as list[Never] cannot simplify to Never because it is inhabited by the empty list [].

The situation for Never type arguments to homogeneous tuples here is closer to the situation for other generic containers than it is to the situation for heterogeneous tuples

[carljm]

Hmm, that makes sense, but I do wonder if there is any practical benefit to that simplification. It seems like there is a practical cost, because tuple[Never, ...] is a type that can arise in unreachable code, and the indexing errors aren't desirable there. It seems like there might be only upside to just leaving tuple[Never, ...] as-is, instead of simplifying it?

[AlexWaygood]

@dcreager added that particular simplification so I'll defer to him, but I'm a bit reluctant to give up on it. It feels obviously correct from the perspective of set-theoretic types.

But we probably don't need the somewhat unprincipled special case I added here that Doug was asking me about -- a better fix would probably just be to check here whether the node is reachable before emitting the diagnostic:

ruff/crates/ty_python_semantic/src/types/infer.rs

Lines 8421 to 8439 in b605c3e

	(Type::Tuple(tuple_ty), Type::IntLiteral(int), _) if i32::try_from(int).is_ok() => {
	let tuple = tuple_ty.tuple(self.db());
	tuple
	.py_index(
	self.db(),
	i32::try_from(int).expect("checked in branch arm"),
	)
	.unwrap_or_else(|_| {
	report_index_out_of_bounds(
	&self.context,
	"tuple",
	value_node.into(),
	value_ty,
	tuple.len().display_minimum(),
	int,
	);
	Type::unknown()
	})
	}

I think the machinery @sharkdp added should allow us to query that realtively easily?

[carljm]

I think currently in order to query reachability for a node we have to know in advance that that node might need its reachability queried, and proactively store it in semantic indexing. Which has a memory cost if the category of nodes we are adding is large.

[dcreager]

Another possibility would be to special-case that iterating Never gives Never, not tuple[Never, ...] or tuple[Unknown, ...]. That is, it is "not iterable", as opposed to "iterable returning no elements" or "iterable returning unknown elements". That would let us keep the simplification of tuple[Never, ...] to tuple[()], which I agree seems consistent with first principles.

[carljm]

If iterating Never were to emit a "not iterable" diagnostic, then I think we'd have to special-case avoiding that diagnostic in unreachable code.

[dcreager]

But we probably don't need the somewhat unprincipled special case I added here that Doug was asking me about -- a better fix would probably just be to check here whether the node is reachable before emitting the diagnostic:

I've left the Never special case for now, and added a TODO to consider doing this as a replacement. (This also leaves in place the simplification of tuple[Never, ...] to tuple[()].)

[dcreager]

This special case isn't needed any more, since we can now interpret str's iteration behavior in the typeshed correctly.

[carljm]

Looks good!