-
Notifications
You must be signed in to change notification settings - Fork 213
Should we restrict cycles of F-bounded types through union types? #433
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Since So, there should be no harm in disallowing those cases. Is there harm in allowing them? That is, can we recognize that these are meaningless bounds that are trivially satisfied by any type, and just ignore them? If we have to detect them anyway to make them errors, it seems equally possible to ignore them. The safe bet is to disallow. I'll defer to your experience here, so do whatever you find best. It shouldn't affect anybody negatively. (The moment we get union types, we can write recursive equations with type variables, like this one that will occur alomost immediately: |
@leafpetersen wrote:
Any F-bounded type variable bound is a recursive equation on types, so maybe this is no worse than what we already have, and we should allow it for consistency? Such a type variable bound just works in cases where explicit type arguments are given and must simply be checked (it's easy to check whether It is actually a tiny inconsistency that we've made So maybe we should simply use the criterion that it is an error to specify a bound (Bounds of the form The graph-and-cycle based check would certainly flag some of the same cases, and the two approaches might be equivalent. In any case, my inclination is also yes. (And I think almost anything would count as a reason for not introducing recursive union types ;-) |
This is not planned. |
@scheglov points out that allowing these causes obvious termination problems with the subtyping algorithm. Specifically, given (for example)
|
I'd think we'd be happy to implement these rules as analyzer warnings. Then Google internal can be clean, and the way would be paved to work on language language to restrict bounds. |
[Edit Oct 16: Using the terminology mentioned in comments below] Here's a proposal for a compile-time error which could be used to get rid of these bounds. It is probably equivalent to the graph based approach in the original posting, but might be seen as a simpler way to say it:
In other words, "replace the unions by their base type and then check for cycles". I think it's about time to proceed and make these semi-cyclic bound structures a compile-time error, thus avoiding some infinite-loop failures in tools. import 'dart:async';
void foo<X extends FutureOr<X>>(X? x) {}
void bar<X extends FutureOr<X>?>(X x) => foo(x); An extra reason why we'd want to do this now is that 'inference-using-bounds' seems to introduce some new situations where the infinite looping occurs. @dart-lang/language-team, WDYT? |
Seems reasonable. I don't see any use for a self-referential type variable bound. It's trivially satisfied by any type, so it makes no difference. We can accept it, but that wouldn't help anyone write better code. About phrasing, I always gets worried when we "erase a union" and choose one of the sides as the canonical one. The other side is equally part of the union, and if we ever get full union type (deity forbid) then we need to be general. In this case, we're not erasing unions, we're looking for an plain type variable, which means we don't need to look at So maybe:
Could we just define it as:
(Not shorter, that's for sure. Maybe just use "erase union", but call it something else.) |
I prefer the formulation (and terminology) from @eernstg I think. If and when we need to generalize it, we can worry about that then (I think the graph formulation generalizes fine, for example). I'm ambivalent as to whether to specify it using the graph formulation from my original comment or using the union reduction approach. Fine with either. |
If we call it "union base type" instead of "union reduction", and/or avoid saying "erase union", I'm fine with it too. It might be enough to just say "reduce the union type" instead of "erase. ..". This really is just nit-picking phrasing. The approach works. |
Sounds good! I adjusted the terminology. |
We don't allow type variable declarations of the form
T extends T
, orT extends S, S extends T
. The analyzer currently accepts type variable declarations of the formT extends FutureOr<T>
, but the CFE crashes on such declarations. With NNBD, we will also have the ability to writeT extends T?
orT extends S, S extends T?
unless we specify otherwise.Should we consider making these an error?
My inclination is yes. In general, this feels to be pushing up against (or crossing) an important boundary in the equational theory that may bite us if we eventually add union types and attempt to re-interpret
?
andFutureOr
as union types. Note that for exampleT extends T | Never
is essentially another way of writingT extends T
, which suggests the kinds of problems that may arise.I also haven't found any uses for this kind of equation, and since the CFE doesn't accept the
FutureOr
example, there is almost certainly no existing code that uses this feature.One way of specifying this is to say that given a set of type variable declarations of the form
X0 extends B0, ..., Xn extends Bn
, construct a graph with one node for everyXi
, and edges fromXi
toXj
iffXi
occurs in a top level union inBj
, and say that it is an error if that graph has any cycles. We say thatXi
occurs in a top level union in a typeT
iff:T
is `XiT
isS?
andXi
occurs in a top level union inS
T
isFutureOr<S>
andXi
occurs in a top level union inS
@eernstg @lrhn @munificent thoughts?
cc @stereotype441
The text was updated successfully, but these errors were encountered: