Skip to content

Correct rule about Object/Object? being a superinterface #3402

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

Closed
wants to merge 2 commits into from

Conversation

eernstg
Copy link
Member

@eernstg eernstg commented Oct 16, 2023

The existing rule about Object or Object? being a superinterface of an extension type was too restrictive. One case which shows that it needs to be broadened is this issue: dart-lang/co19#2313.

This PR changes the rule to include a greater set of cases, thus ensuring that Object or Object? is actually found in the superinterface graph of an extension type in all cases.

It could be claimed that the broader rule introduced by this PR is surprisingly aggressive (we will add an edge in the superinterface graph from every single extension type to Object or to Object?, depending on whether the representation type is non-nullable or not). However, we do need to have this edge in some cases, and it is benign to have the edge even in cases where there is already a path to Object/Object? via other superinterfaces.

`Object?` or `Object` as a direct superinterface, according to the subtype
relation which was specified earlier.
_DV_ has `Object?` or `Object` as a direct superinterface, according to the
subtype relation which was specified earlier.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have at least two worries her.

First the practical: It's potentially a mistake to make Object be a direct superinterface.
Example:

class Bad {
  bool operator ==(covariant Bad other) => something;
}
extension type MoreBad(Bad _) implements Bad {
  /// ...
}
void main() {
  MoreBad(Bad()) == 1; // No error

By making Object a direct superinterface of MoreBad, the member signature of operator== should be the combined member signatures of Bad and Object, which is bool operator==(covariant Object).
That's probably not intended.

(I see the behavior above today if I add , Object to the implements list. Adding things to implements lists is not a no-op, even if the type already implements that interface.)

If being a direct superinterface does not mean that the member signature should be included in the combined member signature, it needs to be stated very, very clearly. And probably should just not use "direct superinterface" at all.

So, maybe don't include Object or Object? if not necesary. Something like:

If DV does not include an <interfaces> clause, then DV has Object as direct superinterface
if the declared representation representation type is non-nullable (a subtype of Object),
otherwise it has Object? as direct superinterface.
If DV does include an <interfaces> clause, then it has Object as direct superinterface if
the declared representation representation type is non-nullable, and non of the declared
superinterfaces from the <interfaces> clause have Object as superinterface.

That is, we add a direct superinterface only if it's needed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, that's certainly something to think about!

We do have a very similar outcome with non-extension types:

class A { foo(num n) {} }
class B1 { foo(covariant int i) {} }
abstract class B2 { foo(num n); }
class C extends B1 implements B2 {}

void main() {
  var b = C();
  b.foo(1.5); // No compile-time error, but throws.
}

The most specific function type wins when we're computing the combined member signature, even though it may seem more useful for developers who actually want to call the given member if they'd get the signature with the strongest constraints (or the signature which is actually implemented). Anyway, we had some discussions about this several years ago (can't find the exact issue), and the unconditional choice of the most specific function type seems to work well enough in practice.

If being a direct superinterface does not mean that the member signature should be included in the combined member signature, it needs to be stated very, very clearly.

I don't think we should go there. I also think that non-standard signatures for the Object members is a rare thing in practice.

So, maybe don't include Object or Object? if not necesary.

Wouldn't that be even more confusing?

Copy link
Member Author

@eernstg eernstg Oct 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does get more complex, but we could add a case like this:

No warning is emitted for a downcast from an extension type E1 to another extension type E2 in the case where the instantiated representation type of E1 differs from the instantiated representation type of E2.

The point is that when there is a non-trivial subtype relationship from one to the other representation type then the test/cast/match of E2 amounts to an actual verification of something which is associated with E2, and we have already accepted that it is appropriate for the given object to have the type E1, and the developer of E2 has declared that there is a subtype relationship from E1 to E2, so it's fair to say that this is sufficient to conclude that it is appropriate for that object to have the type E2.

Nevertheless, we do need to keep the amount of special-casing under control. The above rule is a very good fit for Opt<T>, but the question is how typical that example is, and whether it's going to be typical that we want this special kind of test/cast/match.

Anyway, we also considered the possibility that an extension type can declare that it is 'open' by means of a metadata annotation:

@openExtensionType
extension type Foo(int it) {
  // For some reason, we don't want to export the interface of int,
  // so we don't have `implements int`. But we _do_ want to allow
  // casting into this extension type: "Absolutely any `int` will do".
  ... // Members.
}

void main() {
  var foo = Foo(42);
  int i = foo; // Compile-time error, not a subtype.
  foo = i as Foo; // OK, no lint, `Foo` is open.
  var xs = [1, 2, 3];
  var ys = xs as List<Foo>; // OK, no lint.
}

The lint would treat type tests/casts/matches on open extension types like regular type tests/casts/matches on the representation type.

@eernstg eernstg force-pushed the spec_fix_Object_superinterface_oct23 branch from ef95af0 to 0e01ef7 Compare October 16, 2023 16:54
@eernstg
Copy link
Member Author

eernstg commented Oct 16, 2023

PTAL!

I corrected the error about "this declaration has that superinterface".

However, I did not change the superinterface graph structure (that is, the text still says that every extension type has Object or Object? as a direct superinterface, even in the cases where the given superinterface can already be reached via some other path in the superinterface graph).

I do think it's a surprising property of extension types, but I also think the very small and very well-known set of Object members will not create much confusion by having this extra superinterface, and if it does affect anyone at all then it is probably more confusing if the existence of this direct superinterface depends on the entire superinterface graph.

Note that the disruption of the member signatures will basically only affect operator == and noSuchMethod, and only in the case where the parameter is made covariant, and that's very, very unlikely to be useful with noSuchMethod.

The getters are not affected, because the combined member signature will be the getter with the most special return type, which is surely also what we'd expect and want.

So operator == with a covariant parameter is the only case that could create real confusion, and this is the same kind of confusion that we already have with covariant member signatures (the non-covariant one generally wins).

@eernstg
Copy link
Member Author

eernstg commented Oct 25, 2023

Abandoned: #3415 solves several tricky issues, and in particular this update is obsolete given that the language team has accepted the proposal in #3415.

@eernstg eernstg closed this Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants