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

Why do some interfaces not inherit from Node? #56

Open
Yoric opened this issue Aug 27, 2018 · 8 comments
Open

Why do some interfaces not inherit from Node? #56

Yoric opened this issue Aug 27, 2018 · 8 comments
Assignees

Comments

@Yoric
Copy link
Collaborator

Yoric commented Aug 27, 2018

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

@Yoric
Copy link
Collaborator Author

Yoric commented Aug 29, 2018

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
Copy link
Collaborator

kannanvijayan-zz commented Aug 29, 2018 via email

@Yoric
Copy link
Collaborator Author

Yoric commented Aug 29, 2018

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
Copy link
Collaborator

syg commented Aug 29, 2018

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
Copy link
Collaborator

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
Copy link
Collaborator

@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
Copy link
Collaborator

syg commented Sep 7, 2018

@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
Copy link
Collaborator Author

Yoric commented Sep 20, 2018

@syg I'd go for 2.

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

No branches or pull requests

3 participants