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

Review URL: http://codereview.chromium.org/7218009

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8351 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/parser.cc b/src/parser.cc
index 3348391..8a2258f 100644 (file)
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -1776,7 +1776,7 @@ Statement* Parser::ParseExpressionOrLabelledStatement(ZoneStringList* labels,
   // no line-terminator between the two words.
   if (extension_ != NULL &&
       peek() == Token::FUNCTION &&
-      !scanner().has_line_terminator_before_next() &&
+      !scanner().HasAnyLineTerminatorBeforeNext() &&
       expr != NULL &&
       expr->AsVariableProxy() != NULL &&
       expr->AsVariableProxy()->name()->Equals(
@@ -1818,7 +1818,7 @@ Statement* Parser::ParseContinueStatement(bool* ok) {
   Expect(Token::CONTINUE, CHECK_OK);
   Handle<String> label = Handle<String>::null();
   Token::Value tok = peek();
-  if (!scanner().has_line_terminator_before_next() &&
+  if (!scanner().HasAnyLineTerminatorBeforeNext() &&
       tok != Token::SEMICOLON && tok != Token::RBRACE && tok != Token::EOS) {
     label = ParseIdentifier(CHECK_OK);
   }
@@ -1848,7 +1848,7 @@ Statement* Parser::ParseBreakStatement(ZoneStringList* labels, bool* ok) {
   Expect(Token::BREAK, CHECK_OK);
   Handle<String> label;
   Token::Value tok = peek();
-  if (!scanner().has_line_terminator_before_next() &&
+  if (!scanner().HasAnyLineTerminatorBeforeNext() &&
       tok != Token::SEMICOLON && tok != Token::RBRACE && tok != Token::EOS) {
     label = ParseIdentifier(CHECK_OK);
   }
@@ -1897,7 +1897,7 @@ Statement* Parser::ParseReturnStatement(bool* ok) {
   }
 
   Token::Value tok = peek();
-  if (scanner().has_line_terminator_before_next() ||
+  if (scanner().HasAnyLineTerminatorBeforeNext() ||
       tok == Token::SEMICOLON ||
       tok == Token::RBRACE ||
       tok == Token::EOS) {
@@ -2032,7 +2032,7 @@ Statement* Parser::ParseThrowStatement(bool* ok) {
 
   Expect(Token::THROW, CHECK_OK);
   int pos = scanner().location().beg_pos;
-  if (scanner().has_line_terminator_before_next()) {
+  if (scanner().HasAnyLineTerminatorBeforeNext()) {
     ReportMessage("newline_after_throw", Vector<const char*>::empty());
     *ok = false;
     return NULL;
@@ -2619,7 +2619,7 @@ Expression* Parser::ParsePostfixExpression(bool* ok) {
   //   LeftHandSideExpression ('++' | '--')?
 
   Expression* expression = ParseLeftHandSideExpression(CHECK_OK);
-  if (!scanner().has_line_terminator_before_next() &&
+  if (!scanner().HasAnyLineTerminatorBeforeNext() &&
       Token::IsCountOp(peek())) {
     // Signal a reference error if the expression is an invalid
     // left-hand side expression.  We could report this as a syntax
@@ -3818,7 +3818,7 @@ void Parser::ExpectSemicolon(bool* ok) {
     Next();
     return;
   }
-  if (scanner().has_line_terminator_before_next() ||
+  if (scanner().HasAnyLineTerminatorBeforeNext() ||
       tok == Token::RBRACE ||
       tok == Token::EOS) {
     return;

diff --git a/src/preparser-api.cc b/src/preparser-api.cc
index a0d13ed..1a7402f 100644 (file)
--- a/src/preparser-api.cc
+++ b/src/preparser-api.cc
@@ -169,6 +169,7 @@ class StandAloneJavaScriptScanner : public JavaScriptScanner {
     // Skip initial whitespace allowing HTML comment ends just like
     // after a newline and scan first token.
     has_line_terminator_before_next_ = true;
+    has_multiline_comment_before_next_ = false;
     SkipWhiteSpace();
     Scan();
   }

diff --git a/src/preparser.cc b/src/preparser.cc
index 818f02a..7553430 100644 (file)
--- a/src/preparser.cc
+++ b/src/preparser.cc
@@ -383,7 +383,7 @@ PreParser::Statement PreParser::ParseContinueStatement(bool* ok) {
 
   Expect(i::Token::CONTINUE, CHECK_OK);
   i::Token::Value tok = peek();
-  if (!scanner_->has_line_terminator_before_next() &&
+  if (!scanner_->HasAnyLineTerminatorBeforeNext() &&
       tok != i::Token::SEMICOLON &&
       tok != i::Token::RBRACE &&
       tok != i::Token::EOS) {
@@ -400,7 +400,7 @@ PreParser::Statement PreParser::ParseBreakStatement(bool* ok) {
 
   Expect(i::Token::BREAK, CHECK_OK);
   i::Token::Value tok = peek();
-  if (!scanner_->has_line_terminator_before_next() &&
+  if (!scanner_->HasAnyLineTerminatorBeforeNext() &&
       tok != i::Token::SEMICOLON &&
       tok != i::Token::RBRACE &&
       tok != i::Token::EOS) {
@@ -426,7 +426,7 @@ PreParser::Statement PreParser::ParseReturnStatement(bool* ok) {
   // This is not handled during preparsing.
 
   i::Token::Value tok = peek();
-  if (!scanner_->has_line_terminator_before_next() &&
+  if (!scanner_->HasAnyLineTerminatorBeforeNext() &&
       tok != i::Token::SEMICOLON &&
       tok != i::Token::RBRACE &&
       tok != i::Token::EOS) {
@@ -577,7 +577,7 @@ PreParser::Statement PreParser::ParseThrowStatement(bool* ok) {
   //   'throw' [no line terminator] Expression ';'
 
   Expect(i::Token::THROW, CHECK_OK);
-  if (scanner_->has_line_terminator_before_next()) {
+  if (scanner_->HasAnyLineTerminatorBeforeNext()) {
     i::JavaScriptScanner::Location pos = scanner_->location();
     ReportMessageAt(pos.beg_pos, pos.end_pos,
                     "newline_after_throw", NULL);
@@ -800,7 +800,7 @@ PreParser::Expression PreParser::ParsePostfixExpression(bool* ok) {
 
   i::Scanner::Location before = scanner_->peek_location();
   Expression expression = ParseLeftHandSideExpression(CHECK_OK);
-  if (!scanner_->has_line_terminator_before_next() &&
+  if (!scanner_->HasAnyLineTerminatorBeforeNext() &&
       i::Token::IsCountOp(peek())) {
     if (strict_mode() && expression.IsIdentifier() &&
         expression.AsIdentifier().IsEvalOrArguments()) {
@@ -1274,7 +1274,7 @@ void PreParser::ExpectSemicolon(bool* ok) {
     Next();
     return;
   }
-  if (scanner_->has_line_terminator_before_next() ||
+  if (scanner_->HasAnyLineTerminatorBeforeNext() ||
       tok == i::Token::RBRACE ||
       tok == i::Token::EOS) {
     return;

diff --git a/src/scanner-base.cc b/src/scanner-base.cc
index 89591ba..a55f0f4 100644 (file)
--- a/src/scanner-base.cc
+++ b/src/scanner-base.cc
@@ -80,6 +80,7 @@ JavaScriptScanner::JavaScriptScanner(UnicodeCache* scanner_contants)
 Token::Value JavaScriptScanner::Next() {
   current_ = next_;
   has_line_terminator_before_next_ = false;
+  has_multiline_comment_before_next_ = false;
   Scan();
   return current_.token;
 }
@@ -163,7 +164,7 @@ Token::Value JavaScriptScanner::SkipMultiLineComment() {
     if (unicode_cache_->IsLineTerminator(ch)) {
       // Following ECMA-262, section 7.4, a comment containing
       // a newline will make the comment count as a line-terminator.
-      has_line_terminator_before_next_ = true;
+      has_multiline_comment_before_next_ = true;
     }
     // If we have reached the end of the multi-line comment, we
     // consume the '/' and insert a whitespace. This way all
@@ -449,6 +450,7 @@ void JavaScriptScanner::SeekForward(int pos) {
     // of the end of a function (at the "}" token). It doesn't matter
     // whether there was a line terminator in the part we skip.
     has_line_terminator_before_next_ = false;
+    has_multiline_comment_before_next_ = false;
   }
   Scan();
 }

diff --git a/src/scanner-base.h b/src/scanner-base.h
index 02566dd..2808a97 100644 (file)
--- a/src/scanner-base.h
+++ b/src/scanner-base.h
@@ -474,9 +474,11 @@ class JavaScriptScanner : public Scanner {
   // Returns the next token.
   Token::Value Next();
 
-  // Returns true if there was a line terminator before the peek'ed token.
-  bool has_line_terminator_before_next() const {
-    return has_line_terminator_before_next_;
+  // Returns true if there was a line terminator before the peek'ed token,
+  // possibly inside a multi-line comment.
+  bool HasAnyLineTerminatorBeforeNext() const {
+    return has_line_terminator_before_next_ ||
+           has_multiline_comment_before_next_;
   }
 
   // Scans the input as a regular expression pattern, previous
@@ -529,7 +531,13 @@ class JavaScriptScanner : public Scanner {
   // Start position of the octal literal last scanned.
   Location octal_pos_;
 
+  // Whether there is a line terminator whitespace character after
+  // the current token, and  before the next. Does not count newlines
+  // inside multiline comments.
   bool has_line_terminator_before_next_;
+  // Whether there is a multi-line comment that contains a
+  // line-terminator after the current token, and before the next.
+  bool has_multiline_comment_before_next_;
 };
 
 

diff --git a/src/scanner.cc b/src/scanner.cc
index 21a0c2d..844db1b 100755 (executable)
--- a/src/scanner.cc
+++ b/src/scanner.cc
@@ -337,6 +337,7 @@ void V8JavaScriptScanner::Initialize(UC16CharacterStream* source) {
   // Skip initial whitespace allowing HTML comment ends just like
   // after a newline and scan first token.
   has_line_terminator_before_next_ = true;
+  has_multiline_comment_before_next_ = false;
   SkipWhiteSpace();
   Scan();
 }

diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc
index 08f117b..2b707fc 100755 (executable)
--- a/test/cctest/test-parsing.cc
+++ b/test/cctest/test-parsing.cc
@@ -137,8 +137,9 @@ TEST(ScanHTMLEndComments) {
   // Regression test. See:
   //    http://code.google.com/p/chromium/issues/detail?id=53548
   // Tests that --> is correctly interpreted as comment-to-end-of-line if there
-  // is only whitespace before it on the line, even after a multiline-comment
-  // comment. This was not the case if it occurred before the first real token
+  // is only whitespace before it on the line (with comments considered as
+  // whitespace, even a multiline-comment containing a newline).
+  // This was not the case if it occurred before the first real token
   // in the input.
   const char* tests[] = {
       // Before first real token.
@@ -152,6 +153,16 @@ TEST(ScanHTMLEndComments) {
       NULL
   };
 
+  const char* fail_tests[] = {
+      "x --> is eol-comment\nvar y = 37;\n",
+      "\"\\n\" --> is eol-comment\nvar y = 37;\n",
+      "x/* precomment */ --> is eol-comment\nvar y = 37;\n",
+      "x/* precomment\n */ --> is eol-comment\nvar y = 37;\n",
+      "var x = 42; --> is eol-comment\nvar y = 37;\n",
+      "var x = 42; /* precomment\n */ --> is eol-comment\nvar y = 37;\n",
+      NULL
+  };
+
   // Parser/Scanner needs a stack limit.
   int marker;
   i::Isolate::Current()->stack_guard()->SetStackLimit(
@@ -163,6 +174,13 @@ TEST(ScanHTMLEndComments) {
     CHECK(data != NULL && !data->HasError());
     delete data;
   }
+
+  for (int i = 0; fail_tests[i]; i++) {
+    v8::ScriptData* data =
+        v8::ScriptData::PreCompile(fail_tests[i], i::StrLength(fail_tests[i]));
+    CHECK(data == NULL || data->HasError());
+    delete data;
+  }
 }