Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

need type promotion #31

Closed
jmesserly opened this issue Jan 22, 2015 · 7 comments
Closed

need type promotion #31

jmesserly opened this issue Jan 22, 2015 · 7 comments
Assignees

Comments

@jmesserly
Copy link
Contributor

this is a very handy feature in Dart:

if (x is Foo) {
  // ... assume x is a Foo
}
@jmesserly
Copy link
Contributor Author

fyi, noticed this is leading to some pretty bad SDK codegen ... lots of dynamic invokes that aren't needed

@vsmenon
Copy link
Contributor

vsmenon commented Jun 3, 2015

Yeah, I think this is because we're getting the type off of the Element instead of the AstNode in the checker. We should have the promoted type on the latter as we're using the analyzer's propagation rules.

@jmesserly
Copy link
Contributor Author

aha, yeah, that's a good guess! I was wondering why this one didn't "just work" from their static type rules

@jmesserly jmesserly self-assigned this Jun 18, 2015
@jmesserly
Copy link
Contributor Author

also related: #239

@jmesserly
Copy link
Contributor Author

hmmm, I haven't been able to reproduce this yet. But I recall seeing busted generated code with too many dinvokes. Going to keep looking.

@jmesserly
Copy link
Contributor Author

ah, found some example brokenness:
lib/runtime/dart/async.js#L47-49

    static _getBestStackTrace(error, stackTrace) {
      if (stackTrace != null)
        return stackTrace;
      if (dart.is(error, core.Error)) {
        return dart.as(dart.dload(error, 'stackTrace'), core.StackTrace);
      }
      return null;
    }

however, I think it might be by design :\ ... from the Dart spec, 16.34 Type Test:

Let v be a local variable or a formal parameter. An is-expression of the
form v is T shows that v has type T iff T is more specific than the type S of
the expression v and both T != dynamic and S != dynamic.

and (slightly edited by me, for formatting):

We do not want to refine the type of a variable of type dynamic, as this
could lead to more warnings rather than less. The opposite requirement, that
T != dynamic is a safeguard lest S ever be the bottom type.

@jmesserly jmesserly mentioned this issue Jul 29, 2015
6 tasks
@jmesserly
Copy link
Contributor Author

@vsmenon @leafpetersen and I looked at these. We believe the original bug was fixed (early on, DDC completely disabled type promotion in analyzer), however, there is clearly room for improvement. Tracking that in #274

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

2 participants