Closed Bug 1383157 Opened 4 years ago Closed 4 years ago

change type of ParseNode::pn_type


(Core :: JavaScript Engine, enhancement)

Not set



Tracking Status
firefox56 --- fixed


(Reporter: tromey, Assigned: tromey)



(1 file)

While debugging bug 1370648, I found myself having to cast
a parse node's pn_type to ParseNodeKind to understand what
a parse node actually represented.

It's a bit nicer when debugging if pn_type is actually a ParseNodeKind --
then the debugger can do this work for me.
Looks like pn_op can be converted too, but not pn_arity, due to a gcc warning.
I was thinking changing pn_arity would trip over
But, there's no need to change the underlying type of ParseNodeArity, so it works ok in the end.
But now I wonder if this is advisable given VC++'s approach to bitfield packing.
I think I'll have to scale this back to just changing pn_type, due to MS bitfield packing:

1. It's possible to use the correct types for pn_op and pn_arity.  However, then it is
   undefined if the bitfields are signed.  So, it's best to specify the underlying
   type of the enums.
2. ... but to get correct packing from VC++ (and btw you can test various scenarios with
   gcc using -mms-bitfields) the underlying types must be the same.
3. ... but specifying an underlying type for ParseNodeArity runs into the GCC bug, namely, if an underlying type
   is given then GCC issues a warning about a bitfield of less than full width, even
   when all the enum values could fit in specified width.

pn_type was the thing I was really interested in anyway, and I think changing it is
a small improvement.
Comment on attachment 8889528 [details]
Bug 1383157 - change pn_type to a ParseNodeKind;

Wow, that's all that's needed? Should have happened long ago.
Attachment #8889528 - Flags: review?(jimb) → review+
Pushed by
change pn_type to a ParseNodeKind; r=jimb
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.