Skip to content

Support generic methods in Fasta #30455

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
sjindel-google opened this issue Aug 16, 2017 · 9 comments
Closed

Support generic methods in Fasta #30455

sjindel-google opened this issue Aug 16, 2017 · 9 comments
Assignees
Labels

Comments

@sjindel-google
Copy link
Contributor

Fasta's support for generic methods is not yet complete, and has several bugs, which can evidenced by disabling type erasure and running Fasta's output through the verifier.

When these issues are resolved, we may continue implementing support for generic methods in the VM.
This issue number is used as a tag through the VM source to indicate where this support is incomplete due to the currently inability to test it.

@peter-ahe-google
Copy link
Contributor

Did you also turn on strong mode when you disabled erasure?

@sjindel-google
Copy link
Contributor Author

My understanding is that strong mode is enabled/disabled on a test-by-test basis -- it's given as a parameter to the vm_fasta target.

Currently, were are only performing erasure in this target when strong mode is off, but it seems that most tests run in this way.

@sigmundch
Copy link
Member

@peter-ahe-google I recall you did a bunch of fixes for type-variables/generics in the last few weeks. Is this still an open issue?

Just trying to figure out whether there are still issues to work on before we disable erasure.

@peter-ahe-google
Copy link
Contributor

I think this is still an issue.

@sjindel-google I'd like a bit more details about how to reproduce this problem. For example:

  1. Apply this patch.
  2. Run this command.

@sjindel-google
Copy link
Contributor Author

sjindel-google commented Sep 7, 2017

There are two problems:

  1. Fasta generates invalid output on some programs which is hidden by erasure.

The VM's default behavior for generic methods (in the absence of the "reify" flags) is to replace type-parameters with dynamic. So even if generic method type parameters are not erased, the VM will do it itself. Even so, fasta generates invalid kernel for some programs with generic methods, which the VM cannot read.

To evidence this, disable erasure in vm_fasta.dart and run:

./pkg/front_end/tool/fasta testing fasta_min
  1. VM support for generic methods in Kernel is incomplete.

The support Regis has implemented in the VM for actual generic methods has not been fully ported to the kernel reader. When --reify and --reify-generic-functions are supplied to the VM, it will not erase type parameters on generic methods. However, since support for running kernel code with these is incomplete, the VM will crash when erasure is disabled and these flags are passed.

To evidence this, disable erasure in vm_fasta.dart and run:

python tools/test.py -m release -c dartk --vm-options "--reify --reify_generic_functions" co19

@a-siva
Copy link
Contributor

a-siva commented Sep 15, 2017

@crelier

@peter-ahe-google
Copy link
Contributor

@sjindel-google please add the following information to this bug:

  1. A patch that applies to the sdk repository.

  2. A single command that fails, not all of the co19 test suite.

@sjindel-google
Copy link
Contributor Author

sjindel-google commented Oct 2, 2017

Patch:

diff --git a/test.dart b/test.dart
new file mode 100644
index 0000000000..fcf70b833d
--- /dev/null
+++ b/test.dart
@@ -0,0 +1,5 @@
+void iser<X>(x) {
+  print(x is X);
+}
+
+main() => iser<int>("hi");

Repro:

DART_CONFIGURATION=ReleaseX64 ./pkg/front_end/tool/fasta compile test.dart
out/ReleaseX64/dart --reify --reify_generic_functions -c test.dart.dill

The last command outputs "true" when it should output "false".

@sigmundch sigmundch added the legacy-area-front-end Legacy: Use area-dart-model instead. label Oct 4, 2017
@sjindel-google
Copy link
Contributor Author

Type erasure is dead.

@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
Projects
None yet
Development

No branches or pull requests

5 participants