Consider using mozilla::Variant for ParseNode

NEW
Unassigned

Status

()

Core
JavaScript Engine
P3
normal
8 months ago
2 months ago

People

(Reporter: Yoric, Unassigned)

Tracking

({triage-deferred})

Firefox Tracking Flags

(Not tracked)

Details

I've been bitten a few times by ParseNode's unions. We should consider rewriting it to take advantage of the safer/better self-documented `mozilla::Variant`.
ParseNode is a bit cesspooly, yes.  There have been ideas over time to fix this aspect of it, in one way or another.  Using actual inheritance, for example, to represent the various sorts of node has been idly proposed.  The exact better approach is not entirely obvious/clear/decided.

It also seems worth noting that a better "ParseNode" replacement ideally should well-type its children.  And a distinction between statement nodes and expression nodes, especially, would be a very very nice thing.  Having ExpressionStatement contain an Expression, and not a Statement, in a type-system-enforced way, would be a very good thing.  Quite possibly the fix should address both concerns at once.  (Although not necessarily, logrolling as being potentially a thing that could unduly delay a fix.)

In short, I'd hesitate to prematurely fixate on Variant as the solution for this.  (Not even reaching the fact that Variant's internal tag is not publicly exposed right now, but the ParseNodeKind tag in ParseNode is, and possibly that's something that *should not* be fixt.)
Keywords: triage-deferred
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.