FunctionBody should be a Node in order to use "or" about proposal-binary-ast HOT 13 CLOSED

This is slightly off-topic, for which I apologize, but:

If it's going to be a Node anyway, I think that it should also contain the directives list, as it does in Shift. Then there isn't this problem with the directives attribute being optional on arrow bodies, with an additional required assertion that the attribute is present exactly when the body is a FunctionBody.

I assume the reason for putting the directives list in the various *Contents nodes instead of the FunctionBody which every *Contents node contains is to aid streaming compilation, so you get the directives before the parameters, but I personally don't think that outweighs the awkwardness - since "use strict" directives are already banned for nontrivial parameter lists, how much does this actually save?

from proposal-binary-ast.

This may be a situation where we want to duplicate the directive information (much as we do with bound and captured names in scopes). But I'm not sure about that..

@arai-a could you give me a recap on the specific reasons we wanted to lift directives again?

from proposal-binary-ast.

@bakkot I imagine all implementations would want to consume directives before any actual parsing of any function stuff including parameters, so the ordering makes sense for implementations. Good point about non-simple param lists already ban "use strict". That puts the remaining use case to be just simpler for implementations. (And octal escapes, I guess?)

And by "awkwardness", what operation is now awkward? Reading the tree? Writing tools for the tree?

@arai-a I'm not understanding the desire to make it a Node that just contains a single attribute. Does that help implementation?

Edit: I personally agree with @bakkot and would prefer directives to remain with the statements of a function body. But I'm going to heavily favor implementations here, and I'm under the impression having it first is preferable for impls. If, however, there is an argument that the current schema is awkward for, say, tools, then that might swing the favor back the other way.

from proposal-binary-ast.

@kannanvijayan @efaust will know better about the specific case.
My opinion is that, putting fields in the dependency order is simply better.
(that's the reason why we have Asserted*Scope, right?)

@syg I don't understand the semantics of (Node or FrozenArray) type, like, how to interpret it and encode it.
In all other places where or is used, all operands are Node's sub-interfaces.
(maybe we could have some special-case for that, but I don't think it's cleaner.)

then, considering the optional directives issue, I guess we could have 2 interfaces for expression body and block body, for array function?
so that both (Node or FrozenArray) issue and optional directive issue go away.

from proposal-binary-ast.

@arai-a Ah I see. The optional directives thing is indeed annoying. I think @bakkot's suggestion of putting directives back into a FunctionBody node along with the statement list would be the cleanest way of fixing it. How painful is it for you if directives came after parameters?

from proposal-binary-ast.

@syg I haven't yet implemented streaming compilation of function directive (I have to update binjs-ref to generate .binjs file with updated scope info before that), so I'm not sure if there's some edge case, but maybe we could keep possible error while emitting parameters, and throw it if we hit "use strict", that seems not so much troublesome, but if we don't have to do that, it's much better.

To my understanding, the reason why "use strict" is inside function's body is just that there's no other place to put it into, in term of JS syntax.
"use strict" is semantically function's property, not body's, so if we're creating AST, putting it into function's property makes more sense,
and it benefits implementation as mentioned above,
so I have no reason to move directive back to FunctionBody.

from proposal-binary-ast.

How painful is it for you if directives came after parameters?

FWIW, as a second implementor voice, I would vastly prefer to have strictness as a property of the function itself, rather than in the function body. Consider also the SkippableFunction idea, where you might want to encode the directives of the inner function in a laziness metadata struct without actually inspecting its tree.

from proposal-binary-ast.

from proposal-binary-ast.

going back to the issue with "or", the alternative solution I said in https://github.com/binast/ecmascript-binary-ast/issues/43#issuecomment-399166222 is:

interface ArrowExpressionContentsWithBlockBody : Node {
...
  attribute FunctionBody body;
};

interface ArrowExpressionContentsWithExpressionBody : Node {
...
  attribute Expression body;
};

interface EagerArrowExpressionWithBlockBody : Node {
  attribute boolean isAsync;
  attribute FrozenArray<Directive> directives;
  attribute ArrowExpressionWithBlockBody contents;
};

interface EagerArrowExpressionWithExpressionBody : Node {
  attribute boolean isAsync;
  attribute ArrowExpressionWithExpressionBody contents;
};

from proposal-binary-ast.

FWIW, as a second implementor voice, I would vastly prefer to have strictness as a property of the function itself, rather than in the function body. Consider also the SkippableFunction idea, where you might want to encode the directives of the inner function in a laziness metadata struct without actually inspecting its tree.

One possible approach is to treat these the same way (conceptually) we treat bound and captured names: replicate/cache the info at the function definition level, and also have it in the body contents where it appears. This would preserve the logical structure of these directives showing up in the code, and also give us the ability to read that info without digging into the body contents.

I must say I was surprised initially when I found that directives had a special grammar node instead of being treated as normal literal-expression statements occurring in the body.

from proposal-binary-ast.

I must say I was surprised initially when I found that directives had a special grammar node instead of being treated as normal literal-expression statements occurring in the body.

I think directives as a language feature are a hack designed for the text representation that there's just no value to preserve them in-situ in an AST representation, like with bindings.

from proposal-binary-ast.

@arai-a What do you think of 9aa4feb?

from proposal-binary-ast.

@syg yeah, looks good. thanks!

from proposal-binary-ast.