change type of ParseNode::pn_type

RESOLVED FIXED in Firefox 56



a year ago
a year ago


(Reporter: tromey, Assigned: tromey)



Firefox Tracking Flags

(firefox56 fixed)


MozReview Requests


Submitter Diff Changes Open Issues Last Updated
Error loading review requests:


(1 attachment)



a year ago
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.

Comment 1

a year ago
Looks like pn_op can be converted too, but not pn_arity, due to a gcc warning.

Comment 2

a year ago
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.

Comment 3

a year ago
But now I wonder if this is advisable given VC++'s approach to bitfield packing.

Comment 4

a year ago
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 hidden (mozreview-request)

Comment 6

a year ago
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+

Comment 7

a year ago
Pushed by
change pn_type to a ParseNodeKind; r=jimb

Comment 8

a year ago
Last Resolved: a year ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.