Privatize more stuff in Token

RESOLVED FIXED in mozilla10

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Waldo, Assigned: Waldo)

Tracking

Trunk
mozilla10
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Created attachment 569215 [details] [diff] [review]
Privatize reflags

Further steps to being able to understand tokens, and to make translating them into parse nodes simpler...
Attachment #569215 - Flags: review?(cdleary)
Created attachment 569217 [details] [diff] [review]
Privatize dval

This is somewhat more than "just" privatizing, because I split dval into a uint16 used only for referring to sharp variables and a number version used for everything else.  This theoretically introduces more chances for mis-punning, but the uses are all pretty semantically guarded against error, and it clarifies things a bit, so it seems the right thing to do.
Attachment #569217 - Flags: review?(cdleary)
Comment on attachment 569215 [details] [diff] [review]
Privatize reflags

Review of attachment 569215 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/frontend/Parser.cpp
@@ +7637,4 @@
>  
>          const jschar *chars = tokenStream.getTokenbuf().begin();
>          size_t length = tokenStream.getTokenbuf().length();
> +        RegExpFlag flags = RegExpFlag(tokenStream.currentToken().regExpFlags());

Coersion is no longer necessary, since it's of the right type.

::: js/src/frontend/TokenStream.h
@@ +300,5 @@
> +        /*
> +         * FIXME: Initialize the rest of this early enough to be able to assert
> +         *        type-safety.
> +         */
> +        JS_ASSERT((flags & ~AllFlags) == 0);

I think the positive assertion |(flags & AllFlags) == flags| reads better, no?
Attachment #569215 - Flags: review?(cdleary) → review+
Comment on attachment 569217 [details] [diff] [review]
Privatize dval

Review of attachment 569217 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/frontend/TokenStream.h
@@ +306,5 @@
>          u.reflags = flags;
>      }
>  
> +    void setSharpNumber(uint16 sharpNum) {
> +        u.sharpNumber = sharpNum;

Can these assert the same conditions as the getters?
Attachment #569217 - Flags: review?(cdleary) → review+
On second thought, the FIXME in the first patch should probably have a bug number associated with it. r=me with that!
http://hg.mozilla.org/integration/mozilla-inbound/rev/621ef0bc8809
http://hg.mozilla.org/integration/mozilla-inbound/rev/9e519d2d8dab

Filed bug 697000 (=== 1000 mod 0!) for the bug to be noted in a comment.  Really, that problem applies to every mutator, what with the way we have the one out: through which tp->type is set.  So I added the comment at the top of the mutators section.

That's also why setSharpNumber and such can't assert the same condition as its getter, in case it hadn't become obvious after reading the previous paragraph.
=== 0 mod 1000, you mean?
https://hg.mozilla.org/mozilla-central/rev/621ef0bc8809
https://hg.mozilla.org/mozilla-central/rev/9e519d2d8dab
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Er.  Yes.  :-)
You need to log in before you can comment on or make changes to this bug.