Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jorendorff, Assigned: jorendorff)

Tracking

unspecified
mozilla24
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Precognitive followup to bug 872735 comment 11.

tokenStream.currentToken().pos is such a common idiom in the Parser that a convenience function is kind of nice.

(I found one use of the idiom that is actually totally unnecessary, in the amazingly twisty Parser::forStatement(). We have an assignment to forHead->pn_pos, and then a few lines later both fields are overwritten. D'oh.)
Posted patch v1 (obsolete) — Splinter Review
Attachment #762741 - Flags: review?(jwalden+bmo)
Assignee: general → jorendorff
Posted patch v2Splinter Review
Just like v1, but eliminate unnecessary 'uint32_t end = pos().end;' in a few places where the temporary 'end' isn't needed anymore to avoid splitting lines.
Attachment #762741 - Attachment is obsolete: true
Attachment #762741 - Flags: review?(jwalden+bmo)
Attachment #762780 - Flags: review?(jwalden+bmo)
Comment on attachment 762780 [details] [diff] [review]
v2

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

::: js/src/frontend/Parser.cpp
@@ +4702,4 @@
>          RootedPropertyName label(context);
>          if (!MatchLabel(context, &tokenStream, &label))
>              return null();
> +        pn = handler.newContinue(label, begin, pos().end);

I'd kind of prefer methods take TokenPos arguments (or const&) rather than begin/end separately -- and there are some methods in here which do this already -- but that's not an immediate concern to fix here.
Attachment #762780 - Flags: review?(jwalden+bmo) → review+
OK, I agree. We'll fix it in bug 883333.
https://hg.mozilla.org/mozilla-central/rev/66207c1bc1e1
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.