Skip to content

kernel: Do not abstract and share mixin applications #31118

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
rakudrama opened this issue Oct 16, 2017 · 5 comments
Closed

kernel: Do not abstract and share mixin applications #31118

rakudrama opened this issue Oct 16, 2017 · 5 comments
Assignees
Labels
customer-dart2js front-end-fasta front-end-kernel legacy-area-front-end Legacy: Use area-dart-model instead. P1 A high priority bug; for example, a single project is unusable or has many test failures

Comments

@rakudrama
Copy link
Member

rakudrama commented Oct 16, 2017

dart2js is blocked on this issue.

Consider:

class One<R> {}

class AnInterface<X> implements One<int> {}

class ADifferentInterface<Y> extends AnInterface<Y>{}

class TheMixin<Q> implements AnInterface<Q>  {}

class Two<T> extends One<int> with TheMixin<T> implements AnInterface<T> {}

class Three<T> extends One<int> with TheMixin<T> implements ADifferentInterface<T> {}

main() {
  new Two<double>();
}

What we see is that One<int> with TheMixin<T> is abstracted like so

class OneWithTheMixin<U, V> = One<U> with TheMixin<V>;
class Two<T> extends OneWithTheMixin<int, T> ...

This breaks on dart2js. dart2js has a constraint that a class cannot implement a type with different parameterizations. (The constraint allows the relationship between Two and One to be expressed as a function, the existence of this function is fundamental to the whole type representation.)

However, the introduced type OneWithTheMixin implements One<U> and One<int>. In particular, the raw type OneWithTheMixin implements One<dynamic> and One<int>.

The simplest for dart2js would be to create a fresh class for each with. This is what the old pipeline does - so it would be an advantage to do the same (we can compare outputs), and then reconsider a version of this abstraction later when we are done with the transition to Kernel.

This issue is blocking our next step, compiling angular apps with dart2js-with-kernel, so it is our highest priority issue with kernel.

@rakudrama rakudrama added area-kernel P1 A high priority bug; for example, a single project is unusable or has many test failures labels Oct 16, 2017
@peter-ahe-google
Copy link
Contributor

Preferred name would be:

"Object with Mixin1, Mixin2 for Subclass"

@sigmundch sigmundch added the legacy-area-front-end Legacy: Use area-dart-model instead. label Nov 28, 2017
@sigmundch sigmundch added this to the 2.0-alpha milestone Dec 1, 2017
@karlklose karlklose self-assigned this Dec 15, 2017
whesse pushed a commit that referenced this issue Dec 21, 2017
Workaround for Issue #31118

Change-Id: I1d19eb1adeb7236501c276eeee5446ada36115a3
Reviewed-on: https://dart-review.googlesource.com/31240
Reviewed-by: Emily Fortuna <[email protected]>
Commit-Queue: Sigmund Cherem <[email protected]>
@peter-ahe-google
Copy link
Contributor

Aske, could you take a look at this?

@peter-ahe-google
Copy link
Contributor

I already took a stab at implementing the semantics dart2js need:

https://dart-review.googlesource.com/c/sdk/+/17882

For this CL to land, we'd need to keep the old behavior and introduce a flag in pkg/kernel/lib/target/targets.dart that we can use to pick the desired implementation.

However, Stephen reported that:

  1. language_2/mixin_generic_test fails. The reason appears to be missing type parameters in the asXXX functions. This hints that the mixin application has missing type parameters.

  2. I see the following, which confirms the above:

abstract class _C&S&M = self::S<core::Map<self::C::U, self::C::V>> with self::M<core::List<self::C::U>> {
}
abstract class _C&S&M&N = self::_C&S&M with self::N<core::Set<self::C::V>> {
}
class C<U extends core::Object, V extends core::Object> extends self::_C&S&M&N {
  default constructor •() → void
    : super self::S::•()
    ;
}

It appears that the type parameters are missing and there are references to type parameters no longer in scope (this may explain #3)
I believe that all of the mixin applications _C&... should have their own type parameters exactly matching the same types as C.

  1. I can't fully print the test. If I try

    ./pkg/front_end/tool/fasta compile --dump-ir --target=none tests/language_2/mixin_generic_test.dart

I get a crash:

Unhandled exception:
NoSuchMethodError: The method '>=' was called on null.
Receiver: null
Tried calling: >=(0)
#0      Object.noSuchMethod (dart:core-patch/dart:core/object_patch.dart:46)
#1      BinaryPrinter.writeUInt30 (package:kernel/binary/ast_to_binary.dart:71:18)
#2      BinaryPrinter.visitTypeParameterType (package:kernel/binary/ast_to_binary.dart:1385:5)
#3      TypeParameterType.accept (package:kernel/ast.dart:4761:34)
#4      BinaryPrinter.writeNode (package:kernel/binary/ast_to_binary.dart:148:10)
#5      List.forEach (dart:core-patch/dart:core/growable_array.dart:270)
#6      BinaryPrinter.writeList (package:kernel/binary/ast_to_binary.dart:137:11)
#7      BinaryPrinter.writeNodeList (package:kernel/binary/ast_to_binary.dart:141:5)
#8      BinaryPrinter.visitInterfaceType (package:kernel/binary/ast_to_binary.dart:1338:7)
#9      InterfaceType.accept (package:kernel/ast.dart:4465:34)
#10     BinaryPrinter.writeNode (package:kernel/binary/ast_to_binary.dart:148:10)
#11     List.forEach (dart:core-patch/dart:core/growable_array.dart:270)
#12     BinaryPrinter.writeList (package:kernel/binary/ast_to_binary.dart:137:11)
#13     BinaryPrinter.writeNodeList (package:kernel/binary/ast_to_binary.dart:141:5)
#14     BinaryPrinter.visitSupertype (package:kernel/binary/ast_to_binary.dart:1349:7)
#15     Supertype.accept (package:kernel/ast.dart:4869:26)
#16     BinaryPrinter.writeNode (package:kernel/binary/ast_to_binary.dart:148:10)
#17     BinaryPrinter.writeOptionalNode (package:kernel/binary/ast_to_binary.dart:156:7)
#18     BinaryPrinter.visitClass (package:kernel/binary/ast_to_binary.dart:621:5)
#19     Class.accept (package:kernel/ast.dart:825:30)
#20     BinaryPrinter.writeNode (package:kernel/binary/ast_to_binary.dart:148:10)
#21     List.forEach (dart:core-patch/dart:core/growable_array.dart:270)
#22     BinaryPrinter.writeList (package:kernel/binary/ast_to_binary.dart:137:11)
#23     BinaryPrinter.writeNodeList (package:kernel/binary/ast_to_binary.dart:141:5)
#24     BinaryPrinter.visitLibrary (package:kernel/binary/ast_to_binary.dart:496:5)
#25     Library.accept (package:kernel/ast.dart:390:30)
#26     BinaryPrinter.writeNode (package:kernel/binary/ast_to_binary.dart:148:10)
#27     List.forEach (dart:core-patch/dart:core/growable_array.dart:270)
#28     BinaryPrinter.writeLibraries (package:kernel/binary/ast_to_binary.dart:337:23)
#29     BinaryPrinter.writeProgramFile (package:kernel/binary/ast_to_binary.dart:219:5)
#30     writeProgramToFile (package:front_end/src/fasta/kernel/utils.dart:42:13)
<asynchronous suspension>
#31     CompileTask.compile (file:///usr/local/google/home/sra/Dart/sdk/pkg/front_end/tool/_fasta/entry_points.dart:164:11)
<asynchronous suspension>
#32     compile.<anonymous closure> (file:///usr/local/google/home/sra/Dart/sdk/pkg/front_end/tool/_fasta/entry_points.dart:98:25)
<asynchronous suspension>
#33     withGlobalOptions.<anonymous closure> (file:///usr/local/google/home/sra/Dart/sdk/pkg/front_end/tool/_fasta/command_line.dart:376:13)
#34     CompilerContext.runInContext.<anonymous closure> (package:front_end/src/fasta/compiler_context.dart:89:33)
#35     _rootRun (dart:async/zone.dart:1124)
#36     _CustomZone.run (dart:async/zone.dart:1021)
#37     runZoned (dart:async/zone.dart:1499)
#38     CompilerContext.runInContext (package:front_end/src/fasta/compiler_context.dart:89:12)
#39     CompilerContext.runWithOptions (package:front_end/src/fasta/compiler_context.dart:96:41)
#40     withGlobalOptions (file:///usr/local/google/home/sra/Dart/sdk/pkg/front_end/tool/_fasta/command_line.dart:369:26)
#41     compile (file:///usr/local/google/home/sra/Dart/sdk/pkg/front_end/tool/_fasta/entry_points.dart:91:18)
<asynchronous suspension>
#42     compileEntryPoint (file:///usr/local/google/home/sra/Dart/sdk/pkg/front_end/tool/_fasta/entry_points.dart:49:11)
<asynchronous suspension>

@peter-ahe-google
Copy link
Contributor

We may be in luck and be able to use the approach taken in https://dart-review.googlesource.com/c/sdk/+/33884

@peter-ahe-google
Copy link
Contributor

Fixed in 9f56e28.

@kmillikin kmillikin added legacy-area-front-end Legacy: Use area-dart-model instead. front-end-kernel and removed legacy-area-front-end Legacy: Use area-dart-model instead. labels Sep 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer-dart2js front-end-fasta front-end-kernel legacy-area-front-end Legacy: Use area-dart-model instead. P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Development

No branches or pull requests

6 participants