Skip to content
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

Dart does not throw compile error when declare a variable of the type which is not well bounded #36959

Closed
iarkh opened this issue May 14, 2019 · 9 comments
Assignees
Labels
analyzer-spec Issues with the analyzer's implementation of the language spec area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@iarkh
Copy link
Contributor

iarkh commented May 14, 2019

Dart SDK Version: 2.3.1-dev.0.0
OS: Windows 10 64 bit

Analyzer throws compile error for the following source code and seems like this is an expected behavior:

class A<X extends A<X>> {}
typedef AAlias<T extends A<T>> = T Function<T>();

main() {
  AAlias             a1;
  AAlias<A>          a2;
  AAlias<A<Null>>    a3;
  AAlias<A<dynamic>> a4;
  AAlias<A<Object>>  a5; //# 01: compile-time error
  print("OK");
}

Dart does not throw any errors here and just prints "OK". Seems like expected behavior is to throw a compile-time error for AAlias<A<Object>> a5; like analyzer does here.

Sample output is:

$> dart test.dart
OK

$> dartanalyzer test.dart
Analyzing test.dart...
error - 'A' doesn't extend 'A<A>' at test.dart:9:10 - type_argument_not_matching_bounds
error - 'Object' doesn't extend 'A' at test.dart:9:12 - type_argument_not_matching_bounds
hint - The value of the local variable 'a1' isn't used at test.dart:5:22 - unused_local_variable
hint - The value of the local variable 'a2' isn't used at test.dart:6:22 - unused_local_variable
hint - The value of the local variable 'a3' isn't used at test.dart:7:22 - unused_local_variable
hint - The value of the local variable 'a4' isn't used at test.dart:8:22 - unused_local_variable
hint - The value of the local variable 'a5' isn't used at test.dart:9:22 - unused_local_variable
2 errors and 5 hints found.

@lrhn
Copy link
Member

lrhn commented May 14, 2019

This looks correct to me.

Dart allows "super-bounded types", which are invalid arguments to parameterized types, in some situations. The requirement is that the type would be valid if only some dynamic (or Null in contravariant position) types were replaced with a valid type.

Super-bounded types are useful precisely because recursive type-restrictions like

class A<X extends A<X>> {}

would not otherwise have any expressible super-types. There is no valid type parameter which can be applied to A which would create a super-type of all valid instantiations of A.
We need such a type when we instantiate to bounds, so a super-bounded type is used instead.
It's not a valid parameterization, but it can only be used as a type, you cannot create instances of precisely that type.

Example:

List<A> x = [];
List<A<A>> y = [];
List<A<dynamic>> z = [];

These three declarations are valid, but because A is generic, we need to fill in the type arguments of one A in the first two, and provide a type argument for the list literals as well.
After type inference, those lines become:

List<A<A<dynamic>>> x = <A<A<dynamic>>>[];
List<A<A<A<dynamic>>>> y = <A<A<A<dynamic>>>>[];
List<A<dynamic>> z = <A<dynamic>>[];

which you can check by adding:

print(x.runtimeType);
print(y.runtimeType);
print(z.runtimeType);

All of those A types are super-bounded, and there is no trivial, finite and valid bound we could use instead.
(When instantiating to bounds, say for A itself, we insert a type argument which is the bound of that type parameter, and if that bound is recursive, we insert dynamic in the recursive positions, which is why A becomes A<A<dynamic>> because the bound of T is A<T>.)

Or something close to his, I'm actually not an expert. @eernstg?

@iarkh
Copy link
Contributor Author

iarkh commented May 14, 2019

If dart behavior is correct, there is a bug in analyzer here - it should not throw compile error with the example above, but it does actually...

Please note that the problem is in the line 9 here (AAlias<A<Object>> a5;) - dart passes with this and analyzer throws an error. All the other lines passes both with dart and analyzer in the same way.,

@mit-mit mit-mit added the area-front-end Use area-front-end for front end / CFE / kernel format related issues. label May 14, 2019
@mit-mit
Copy link
Member

mit-mit commented May 14, 2019

CFE issue?

@lrhn lrhn added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) and removed area-front-end Use area-front-end for front end / CFE / kernel format related issues. labels May 15, 2019
@lrhn
Copy link
Member

lrhn commented May 15, 2019

I believe it's an analyzer issue. I just read the specification section "Super-Bounded Types", and AAlias<A<Object>> os a valid super-bounded type expression (replacing the top-type Object with Null would make it a regularly bounded type).

I am a little concerned about the declaration

typedef AAlias<T extends A<T>> = T Function<T>();

There is no use of T on the right-hand side (it introduces its own T type variable which shadows the typedef's type parameter). It could just as well have been:

typedef AAlias<T extends A<T>> = void Function();

and the fact that it isn't suggests that the test might not be testing what it intends to test.

Also, it is a little confusing that we talk about types and not type expressions, because AAlias<A<Object>> is an instantiation of a generic type alias, which means that it denotes a type. That type is T Function<T>(), which is completely un-bounded and unrelated to the original type argument. The type expression AAlias<A<Object>> is super-bounded, but the specification only talks about super-bounded types, so it's unclear which rules really apply here.
@eernstg, do we need to do something here?

@eernstg
Copy link
Member

eernstg commented May 16, 2019

I agree: AAlias<A<Object>> is a correctly super-bounded type because AAlias<A<Null>> (which will be AAlias<A<Never>> when we start using non-null types) is regular-bounded, because A<Null> <: A<A<Null>>. It's confusing that the body of the type alias doesn't use T, but it might still be useful to test it. ;-) I can't immediately guess what it was intended to be like.

The subtype rules talk about types and they include rules for type aliases, so in that sense we do have precedents for calling terms like AAlias<...> a type.

So my conclusion is also that this is an analyzer issue.

@srawlins srawlins added the analyzer-spec Issues with the analyzer's implementation of the language spec label Jun 16, 2020
@scheglov scheglov self-assigned this Aug 1, 2020
@scheglov
Copy link
Contributor

scheglov commented Aug 1, 2020

This test case does not have errors when the library is legacy (with // @dart = 2.8), so I support it was fixed.
Possibly as part of #42196.

With null safety a3 and a5 are reported as errors.

@scheglov scheglov closed this as completed Aug 1, 2020
@iarkh
Copy link
Contributor Author

iarkh commented Aug 3, 2020

This issue is still reproducible with the recent Dart, it's:
Dart SDK version: 2.10.0-2.0.dev (dev) (Thu Jul 30 15:00:42 2020 +0200) on "windows_x64"

It does not matter if I add // @dart = 2.8 tag there or not.

To test it with nnbd test source should be changed a liittle (and it also fails):

class A<X extends A<X>> {}
typedef AAlias<T extends A<T>> = T Function<T>();

main() {
  AAlias             a1;
  AAlias<A>          a2;
  AAlias<A<Never>>    a3;
  AAlias<A<dynamic>> a4;
  AAlias<A<Object?>>  a5; //# 01: compile-time error
  print("OK");
} 

@scheglov
Copy link
Contributor

scheglov commented Aug 3, 2020

Reopen for investigation.

@scheglov scheglov reopened this Aug 3, 2020
@scheglov
Copy link
Contributor

scheglov commented Aug 7, 2020

Your SDK build did not include the fix. The fix is in c9ee369, and right before it I see extra compilation errors in legacy and null safety. No errors on the bleeding edge.

@scheglov scheglov closed this as completed Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-spec Issues with the analyzer's implementation of the language spec area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

6 participants