search

tc39 / proposal-binary-ast

Binary AST proposal for ECMAScript
963 stars 23 forks source link

Why do some interfaces not inherit from Node? #56

Open Yoric opened 6 years ago

Yoric commented 6 years ago

Is this a typo or is there a semantics associated to this?

Yoric commented 6 years ago

More importantly, I wish to know whether this has implications for the en/decoder.

In particular that field

[TypeIndicator] readonly attribute Type type;
kannanvijayan-zz commented 6 years ago

I've been skipping the type attribute entirely and just deriving it from the node name.

On Wed, Aug 29, 2018, at 1:39 AM, David Teller wrote:

More importantly, I wish to know whether this has implications for the en/decoder.> In particular that field

Yoric commented 6 years ago

I've been skipping the type attribute entirely and just deriving it from the node name.

Yes, that's what I've been doing, too. Just double-checking that this is correct.

syg commented 6 years ago

Deriving it from the node name is fine. The intention was all things that need to be distinguished by some kind of tag have the type indicator. That the Asserted* interfaces do not is a bug.

kannanvijayan-zz commented 6 years ago

I thought this was mostly an artifact of distinguishing between "real" AST nodes and composite field values. I.e. it related to some notion of a 'child attribute' (a node-typed thing), and a 'field attribute' (a value-typed or non-node-interface type, or arrays thereof).

All the places where the Asserted* show up, they are monomorphically constrained (i.e. type tags not necessary). I had assumed this was by design, and both of those characteristics were related.

kannanvijayan-zz commented 6 years ago

@syg @Yoric to expand on my previous comment - the reason this distinction is useful for is that it allows us to distinguish between logical "child nodes" from "non-scalar attributes of the node".

This distinction is used in constructing the path suffix for contextual prediction in the compressor. A single "step" in the path is the path to the prior node-typed value.

If it's not too much of an issue, I'd like to leave the Asserted* as bare interfaces (not inheriting from Node), and also add FunctionOrMethodContents to that list (it's an aggregator of children, but it doesn't correspond to a node in the tree for me, at an intuitive level).

To me, making these inherit from node sort of feels like making all of the Array<*> also "structures inheriting from Node".

syg commented 6 years ago

@kannanvijayan It's all fine by me to keep that kind of distinction via Node subclassing.

Two choices:

  1. We can make another common ancestor for the non-real AST nodes to also have a type indicator.
  2. We could also remove the type indicator thing and simply say that all IDL nodes have a queryable type id, which is probably cleaner.

I'm leaning towards 2.

Yoric commented 6 years ago

@syg I'd go for 2.