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

Eliminate NodeStyle in Loyc trees? #80

Open
qwertie opened this issue Mar 18, 2019 · 1 comment
Open

Eliminate NodeStyle in Loyc trees? #80

qwertie opened this issue Mar 18, 2019 · 1 comment
Labels

Comments

@qwertie
Copy link
Owner

qwertie commented Mar 18, 2019

Using NodeStyle (holds 8 bits of formatting info) has two genuine advantages:

  1. It's "free" - it uses no memory as it is hidden away in the top 8 bits of a node's 32-bit length field.
  2. It's mutable - as it is intended to affect output formatting, not semantics, it's the only part of LNode that is allowed to change. Eliminating it would preclude certain timesaving tricks that might come in handy in a compiler.

Instead of NodeStyle we could use the flyweight pattern to store the styles as a normal attribute list.

  • Pro: the main reason to do this is to reduce the conceptual burden of Loyc trees. It's supposed to be a simple concept, so I'm concerned about distracting people with these oddball style bits and I would also not want to suggest that any 3rd party implementations of Loyc trees should support them.
  • Con: Since nodes without attributes are optimized into special subclasses, the flyweight pattern would typically cost two extra words of memory (for a VList fat-pointer) in nodes that use style bits. I'm guessing 25-50% of nodes will have a style bit. (Nodes that have both styles and normal attributes will pay a different cost, not sure how large.) Another potential cost is that by raising the proportion of nodes with attributes, branch prediction may suffer when calling the (virtual) Attrs property.
  • Con: Style attributes would live in an EmptySourceFile disjoint from whatever is being parsed, so if someone is hoping for an invariant where the Range of a child is "within" the Range of the parent will be disappointed. (Such disappointment already occurs when using macros.)

Any thoughts @jonathanvdc?

@qwertie
Copy link
Owner Author

qwertie commented Feb 9, 2020

Hmm, getting rid of NodeStyle isn't that easy... it's not just LNode that uses it, but also Token, and Token has a stronger case for actually needing it. It's sometimes convienient for a parser to copy the NodeStyle over from the token unchanged; transforming it automatically is not trivial since NodeStyle values are ambiguous (e.g. 6 is used for both TQStringLiteral and HexLiteral.)

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

No branches or pull requests

1 participant