#2217. Add tests for calculating of the least upper bound for extension types #2223

sgrekhov · 2023-08-21T13:33:20Z

@eernstg if you believe that we need more tests, please let me know

…r extension types

eernstg

It looks like this test expects no errors in a case where we must have an error.

LanguageFeatures/Extension-types/upper_bound_A01_t05.dart

eernstg

There are a number of cases where the UP type function yields a result which is different from that which seems to be expected by the test, mainly because UP(S, T) == T when S <: T.

We should also have some tests where the compute UP(E, T) where E is an extension type and T is a non-extension type, and vice versa.

LanguageFeatures/Extension-types/upper_bound_A01_t05.dart

LanguageFeatures/Extension-types/upper_bound_A01_t01.dart

LanguageFeatures/Extension-types/upper_bound_A01_t04.dart

sgrekhov

Tests updated. Please take another look

LanguageFeatures/Extension-types/upper_bound_A01_t01.dart

LanguageFeatures/Extension-types/upper_bound_A01_t04.dart

LanguageFeatures/Extension-types/upper_bound_A01_t05.dart

eernstg

A few more comments.

eernstg · 2023-08-22T10:01:27Z

LanguageFeatures/Extension-types/upper_bound_A01_t06.dart

+  v2.expectStaticType<Exactly<String>>();
+
+  var v3 = 2 > 1 ? "String" : ET3("ET3");
+  v3.expectStaticType<Exactly<String?>>();


Please double check this one, it looks like it should be String, not String?.

Seems so. First I thought that implementing ET1<String?> makes LUB to be String?.

I don't really think we have any shortcuts: We need to check first whether there is a subtype relationship (to detect that we've using the rule UP(S, T) == T when S <: T, or UP(S, T) == S when T <: S), and then we need to consider the Dart 1 algorithm, and for that we need the superinterface graph. Then we need to find the common supertypes, organize them into depth buckets, and then take the deepest singleton. And String? just doesn't occur in that superinterface graph (no types of the form T? do, other than Object?).

LanguageFeatures/Extension-types/upper_bound_A01_t06.dart

LanguageFeatures/Extension-types/upper_bound_A01_t04.dart

LanguageFeatures/Extension-types/upper_bound_A01_t05.dart

eernstg · 2023-08-22T10:21:27Z

LanguageFeatures/Extension-types/upper_bound_A01_t05.dart

+  v5.expectStaticType<Exactly<ET1<String?>>>();
+
+  var v6 = 2 > 1 ? ET2<num>(4) : ET4(5);
+  v6.expectStaticType<Exactly<ET2<num?>>>();


(Github doesn't want to show the comment on this line, but it should be visible in the 'conversation' tab as 'obsolete'.) I think it should be Object.

Very detailed, thank you. But the scheme on conversation tab shows Object?, not Object. So I set Object? here. Please correct me if I'm wrong

(I think this thread has been superseded by other threads, please comment again if I'm wrong.)

sgrekhov

Updated

LanguageFeatures/Extension-types/upper_bound_A01_t04.dart

sgrekhov · 2023-08-22T13:50:54Z

LanguageFeatures/Extension-types/upper_bound_A01_t05.dart

+  v5.expectStaticType<Exactly<ET1<String?>>>();
+
+  var v6 = 2 > 1 ? ET2<num>(4) : ET4(5);
+  v6.expectStaticType<Exactly<ET2<num?>>>();


Very detailed, thank you. But the scheme on conversation tab shows Object?, not Object. So I set Object? here. Please correct me if I'm wrong

LanguageFeatures/Extension-types/upper_bound_A01_t05.dart

sgrekhov · 2023-08-22T14:40:26Z

LanguageFeatures/Extension-types/upper_bound_A01_t06.dart

+  v2.expectStaticType<Exactly<String>>();
+
+  var v3 = 2 > 1 ? "String" : ET3("ET3");
+  v3.expectStaticType<Exactly<String?>>();


Seems so. First I thought that implementing ET1<String?> makes LUB to be String?.

LanguageFeatures/Extension-types/upper_bound_A01_t06.dart

eernstg

A couple of comments.

eernstg · 2023-08-23T07:49:08Z

LanguageFeatures/Extension-types/upper_bound_A01_t06.dart

+
+main() {
+  var v1 = 2 > 1 ? ET2("ET2") : ET3("ET3");
+  v1.expectStaticType<Exactly<String?>>();


Can't be String? -- shared superinterfaces are String, Comparable<String>, Object, Object?, and String wins.

sgrekhov · 2023-08-23T08:04:48Z

LanguageFeatures/Extension-types/upper_bound_A01_t05.dart

+extension type ET1<T>(T id) {}
+extension type ET2<T>(T id) {}
+
+extension type ET3<T extends String>(T id) implements ET1<T>, String {}


@eernstg I'm getting an error in a freshly-built analyzer here

COMPILE_TIME_ERROR.CONFLICTING_GENERIC_INTERFACES The extension type 'ET3' can't implement both 'Object?' and 'Object' because the type arguments are different.

Is this a valid error?

Immediately sounds wrong. But let's think about it. ;-)

There is nothing wrong in being a subtype of Object and also a subtype of Object? (almost everything is), and those two types do not satisfy the requirement that usually justifies words like "the type arguments are different" (there are no type arguments at all).

It sounds like the implementation of the analyzer is running code where it is assumed that we have both C<T1> and C<T2> as superinterfaces of a given interface type, where C is some generic type and T1 and T2 are not subtypes of each other, even though we don't have that situation at all here. But somehow finding Object? as well as Object in the superinterface graph causes the confusion.

So it is not a valid error.

sgrekhov · 2023-08-23T08:43:56Z

Thank you. Updated. But if this error is valid? There are more similar error in these tests

eernstg · 2023-08-23T09:01:07Z

LanguageFeatures/Extension-types/upper_bound_A01_t05.dart

+extension type ET1<T>(T id) {}
+extension type ET2<T>(T id) {}
+
+extension type ET3<T extends String>(T id) implements ET1<T>, String {}


Immediately sounds wrong. But let's think about it. ;-)

There is nothing wrong in being a subtype of Object and also a subtype of Object? (almost everything is), and those two types do not satisfy the requirement that usually justifies words like "the type arguments are different" (there are no type arguments at all).

It sounds like the implementation of the analyzer is running code where it is assumed that we have both C<T1> and C<T2> as superinterfaces of a given interface type, where C is some generic type and T1 and T2 are not subtypes of each other, even though we don't have that situation at all here. But somehow finding Object? as well as Object in the superinterface graph causes the confusion.

So it is not a valid error.

eernstg · 2023-08-23T09:01:55Z

LanguageFeatures/Extension-types/upper_bound_A01_t05.dart

+  v5.expectStaticType<Exactly<ET1<String?>>>();
+
+  var v6 = 2 > 1 ? ET2<num>(4) : ET4(5);
+  v6.expectStaticType<Exactly<Object>>();


eernstg · 2023-08-23T09:06:46Z

LanguageFeatures/Extension-types/upper_bound_A01_t06.dart

+  v2.expectStaticType<Exactly<String>>();
+
+  var v3 = 2 > 1 ? "String" : ET3("ET3");
+  v3.expectStaticType<Exactly<String?>>();


I don't really think we have any shortcuts: We need to check first whether there is a subtype relationship (to detect that we've using the rule UP(S, T) == T when S <: T, or UP(S, T) == S when T <: S), and then we need to consider the Dart 1 algorithm, and for that we need the superinterface graph. Then we need to find the common supertypes, organize them into depth buckets, and then take the deepest singleton. And String? just doesn't occur in that superinterface graph (no types of the form T? do, other than Object?).

eernstg

LGTM!

I'll land this now; we can have another PR if something hasn't been resolved.

2023-08-25 sgrekhov22@gmail.com Fixes dart-lang/co19#2236. Expect a secondary error for CFE in syntax_A08_t03.dart (dart-lang/co19#2237) 2023-08-24 sgrekhov22@gmail.com dart-lang/co19#2232. Fix secondary errors in not_a_constant_in_superclass_t02.dart (dart-lang/co19#2233) 2023-08-24 sgrekhov22@gmail.com Fixes dart-lang/co19#2230. Fix roll failures (dart-lang/co19#2231) 2023-08-23 sgrekhov22@gmail.com Fixes dart-lang/co19#2228. Fix syntax error in static_analysis_extension_types_A10_t08.dart (dart-lang/co19#2229) 2023-08-23 sgrekhov22@gmail.com dart-lang/co19#1400. Extension types subtyping tests. Resolve name conflict, add missing tests (dart-lang/co19#2227) 2023-08-23 sgrekhov22@gmail.com dart-lang/co19#1400. Add tests for external members, covariant parameters, nullable supertypes (dart-lang/co19#2225) 2023-08-23 sgrekhov22@gmail.com dart-lang/co19#2217. Add tests for calculating of the least upper bound for extension types (dart-lang/co19#2223) 2023-08-22 sgrekhov22@gmail.com Fixes dart-lang/co19#2224. Remove excessive error expectation for CFE (dart-lang/co19#2226) 2023-08-21 sgrekhov22@gmail.com Fixes dart-lang/co19#2221. Replace inline classes by extension types (dart-lang/co19#2222) 2023-08-21 sgrekhov22@gmail.com Fixes dart-lang/co19#2219. Replace `inline class` by `extension type` in test description (dart-lang/co19#2220) 2023-08-18 sgrekhov22@gmail.com dart-lang/co19#2142. Add Subtyping tests for extension types (dart-lang/co19#2215) 2023-08-18 sgrekhov22@gmail.com Fixes dart-lang/co19#2216. Add test for extension type abstract members (dart-lang/co19#2218) 2023-08-17 sgrekhov22@gmail.com Fixes dart-lang/co19#2213. Fix stack trace comparison (dart-lang/co19#2214) Change-Id: I009f43878130f934fc6c2eea3368f9763c94e7a5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/322660 Reviewed-by: Alexander Thomas <athom@google.com>

dart-lang#2217. Add tests for calculating of the least upper bound fo…

67cb664

…r extension types

sgrekhov requested review from chloestefantsova and eernstg August 21, 2023 13:33

Fix syntax error

0ae502d

sgrekhov mentioned this pull request Aug 21, 2023

[extension-types] Add rules about non-extension types dart-lang/language#3276

Merged

eernstg reviewed Aug 21, 2023

View reviewed changes

LanguageFeatures/Extension-types/upper_bound_A01_t05.dart Outdated Show resolved Hide resolved

Fix syntax error 2

9231ee7

sgrekhov requested a review from eernstg August 21, 2023 14:04

eernstg reviewed Aug 21, 2023

View reviewed changes

Implement review recommendations

d1825c5

sgrekhov commented Aug 22, 2023

View reviewed changes

sgrekhov requested a review from eernstg August 22, 2023 08:22

eernstg reviewed Aug 22, 2023

View reviewed changes

Fix tests according to review

3dc86d9

sgrekhov commented Aug 22, 2023

View reviewed changes

eernstg reviewed Aug 23, 2023

View reviewed changes

sgrekhov commented Aug 23, 2023

View reviewed changes

sgrekhov added 2 commits August 23, 2023 11:40

Fix wrong expectations and typos

a2c9eec

Fix typo

8026076

sgrekhov requested a review from eernstg August 23, 2023 08:44

eernstg reviewed Aug 23, 2023

View reviewed changes

eernstg approved these changes Aug 23, 2023

View reviewed changes

eernstg merged commit 4be0eaf into dart-lang:master Aug 23, 2023

sgrekhov deleted the co19-2217 branch November 14, 2023 12:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#2217. Add tests for calculating of the least upper bound for extension types #2223

#2217. Add tests for calculating of the least upper bound for extension types #2223

sgrekhov commented Aug 21, 2023

eernstg left a comment

eernstg left a comment

sgrekhov left a comment

eernstg left a comment

eernstg Aug 22, 2023

sgrekhov Aug 22, 2023

eernstg Aug 23, 2023

eernstg Aug 22, 2023

sgrekhov Aug 22, 2023

eernstg Aug 23, 2023

sgrekhov left a comment

sgrekhov Aug 22, 2023

sgrekhov Aug 22, 2023

eernstg left a comment

eernstg Aug 23, 2023

sgrekhov Aug 23, 2023

eernstg Aug 23, 2023

sgrekhov commented Aug 23, 2023

eernstg Aug 23, 2023

eernstg Aug 23, 2023

eernstg Aug 23, 2023

eernstg left a comment

#2217. Add tests for calculating of the least upper bound for extension types #2223

#2217. Add tests for calculating of the least upper bound for extension types #2223

Conversation

sgrekhov commented Aug 21, 2023

eernstg left a comment

Choose a reason for hiding this comment

eernstg left a comment

Choose a reason for hiding this comment

sgrekhov left a comment

Choose a reason for hiding this comment

eernstg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgrekhov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eernstg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgrekhov commented Aug 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eernstg left a comment

Choose a reason for hiding this comment