883207 - Add Parser::pos()

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

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.)

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: v1

Comment 2

[Jason Orendorff [:jorendorff]]

Attachment: v2

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.

Comment 3

[Jeff Walden [:Waldo]]

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.

Comment 4

[Jason Orendorff [:jorendorff]]

OK, I agree. We'll fix it in bug 883333.

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/66207c1bc1e1