Make multi-line comments not count when checking whether --> is first on a line.

Make multi-line comments not count when checking whether --> is first on a line.

A multi-line comment containing a newline is considered a line-terminator for
other purposes, but a "-->" following such a comment is considered as being
on the same line as the text preceeding the multi-line comment.
This behavior matches JSC matching Firefox.

TEST=cctest/test-parsing/ScanHTMLEndComments

Committed: http://code.google.com/p/v8/source/detail?r=8351

[William Hesse, 2011-06-21 11:37:41]

LGTM, but I suggest better names for the flags.

http://codereview.chromium.org/7218009/diff/1/src/scanner-base.cc
File src/scanner-base.cc (right):

http://codereview.chromium.org/7218009/diff/1/src/scanner-base.cc#newcode83
src/scanner-base.cc:83: next_is_first_on_line_ = false;
Could we call these things something more parallel, like:
has_line_terminator_before_next and
has_line_terminator_outside_comment_before_next
or
saw_line_terminator_outside_comment
saw_line_terminator_inside_comment?

The meanings and algorithm would be clearer.

Or better yet:
newline_before_current_token  and
multiline_comment_before_current_token.
The condition for handling --> would be newline_before...
and the condition for automatic semicolon insertion
would be newline_before... || multiline_comment_before...

[William Hesse, 2011-06-21 12:38:54]

LGTM, with comments.

http://codereview.chromium.org/7218009/diff/2001/src/scanner-base.cc
File src/scanner-base.cc (right):

http://codereview.chromium.org/7218009/diff/2001/src/scanner-base.cc#newcode172
src/scanner-base.cc:172: // when checking whether there is non-whitespace before
a
This addition is wrong.  They are treated as whitespace in all cases.

http://codereview.chromium.org/7218009/diff/2001/src/scanner-base.h
File src/scanner-base.h (right):

http://codereview.chromium.org/7218009/diff/2001/src/scanner-base.h#newcode479
src/scanner-base.h:479: bool has_line_terminator_before_next() const {
You can't have a getter which does not get the actual variable with the same
name, but the value of an expression.
I would call this "should_insert_implicit_semicolon"

http://codereview.chromium.org/7218009/diff/2001/src/scanner-base.h#newcode481
src/scanner-base.h:481: // multi-line comments only matter inside the scanner.
I don't think this sentence makes sense or adds anything.  I think you mean that
"newlines inside multi-line comments only affect semicolon insertion."

http://codereview.chromium.org/7218009/diff/2001/src/scanner-base.h#newcode540
src/scanner-base.h:540: // Whether there is a multi-line comment (actually
containing a
... comment that actually contains a newline ...

(restrictive "that" clause rather than non-restrictive "which" clause or
parenthetical clause)

[Lasse Reichstein, 2011-06-21 13:05:37]

http://codereview.chromium.org/7218009/diff/2001/src/scanner-base.cc
File src/scanner-base.cc (right):

http://codereview.chromium.org/7218009/diff/2001/src/scanner-base.cc#newcode172
src/scanner-base.cc:172: // when checking whether there is non-whitespace before
a
Fixed.

http://codereview.chromium.org/7218009/diff/2001/src/scanner-base.h
File src/scanner-base.h (right):

http://codereview.chromium.org/7218009/diff/2001/src/scanner-base.h#newcode479
src/scanner-base.h:479: bool has_line_terminator_before_next() const {
That would be using parser-level terminology in the scanner.

I'll rename the function to not look like a getter, and rename the variable.

http://codereview.chromium.org/7218009/diff/2001/src/scanner-base.h#newcode481
src/scanner-base.h:481: // multi-line comments only matter inside the scanner.
Again, semicolon-insertion is a parser-level concept, so it I won't refer to it
in the scanner (unless absolutely necessary).
I'll drop the explanation now that the names are different.

http://codereview.chromium.org/7218009/diff/2001/src/scanner-base.h#newcode540
src/scanner-base.h:540: // Whether there is a multi-line comment (actually
containing a
Done.

[William Hesse, 2011-06-21 13:29:38]

LGTM.