Skip to content

Minification interferes with generic functions #38005

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
ciriousjoker opened this issue Aug 26, 2019 · 5 comments
Closed

Minification interferes with generic functions #38005

ciriousjoker opened this issue Aug 26, 2019 · 5 comments
Labels
area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. P2 A bug or feature request we're likely to work on web-dart2js

Comments

@ciriousjoker
Copy link

This tracker is for issues related to:

  • dart2js

  • Dart SDK Version (dart --version)

2.4.1

TLDR:

Minification changes generic type names which could lead to problems if they're hardcoded.

Example code:

class A {}

void doSmth<T>() {
    switch(T) {
      case A:
        // This will never be reached since during runtime,
        // T might be mB or some other minified string
        doSmthElse();
        break;
    }
}

doSmth<A>();

Proposed solution:
Don't minify generic type names.

Are there any workarounds? I've tried using enums but it doesn't work like Typescript..

@ciriousjoker
Copy link
Author

@jodinathan on Gitter mentioned that this code shouldn't work at all and that I should instead use if statements to achieve the same.

His solution worked and now I'm using a lot of if statements instead of a large switch statement.

However, if this isn't supposed to work anyway, why does it work without minification and throws an error if I use minification?
Why doesn't it throw a compile time error if I'm not supposed to do this?
Why aren't switch statements and the corresponding if statements completely equal in all cases?

@vsmenon vsmenon added area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. P2 A bug or feature request we're likely to work on web-dart2js labels Aug 26, 2019
@vsmenon
Copy link
Member

vsmenon commented Aug 26, 2019

FYI - @fishythefish @rakudrama - missing tracking on direct type var use?

@fishythefish
Copy link
Member

fishythefish commented Aug 29, 2019

class MyClass {}

void foo<T>() {
  switch (T) {
    case MyClass:
      print("MyClass");
      return;
    default:
      print("other");
      return;
  }
}

void main() {
  foo<MyClass>();
}

This currently prints other when compiled with dart2js, with or without minification. Here's the essential part of the compiled code:

main: function() {
  switch (new H.TypeImpl(V.MyClass)) {
    case C.Type_MyClass_GMm:
      P.print("MyClass");
      break;
    default:
      P.print("other");
      break;
  }
}

You can see that T in switch (T) is lowered to a new TypeImpl (RTI). On the other hand, since cases must be constants, we lower MyClass in case MyClass to a compile-time constant RTI. Since these are distinct objects, JS switch doesn't consider them equal.

This issue will be fixed when the new dart2js RTI rolls out, since we canonicalize RTI objects.

@sigmundch
Copy link
Member

As @fishythefish points out, the root cause is that type objects are not canonicalized by dart2js and therefore, types are not supported in switch cases except on some simple cases where all values are type literals. There use to be an error reported for this in the past, but a recent bug made us not show the compile-time error.

We have a duplicate issue filed under #17207 with details about the underlying problem. I'm going to keep that one open and close this one as a duplicate, but if we are missing something detail that we should be tracking, please let us know.

@rakudrama
Copy link
Member

Although this issue will eventually be fixed by the new type representation that we are currently working on, we should make sure that there is test coverage of the use case reported here.

Re-close after the test coverage is confirmed or implemented.

@rakudrama rakudrama reopened this Sep 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. P2 A bug or feature request we're likely to work on web-dart2js
Projects
None yet
Development

No branches or pull requests

5 participants