-
Notifications
You must be signed in to change notification settings - Fork 214
Run-time type checks of protected extension types. #1466
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
Yes, it was not specified in the proposal, but I agree that an implicit downcast from For the implicit type checks, I think we should add a rule: If a type variable Note that we'd need some minimal core of use-site variance in order to be able to create such things as a |
See also https://github.com/dart-lang/language/blob/6a1f57b1684aed351d6ed8f6a637def29b2bb40e/working/1426-extension-types/feature-specification.md#casting-to-a-protected-extension-type, proposing that we use the getter class IntBox { int i; IntBox(this.i); }
protected extension type EvenIntBox on IntBox {
factory EvenIntBox(IntBox intBox) => intBox.i.verifyThis ? intBox : throw 'Error';
bool get verifyThis => this.i.isEven;
void step() => this.i += 2;
}
void main() {
(IntBox(4) as EvenIntBox).step(); // Implicitly invokes `verifyThis`.
} |
True, with a So we probably don't want to make constructors use We might still want to make it really convenient to do it, anyway: class IntBox { int i; IntBox(this.i); }
protected extension type EvenIntBox on IntBox {
bool get verifyThis => this.i.isEven;
factory EvenIntBox(IntBox intBox) {
intBox as EvenIntBox; // Implicitly invokes `verifyThis`.
return intBox;
}
void next() => i += 2;
} We could allow the return statement to have an expression of type protected extension type EvenIntBox {
factory EvenIntBox(IntBox intBox) => intBox as EvenIntBox;
...
} |
This doesn't work. This implies that I'm highly skeptical that you can make this work out. My baseline assumption is that either we box, or we accept that subsumption into object breaks abstraction. Abstraction in languages with abstract types relies on parametric polymorphism, and Dart's polymorphism is not parametric. |
@lrhn Yes, I've thought about trying to move in this direction, but I don't know how to make it work out. And it still doesn't solve the issue, unless you feed this into the generic system. Example:
I don't see any feasible way to enforce abstraction for these in the Dart type system short of making them not be a subtype of Object (and hence entirely outside the existing generic system). |
@tatumizer wrote (about constructors used to create large structures with O(1) work):
For instance, protected extension type TinyJson {
factory TinyJson.mergeTwo(TinyJson left, TinyJson right) => <dynamic>[left, right];
...
} If you insist on always calling a general So we don't want to make it a property of the language that we always call
True, extension constructors are about introducing a new typing for an object which may or may not be new. However, they are (intentionally) marked as I think this is not just tolerable but actually useful: There is no actual wrapper object (unless we With that understanding as the starting point, we still need to remember that there is no wrapper object and the mechanism is purely static, such that we understand that the illusion breaks down if we drop that static type (say, by upcasting to |
@leafpetersen wrote (about casting to a protected extension type):
True. I addressed that in a couple of ways: I mentioned here that we could use use-site variance to handle this: protected extension type E ... {...}
void main() {
var o = ...;
List<exactly E> l = [];
l.add(E(o)); // Will call a version of `add` that does not check the argument type dynamically.
} Also, I mentioned here that I added a discussion section to the proposed feature specification about how we could support dynamic casts in a way where developers are still able to maintain a given computational discipline (that is, checking invariants that the type system can't understand). protected extension type EvenIntBox {
bool get verifyThis => this.i.isEven;
factory EvenIntBox(IntBox intBox) => intBox as EvenIntBox; // Implicitly invokes `verifyThis`.
}
void main() {
var o = ...;
List<Object?> l = <EvenIntBox>[]; // Could be `List<EvenIntBox>`, but this works as well.
l.add(o); // Implicitly invokes `verifyThis`.
}
We can of course choose to accept that. This is quite similar to what we'll get in the proposal if we use a non-protected extension type with constructors: Explicit constructor invocations can be used to enforce the invariant, but dynamic casts will only verify the on-type, no implicit invariant checks are performed. We could then define The benefit offered by a mechanism like |
@lrhn then @leafpetersen wrote:
Use-site invariance would do it. If we don't add use-site invariance then we can still consider the T cast<S, T>(S x) => x as T;
protected extension type E on int {
bool get verifyThis => isEven;
E(int i) => i as E;
}
void main() {
E x = cast<int, E>(4); // `x as T` will implicitly invoke `verifyThis` and succeed.
} You could claim that it is not acceptable to include general computation as a step in a type cast, but the nature of the type cast remains (it occurs at run time, and it may succeed or fail), and I don't think it's unreasonable to say that this is one possible way to integrate the invariant enforcement capability of |
With the The If we have subset types, then we don't need to lock the subset behavior into the I think it's a red herring to think of protected extension types as the way to do subset types, or in terms of it being a subset type. It's just a protected extension type on some other type which prevents arbitrary assignments in either direction, and because of that, |
Maybe it was my idea, and then I'm no longer convinced it was a good idea (or rather, that it was a good idea for that thing). I still think constructors are a good idea for creating values of the extension type from other types, but I'm not sure extension types are the right choice for making subset types. Maybe it's better to make subset types a feature of their own, instead of linking them to extension types and forcing you to write constructors and Trying to make two different use-cases both be matched by the same feature can be worse than having two different features, and then only matching them up when it makes sense, not every time. If the protected extension type doesn't require a subset, assigning from the Not all subsets need new methods, sometimes a string is just a string. Requiring you to introduce a protected extension type for that, with |
(I actually wanted delegation separate from structs, and and then to combine it with structs to get something close to protected extension types). I was thinking of something like this for subset types, a backing type and a boolean function on that. It still makes it impossible to do constant typedef nat = int if (this > 0);
typedef flags = String in {"yes", "no", "maybe"}; We can canonicalize the flag-like constant sets of values, so they actually use subset for subtyping (so |
I think the run-time cost of doing It's never going to be zero-cost to do a type check, but for subset types, you can decide how expensive it's going to be by writing good test. (I don't trust debug-only checks. They're great for debugging things that should never happen, but they provide no actual safety.) |
I'm skeptical that use site invariance is a reasonable solution to this. I believe that it means that there is no way to call a generic add function on a list containing extension type objects:
You have to rewrite all of your generic code that you want to work with extensions to use exactness. Am I missing something? I'm also skeptical that gating this feature on use site invariance is a good idea, but maybe. |
I am intensely skeptical about the idea of adding a virtual method call to every generic type test. We need to be making Dart faster, not slower. |
@leafpetersen I can see the problem with the virtual call. I think it was intended as a non-virtual call here. It only applies to extension types, so it's effectively an extension method invocation, which should be possible to inline where applicable. |
FWIW at least on the VM an expression So if you want you can add custom code into type checking logic for some types (e.g. add an invocation of a Dart method) without penalising all types. |
I'm skeptical about this. I think you're suggesting to fold the custom code into |
Yes, that certainly is a bit of a concern. However I do feel like you could probably still keep a lot of these type checks unaffected via a combination of various optimizations (global flow for type arguments, plus some local static information). That's is not to say that I would be especially thrilled to implement this feature - my position I think aligns with yours is that language features should be trivially always optimisable rather than non-trivially maybe-optimisable. |
@tatumizer It is as you say a question that is hard to really answer in such abstract terms. When we talk about virtual call we should take into account not just the call itself but the effects it has on the surrounding code (disabled optimisations, spilling of live register values, etc). Similarly when we talk about wrapper object allocation, it not the allocation itself that is going to cost a lot (it's 11 instructions on ARM, of which 2 are memory loads and 3 are memory stores), but other factors (are you gonna allocate a lot? how is this wrapper going to flow the program? etc). I think performance is much easier to discuss in more concrete terms. |
@leafpetersen wrote:
If the extension type isn't protected then the run-time representation of the extension type is the on-type, so there won't be any new issues with protected extension type E on int {...}
void add<T>(List<T> l, T x) => l.add(x);
void main() {
List<exactly E> l = [E(3)];
add(l, E(4)); // Could call `verifyThis`.
}
I don't think anyone has proposed a mechanism doing that, the invocation of For any target type which is not a top type, it is statically known that the type test/cast does not have a target which is a protected extension type. Of course, when the target type is a protected extension type then it is statically known that Only one case remains, but it is a very important case: Type tests/casts where the target type is a type variable whose bound is a top type (explicitly or by default). @mraleph noted here that this case, at least on the VM, can be confined in the sense that the extra cost is incurred when the target type is actually a protected extension type, not for any other types. @leafpetersen wrote:
Granted, code cannot be moved up in front of a type test that could invoke a I think the effect on cached outcomes of type tests is important, too. @mkustermann told me that there are several different kinds of caching in the VM for this kind of result. A type test/cast where the target type is statically known to be a protected extension type (like For a type test/cast to a type variable whose bound is a top type the VM would execute In summary, it doesn't look like it would be costly for Dart in general to support invocation of protected extension ... {
bool get verifyThis => bool.fromEnvironment("DEBUG")? realVerification : true;
...
} |
This is not statically determinable in general - therefore every type test against a generic type parameter incurs a virtual method call, exactly as I said.
No, not for generics, that's my entire point.
I can only repeat that I am intensely skeptical of this, and nothing in the discussion you summarize has changed my mind about that. This feature would disable a huge category of optimizations for all generic type tests, which are currently exceptionally pervasive in Dart. |
@leafpetersen wrote:
That is not quite true: The only situation where a However, a type variable whose bound is a top type is so common that it is crucial to ensure that type tests/casts with such a target type is not penalized. In the VM, @mraleph mentioned that the type test in this case amounts to an invocation of
Sorry, I wasn't quite precise there, but a few lines further down I said the following, which gives a hint that I meant 'not a top type, and not a type variable whose bound is a top type':
However, on the VM, the expression
The most serious issue I've noted is caching: The VM caches the outcome of type tests in a number of ways, and there will probably be a need to take one extra step during a type test/cast where the target type is a protected extension type in order to ensure that the result is not cached (because we can't assume that It would be good to know more about the actual impact of missed optimizations. The support for Also, it prevents removal of expressions of the form Do you really think this amounts to a huge cost? |
I don't know what you mean by caching here. My point is that unless global analysis is able to prove that for an expression
I don't know. I suspect that it is not a huge cost for global compilers. I am confident that there is some cost, even if only in the form of opportunity cost for optimizations that we do not yet do. The key point is that the cost is not pay as you go. I'm fine with adding features which are themselves slow, if they provide corresponding value. I have an extremely high bar for adding features which slow down unrelated parts of the program. These kind of features are death by a thousand cuts. They add up, and you can't avoid them by not using the feature. As soon as anyone anywhere in your import graph uses a protected extension type, everything gets a little slower unless compiler magic is successful, and I've been working on compilers for almost 30 years now and have a healthy respect for the limitations of compiler magic (amazing as it can be!). And all of the above is specific to the caveat "global compiler". For a modular compiler, there is no global analysis to help here - everything just gets slower (or bigger, if you speculatively duplicate the code in advance). We don't currently rely on modular compilation for production, but it's not at all clear to me that that is always going to be the case. |
@leafpetersen wrote:
@mkustermann told me that the outcome of a test like
Indeed. However, I don't see why we would refer to a global analysis in order to conclude that
The target type It would be possible in some cases for a global analysis to prove that a given type variable without a bound will never have a value which is a protected extension type, but I would expect that kind of property to be so fragile that it should basically be ignored. Especially for classes like (Implicitly induced type casts caused by dynamically checked covariance or covariant parameters would not be relevant, because they are performed at the very beginning of a function body, and it doesn't improve performance to move any code up in front of such casts. Also, we can't move any code with effects across an action that might throw.) In order to get an impression of the number of locations I looked through the code of
|
Yes, that is the case that I'm referring to.
This is not true. Inlining moves the checks into enclosing scopes, in which all of the optimizations that I describe apply. |
@leafpetersen wrote:
Ah, of course! But it is still not correct to move code with effects across a type cast, and I can't see how that situation would be worse if the type cast can invoke class C<X> {
void m(X x) {}
}
var j = 0;
void main() {
int Function() f(C<Object> c) {
var y = 1;
try {
do {
c.m(Object()); // `dart2js` does inline this as a mere cast, `t1._as(new P.Object());`.
y = 2; // Even an assignment to a local variable can be observable after throwing.
} while (++j <= 10);
} catch (_) {}
return () => y;
}
g = f(C<Object>()); // The cast doesn't throw.
print(g()); // '2'
var g = f(C<int>()); // The cast throws.
print(g()); // '1', would be '2' if we move `y = 2` across the cast.
} So we cannot move/hoist expressions that may have effects across a type cast, and this limitation applies for both regular type casts and type casts that may invoke We could evaluate So the loss of optimization opportunities associated with type casts basically amounts to hoisting an expression |
@eernstg I have to admit I'm finding this discussion fairly frustrating. I described very concretely the optimizations that do apply here, and it is trivial to construct programs right now where the VM does in fact do the optimizations that I am describing, and for which it would be invalid to do so were this proposal accepted. To reiterate: For an instance check (and this is not an exhaustive list):
For a cast (and this is not an exhaustive list):
None of these need be specialized optimizations targeting casts. They are simply the natural optimizations that fall out when you apply LICM, CSE, tail merging, PRE, etc to operations with statically known effect sets. This proposal moves instance checks from the most optimizable effect category (dataflow) to the worst (all effects), and casts from the second most optimizable effect category (idempotent effects) to the worst (all effects). Per your example, looking at dart2js optimization is never going to be useful here - dart2js does few optimizations around casts, because they are elided in prod (cc @rakudrama ). I doubt the VM will optimize your example either (@alexmarkov can correct me if I'm wrong), but in principle this is a fairly easy LICM opportunity if the IR has the right level of granularity. Specifically, your inner loop: do {
c.m(Object());
y = 2;
} while (++j <= 10); can be lowered + inlined to: do {
if (!isSubtype(Object, T) throw CastError;
y = 2;
} while (++j <= 10); Note that both if (!isSubtype(Object, T) throw CastError
do {
y = 2;
} while (++j <= 10); I don't know if our native compilers represent this at the level of granularity necessary to separate out the type test from the allocation, but they could, and might in the future. This optimization would be invalid under this proposal. Here is another simpler example: class Pair<T> {
String toString() => "<$first, $second>";
T first;
T second;
Pair(this.first, this.second);
}
void setTo(Pair<Object?> pair, Object? fill) {
pair.first = fill;
pair.second = fill;
return;
} Suitably wrapped to make sure that this code is in fact runtime polymorphic and is not inlined, the VM nonetheless successfully eliminates the second dominated implicit cast. This would not be possible under this proposal (again, except by relying on global analysis to ensure that this feature was not used). |
Taking this from the main discussion.
The current proposal disallows
is E
andas E
whereE
is a protected extension type.However, there are places in the language semantics where implicit
is
/as
checks are performed:dynamic
.I believe downcast from dynamic is already disallowed,
E e = (o as dynamic);
does not introduce an implicit downcast, it's just an error.The other two are harder to avoid. Example:
So, what should the run-time behavior of
List<Ext>.add(o)
be? Should it do anis
test like otherOr should we just make extension types completely invariant when used as type arguments? Then a
List<Ext>
is-not aList<Object?>
or evenList<Ext?>
, even thoughExt
is-anObject?
and anExt?
. (Does that make sense at all, to make the variance depend on the type argument, not the type constructor?)If we do so, then a
List<Ext>
is always an exact type (at the argument type at least), and we can skip the covariant type checks inadd
, etc.The text was updated successfully, but these errors were encountered: