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

Unnecessary checks on declared-covariant parameters #35317

Open
sjindel-google opened this Issue Dec 4, 2018 · 2 comments

Comments

Projects
None yet
4 participants
@sjindel-google
Contributor

sjindel-google commented Dec 4, 2018

Consider this (simplified) example:

class RenderObject {}
class RenderClipPath extends RenderObject {}
class RenderCustomPaint extends RenderObject {}

class RenderObjectWidget {
  void didUnmountRenderObject(covariant RenderObject _) {}
}

class ClipPath extends RenderObjectWidget {
  void didUnmountRenderObject(RenderClipPath _) {}
}

class CustomPaint extends RenderObjectWidget {
  void didUnmountRenderObject(RenderCustomPaint _) {}
}

RenderObjectWidget.didUnmountRenderObject will have a type-check for it's argument even though it provably cannot fail. In general, if a parameter is declared covariant on a method which is not seen in the interface of any supertypes of its enclosing class, it doesn't need a type-check.

I'm working on fixing this on the VM-site, but the CFE should distinguish between parameters labeled "covariant" by the user and parameters which need to be checked due to declared-covariance.

@mraleph

@lrhn

This comment has been minimized.

Member

lrhn commented Dec 5, 2018

Why can't it fail?

dynamic o = RenderObjectWidget();
void Function(Object) f = o.didUnmountRenderObject;  // Success!
f(Object());  // Should catch type error.
@eernstg

This comment has been minimized.

Member

eernstg commented Dec 5, 2018

You're right that a tear-off operation must always return a function object that performs the checks (and that's true for any implementation of an instance method with one or more covariant parameters, not just the special case of interest here), because a torn off instance method can always be assigned to a variable whose type is a function type where every covariant parameter of that function object has declared type Object.

But consider the special case of a statically checked invocation where the actual arguments corresponding to parameters that are statically known to be covariant have a type that is a subtype of the statically known type of the corresponding parameter (that is, consider a "safe invocation"): In that situation the dynamically required parameter type is exactly the same as the statically known parameter type (possibly plugging in the values of type variables in scope, but the static subtype relation is sound no matter which values they have), and this means that the dynamic "covariance check" on the type of the actual argument is guaranteed to succeed.

I don't know how important this optimization opportunity would be, but I do believe that it is applicable for that special case.

If so, then it could be worthwhile to have an entry point with no covariance checks that could be used for safe invocations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment