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

should staticElements always be available? #132

Closed
jmesserly opened this issue Apr 10, 2015 · 9 comments
Closed

should staticElements always be available? #132

jmesserly opened this issue Apr 10, 2015 · 9 comments

Comments

@jmesserly
Copy link
Contributor

jmesserly commented Apr 10, 2015

[edit#2: clarify]

it seems we don't populate staticElements in some cases, like inference and synthetic AST nodes. I wanted to use these in js_codegen, as they're an easy way to tell if an operation is "resolved" to some known operation. It provides convenient access to the name (including a trailing "=" if it's a setter), as well as declaring type/library and if the member is static.

At this point I don't fully understand the causes, but I tracked down a few places staticElements aren't filled in.

We'll need to fix this if we want to hook our system into "go to definition" in analyzer, so it might be good to get these ironed out.

@jmesserly
Copy link
Contributor Author

Hit send too fast ... I blamed the wrong culprit :). Going to edit subject.

Reproduced by adding this to visitBinaryExpression in js_codegen:

      assert(rules.isDynamicTarget(left) || node.staticElement != null);

an example failing node from the SDK is

result + "\nStack Trace:\n$stackTrace"

which came from:

result += "\nStack Trace:\n$stackTrace";

... result is typed as String though, so it's not inference. Probably our AstBuilder nodes when we desugar op assign.

@jmesserly jmesserly changed the title staticElements are not available for inferred calls staticElements are not available for opassign Apr 10, 2015
@jmesserly
Copy link
Contributor Author

Oh, there is one type inference related staticElement bug, but I already have a fix for it: the inferObject logic in checker/resolver.dart wasn't populating elements in some cases

@jmesserly
Copy link
Contributor Author

Here's another inference related bug with staticElement:

    StreamController controller;
    StreamSubscription subscription;
    void onListen () {
      final add = controller.add;
      assert(controller is _StreamController ||
             controller is _BroadcastStreamController);
      final eventSink = controller; // is the bug because of this inferred type?
      final addError = eventSink._addError; // BUG: no staticElement for _addError

@jmesserly jmesserly changed the title staticElements are not available for opassign staticElements are not always available Apr 10, 2015
@jmesserly
Copy link
Contributor Author

Chatting with @leafpetersen it seems like our inference is not integrated with ElementResolver.
I wonder if there's a way to hook it up that doesn't introduce additional cost. I presume analyzer's propagated types work that way, so there must be a way.

@jmesserly
Copy link
Contributor Author

It sounds like what we want here might is just the "DynamicInvoke" node ... we compute this bit, but don't store it anywhere, leaving codegen to recompute it. Then we wouldn't need isDynamicCall, because the MethodInvocation/FunctionExpressionInvocation would be marked already.

@jmesserly
Copy link
Contributor Author

Oh, yet another idea: along the lines of coercion reifier, modify the tree to make those dynamic operations reified in the AST. That would be really nice in codegen, then we could always compile static ASTs.

One oddity is dsend and dcall take variable number of arguments, which we can't express in Dart. But js_codegen could happily trust the incoming AST and just emit the call.

[edit: for clarity]

@jmesserly jmesserly changed the title staticElements are not always available should staticElements always be available? Jun 10, 2015
@jmesserly
Copy link
Contributor Author

tagging question ... while I would really like to have these, unclear if worth the cost

@jmesserly
Copy link
Contributor Author

#239, also #240 will fix this

@jmesserly
Copy link
Contributor Author

closing as dupe

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

No branches or pull requests

1 participant