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

Dart2JS: Fatal error paths not treated a such? #33917

Closed
matanlurey opened this issue Jul 19, 2018 · 4 comments
Closed

Dart2JS: Fatal error paths not treated a such? #33917

matanlurey opened this issue Jul 19, 2018 · 4 comments

Comments

@matanlurey
Copy link
Contributor

Forked from #33372.

I am a bit confused on the output of the following program:

void main(var arg) {
  switch (arg) {
    case 1:
      willThrowNoSuchMethod();
      break;
    case 2:
      willThrowCastError();
      break;
    case 3:
      willThrowTypeError();
      break;
  }
}

void willThrowNoSuchMethod() {
  dynamic x = [];
  x.aMissingMethod();
  return null;
  return null;
}

void willThrowCastError() {
  var x = [];
  List<String> y = x;
  return null;
  return null;
}

void willThrowTypeError() {
  dynamic x = 5;
  String y = x;
  return null;
  return null;
}

... emits ...

main: function(arg) {
  switch (arg) {
    case 1:
      F.willThrowNoSuchMethod();
      break;
    case 2:
      F.willThrowCastError();
      break;
    case 3:
      F.willThrowTypeError();
      break;
  }
},
willThrowNoSuchMethod: function() {
  [].aMissingMethod$0();
  return;
},
willThrowCastError: function() {
  H.assertSubtype([], "$isList", [P.String], "$asList");
  return;
},
willThrowTypeError: function() {
  H.stringTypeCheck(5);
  return;
}

Questions:

  1. Why isn't a NSM error inlined in lieu of [].aMissingMethod$()?
  2. Why isn't a TypeError inlined in lieu of H.assertSubtype?
  3. Why isn't a TypeError inlined in lieu of H.stringTypeCheck?
@rakudrama
Copy link
Member

If the code was 'inlined' it would be bigger.

How often do programs contain such mistakes? The type checks are not represented as method calls, so special code would be required to lower the guaranteed-to-fail checks to throw statements. The special code would have to be kept in sync with the behaviour of code that throws the same errors but in a way that is not obvious to the compiler. I don't think optimizing broken programs should be a priority.

In most real programs the code called above is called from other places, so the size cost of the called helper code is shared with the other call sites.

@matanlurey
Copy link
Contributor Author

Makes sense. This actually wasn't a size question as much as a being curious why methods that will always throw check at runtime if they will throw. A sort of silly example would be:

void main() {
  dynamic iAmNotABoolean = 'Hello';
  bool castError = iAmNotABoolean;
  print('I am not reachable!');
}

... which generates ...

main: function() {
  H.boolTypeCheck("Hello");
  H.printString("I am not reachable!");
}

So I guess it is more of a meta question on why NSM/CastError/TypeError is not marked unreachable.

@rakudrama
Copy link
Member

It is just that we have not bothered to write an optimization that removes code that follows an always-throw operation. Do you think it happens often enough that writing such an optimization would pay for itself?

We do remove the code in the case where the operation is a call:

void main() {
  check();
  print('I am not reachable!');
}

void check() {
  throw 123;
}
    main: function() {
      D.check();
    },
    check: function() {
      throw H.wrapException(123);
    }

The mechanism is different here, as we have an analysis that knows that check always throws before we compile to JavaScript - it is part of the global type analysis.

Note that although the type checks look like function calls in JavaScript, they are not represented at any point in the optimization as function calls. Most of the optimization happens at a level in-between Dart and JavaScript.

What action is required to close this issue?

@matanlurey
Copy link
Contributor Author

It is just that we have not bothered to write an optimization that removes code that follows an always-throw operation. Do you think it happens often enough that writing such an optimization would pay for itself?

Nope, it was just a curiosity.

What action is required to close this issue?

None. I can close this issue now; I was trying to better understand #33372.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants