2008-12-30 Cameron Zwarich <cwzwarich@uwaterloo.ca>
Reviewed by Sam Weinig.
Bug 23037: Parsing and reparsing disagree on automatic semicolon insertion
<https://bugs.webkit.org/show_bug.cgi?id=23037>
<rdar://problem/6467124>
Parsing and reparsing disagree about automatic semicolon insertion, so that a
function like
function() { a = 1, }
is parsed as being syntactically valid but gets a syntax error upon reparsing.
This leads to an assertion failure in Parser::reparse(). It is not that big of
an issue in practice, because in a Release build such a function will return
'undefined' when called.
In this case, we are not following the spec and it should be a syntax error.
However, unless there is a newline separating the ',' and the '}', WebKit would
not treat it as a syntax error in the past either. It would be a bit of work to
make the automatic semicolon insertion match the spec exactly, so this patch
changes it to match our past behaviour.
The problem is that even during reparsing, the Lexer adds a semicolon at the
end of the input, which confuses allowAutomaticSemicolon(), because it is
expecting either a '}', the end of input, or a terminator like a newline.
JavaScriptCore:
* parser/Lexer.cpp:
(JSC::Lexer::Lexer): Initialize m_isReparsing to false.
(JSC::Lexer::lex): Do not perform automatic semicolon insertion in the Lexer if
we are in the middle of reparsing.
(JSC::Lexer::clear): Set m_isReparsing to false.
* parser/Lexer.h:
(JSC::Lexer::setIsReparsing): Added.
* parser/Parser.cpp:
(JSC::Parser::reparse): Call Lexer::setIsReparsing() to notify the Lexer of
reparsing.
LayoutTests:
* fast/js/reparsing-semicolon-insertion-expected.txt: Added.
* fast/js/reparsing-semicolon-insertion.html: Added.
* fast/js/resources/reparsing-semicolon-insertion.js: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@39521 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 1f29221..52d93e7 100644
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,42 @@
+2008-12-30 Cameron Zwarich <cwzwarich@uwaterloo.ca>
+
+ Reviewed by Sam Weinig.
+
+ Bug 23037: Parsing and reparsing disagree on automatic semicolon insertion
+ <https://bugs.webkit.org/show_bug.cgi?id=23037>
+ <rdar://problem/6467124>
+
+ Parsing and reparsing disagree about automatic semicolon insertion, so that a
+ function like
+
+ function() { a = 1, }
+
+ is parsed as being syntactically valid but gets a syntax error upon reparsing.
+ This leads to an assertion failure in Parser::reparse(). It is not that big of
+ an issue in practice, because in a Release build such a function will return
+ 'undefined' when called.
+
+ In this case, we are not following the spec and it should be a syntax error.
+ However, unless there is a newline separating the ',' and the '}', WebKit would
+ not treat it as a syntax error in the past either. It would be a bit of work to
+ make the automatic semicolon insertion match the spec exactly, so this patch
+ changes it to match our past behaviour.
+
+ The problem is that even during reparsing, the Lexer adds a semicolon at the
+ end of the input, which confuses allowAutomaticSemicolon(), because it is
+ expecting either a '}', the end of input, or a terminator like a newline.
+
+ * parser/Lexer.cpp:
+ (JSC::Lexer::Lexer): Initialize m_isReparsing to false.
+ (JSC::Lexer::lex): Do not perform automatic semicolon insertion in the Lexer if
+ we are in the middle of reparsing.
+ (JSC::Lexer::clear): Set m_isReparsing to false.
+ * parser/Lexer.h:
+ (JSC::Lexer::setIsReparsing): Added.
+ * parser/Parser.cpp:
+ (JSC::Parser::reparse): Call Lexer::setIsReparsing() to notify the Lexer of
+ reparsing.
+
2008-12-29 Oliver Hunt <oliver@apple.com>
Reviewed by NOBODY (Build fix).
diff --git a/JavaScriptCore/parser/Lexer.cpp b/JavaScriptCore/parser/Lexer.cpp
index 3fad519..304da16 100644
--- a/JavaScriptCore/parser/Lexer.cpp
+++ b/JavaScriptCore/parser/Lexer.cpp
@@ -67,6 +67,7 @@
, m_position(0)
, m_code(0)
, m_length(0)
+ , m_isReparsing(false)
, m_atLineStart(true)
, m_current(0)
, m_next1(0)
@@ -192,7 +193,7 @@
shift(1);
m_state = InMultiLineComment;
} else if (m_current == -1) {
- if (!m_terminator && !m_delimited) {
+ if (!m_terminator && !m_delimited && !m_isReparsing) {
// automatic semicolon insertion if program incomplete
token = ';';
m_stackToken = 0;
@@ -890,6 +891,8 @@
newBuffer16.reserveCapacity(initialReadBufferCapacity);
m_buffer16.swap(newBuffer16);
+ m_isReparsing = false;
+
m_pattern = 0;
m_flags = 0;
}
diff --git a/JavaScriptCore/parser/Lexer.h b/JavaScriptCore/parser/Lexer.h
index 3cb7302..931d0d3 100644
--- a/JavaScriptCore/parser/Lexer.h
+++ b/JavaScriptCore/parser/Lexer.h
@@ -36,6 +36,7 @@
class Lexer : Noncopyable {
public:
void setCode(const SourceCode&);
+ void setIsReparsing() { m_isReparsing = true; }
int lex(void* lvalp, void* llocp);
int lineNo() const { return yylineno; }
@@ -141,6 +142,7 @@
const SourceCode* m_source;
const UChar* m_code;
unsigned int m_length;
+ bool m_isReparsing;
int m_atLineStart;
bool m_error;
diff --git a/JavaScriptCore/parser/Parser.cpp b/JavaScriptCore/parser/Parser.cpp
index 2e3fa7a..128068b 100644
--- a/JavaScriptCore/parser/Parser.cpp
+++ b/JavaScriptCore/parser/Parser.cpp
@@ -71,8 +71,10 @@
ASSERT(!functionBodyNode->data());
m_source = &functionBodyNode->source();
+ globalData->lexer->setIsReparsing();
parse(globalData, 0, 0);
ASSERT(m_sourceElements);
+
functionBodyNode->adoptData(std::auto_ptr<ScopeNodeData>(new ScopeNodeData(m_sourceElements.get(),
m_varDeclarations ? &m_varDeclarations->data : 0,
m_funcDeclarations ? &m_funcDeclarations->data : 0,
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index f71e97f..0626b10 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2008-12-30 Cameron Zwarich <cwzwarich@uwaterloo.ca>
+
+ Reviewed by Sam Weinig.
+
+ Add tests for bug 23037: Parsing and reparsing disagree on automatic semicolon insertion
+ <https://bugs.webkit.org/show_bug.cgi?id=23037>
+ <rdar://problem/6467124>
+
+ * fast/js/reparsing-semicolon-insertion-expected.txt: Added.
+ * fast/js/reparsing-semicolon-insertion.html: Added.
+ * fast/js/resources/reparsing-semicolon-insertion.js: Added.
+
2008-12-30 Dan Bernstein <mitz@apple.com>
Reviewed by Adele Peterson.
diff --git a/LayoutTests/fast/js/reparsing-semicolon-insertion-expected.txt b/LayoutTests/fast/js/reparsing-semicolon-insertion-expected.txt
new file mode 100644
index 0000000..421e867
--- /dev/null
+++ b/LayoutTests/fast/js/reparsing-semicolon-insertion-expected.txt
@@ -0,0 +1,12 @@
+This test checks that automatic semicolon insertion for parsing and reparsing agree. In a debug build, this test will fail with an assertion failure if they do not.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS commaTest() is undefined.
+PASS varCommaTest() is undefined.
+PASS constCommaTest() is undefined.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/js/reparsing-semicolon-insertion.html b/LayoutTests/fast/js/reparsing-semicolon-insertion.html
new file mode 100644
index 0000000..2b26cba
--- /dev/null
+++ b/LayoutTests/fast/js/reparsing-semicolon-insertion.html
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="resources/js-test-style.css">
+<script src="resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="resources/reparsing-semicolon-insertion.js"></script>
+<script src="resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/fast/js/resources/reparsing-semicolon-insertion.js b/LayoutTests/fast/js/resources/reparsing-semicolon-insertion.js
new file mode 100644
index 0000000..dc88c81
--- /dev/null
+++ b/LayoutTests/fast/js/resources/reparsing-semicolon-insertion.js
@@ -0,0 +1,25 @@
+description(
+"This test checks that automatic semicolon insertion for parsing and reparsing agree. In a debug build, this test will fail with an assertion failure if they do not."
+);
+
+// According to the ECMA spec, these should all be syntax errors. However, the
+// pre-existing behaviour of JavaScriptCore has always been to accept them. If
+// JavaScriptCore is changed so that these are syntax errors in the future, then
+// this test can simply be changed to reflect that.
+
+// It is important that the closing braces be on the same line as the commas, so
+// that a newline doesn't act as a terminator when lexing inbetween.
+
+function commaTest() { a = 1, }
+
+shouldBeUndefined("commaTest()");
+
+function varCommaTest() { var a = 1, }
+
+shouldBeUndefined("varCommaTest()");
+
+function constCommaTest() { const a = 1, }
+
+shouldBeUndefined("constCommaTest()");
+
+var successfullyParsed = true;
