Title: [148167] trunk/Source/_javascript_Core
Revision
148167
Author
[email protected]
Date
2013-04-10 20:17:08 -0700 (Wed, 10 Apr 2013)

Log Message

Unify JSC Parser's error and error message
https://bugs.webkit.org/show_bug.cgi?id=114363

Reviewed by Geoffrey Garen.

The parser kept the error state over two attributes:
error and errorMessage. They were changed in sync,
but had some discrepancy (for example, the error message
was always defined to something).

This patch unifies the two. There is an error if
if the error message is non-null or if the parsing finished
before the end.

This also gets rid of the allocation of the error message
when instantiating a parser.

* parser/Parser.cpp:
(JSC::::Parser):
(JSC::::parseInner):
(JSC::::parseSourceElements):
(JSC::::parseVarDeclaration):
(JSC::::parseConstDeclaration):
(JSC::::parseForStatement):
(JSC::::parseSwitchStatement):
(JSC::::parsePrimaryExpression):
* parser/Parser.h:
(JSC::Parser::updateErrorMessage):
(JSC::Parser::updateErrorWithNameAndMessage):
(JSC::Parser::hasError):
(Parser):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (148166 => 148167)


--- trunk/Source/_javascript_Core/ChangeLog	2013-04-11 03:12:14 UTC (rev 148166)
+++ trunk/Source/_javascript_Core/ChangeLog	2013-04-11 03:17:08 UTC (rev 148167)
@@ -1,3 +1,37 @@
+2013-04-10  Benjamin Poulain  <[email protected]>
+
+        Unify JSC Parser's error and error message
+        https://bugs.webkit.org/show_bug.cgi?id=114363
+
+        Reviewed by Geoffrey Garen.
+
+        The parser kept the error state over two attributes:
+        error and errorMessage. They were changed in sync,
+        but had some discrepancy (for example, the error message
+        was always defined to something).
+
+        This patch unifies the two. There is an error if
+        if the error message is non-null or if the parsing finished
+        before the end.
+
+        This also gets rid of the allocation of the error message
+        when instantiating a parser.
+
+        * parser/Parser.cpp:
+        (JSC::::Parser):
+        (JSC::::parseInner):
+        (JSC::::parseSourceElements):
+        (JSC::::parseVarDeclaration):
+        (JSC::::parseConstDeclaration):
+        (JSC::::parseForStatement):
+        (JSC::::parseSwitchStatement):
+        (JSC::::parsePrimaryExpression):
+        * parser/Parser.h:
+        (JSC::Parser::updateErrorMessage):
+        (JSC::Parser::updateErrorWithNameAndMessage):
+        (JSC::Parser::hasError):
+        (Parser):
+
 2013-04-10  Oliver Hunt  <[email protected]>
 
         Set trap is not being called for API objects

Modified: trunk/Source/_javascript_Core/parser/Parser.cpp (148166 => 148167)


--- trunk/Source/_javascript_Core/parser/Parser.cpp	2013-04-11 03:12:14 UTC (rev 148166)
+++ trunk/Source/_javascript_Core/parser/Parser.cpp	2013-04-11 03:17:08 UTC (rev 148167)
@@ -35,11 +35,11 @@
 #include <wtf/OwnPtr.h>
 #include <wtf/WTFThreadData.h>
 
-#define fail() do { if (!m_error) updateErrorMessage(); return 0; } while (0)
-#define failWithToken(tok) do { if (!m_error) updateErrorMessage(tok); return 0; } while (0)
-#define failWithMessage(msg) do { if (!m_error) updateErrorMessage(msg); return 0; } while (0)
-#define failWithNameAndMessage(before, name, after) do { if (!m_error) updateErrorWithNameAndMessage(before, name, after); return 0; } while (0)
-#define failWithStackOverflow() do { m_error = true; m_hasStackOverflow = true; return 0; } while (0)
+#define fail() do { if (!hasError()) updateErrorMessage(); return 0; } while (0)
+#define failWithToken(tok) do { if (!hasError()) updateErrorMessage(tok); return 0; } while (0)
+#define failWithMessage(msg) do { if (!hasError()) updateErrorMessage(msg); return 0; } while (0)
+#define failWithNameAndMessage(before, name, after) do { if (!hasError()) updateErrorWithNameAndMessage(before, name, after); return 0; } while (0)
+#define failWithStackOverflow() do { updateErrorMessage("Stack exhausted"); m_hasStackOverflow = true; return 0; } while (0)
 #define failIfFalse(cond) do { if (!(cond)) fail(); } while (0)
 #define failIfFalseWithMessage(cond, msg) do { if (!(cond)) failWithMessage(msg); } while (0)
 #define failIfFalseWithNameAndMessage(cond, before, name, msg) do { if (!(cond)) failWithNameAndMessage(before, name, msg); } while (0)
@@ -67,8 +67,6 @@
     , m_source(&source)
     , m_stack(wtfThreadData().stack())
     , m_hasStackOverflow(false)
-    , m_error(false)
-    , m_errorMessage("Parse error")
     , m_allowsIn(true)
     , m_lastLine(0)
     , m_lastTokenEnd(0)
@@ -114,8 +112,12 @@
         m_statementDepth--;
     ScopeRef scope = currentScope();
     SourceElements* sourceElements = parseSourceElements<CheckForStrictMode>(context);
-    if (!sourceElements || !consume(EOFTOK))
-        parseError = m_errorMessage;
+    if (!sourceElements || !consume(EOFTOK)) {
+        if (hasError())
+            parseError = m_errorMessage;
+        else
+            parseError = ASCIILiteral("Parser error");
+    }
 
     IdentifierSet capturedVariables;
     scope->getCapturedVariables(capturedVariables);
@@ -174,7 +176,7 @@
                     next();
                     m_lexer->setLastLineNumber(oldLastLineNumber);
                     m_lexer->setLineNumber(oldLineNumber);
-                    failIfTrue(m_error);
+                    failIfTrue(hasError());
                     continue;
                 }
             } else
@@ -182,9 +184,8 @@
         }
         context.appendStatement(sourceElements, statement);
     }
-    
-    if (m_error)
-        fail();
+
+    failIfTrue(hasError());
     return sourceElements;
 }
 
@@ -200,7 +201,7 @@
     TreeExpression scratch2 = 0;
     int scratch3 = 0;
     TreeExpression varDecls = parseVarDeclarationList(context, scratch, scratch1, scratch2, scratch3, scratch3, scratch3);
-    failIfTrue(m_error);
+    failIfTrue(hasError());
     failIfFalse(autoSemiColon());
     
     return context.createVarStatement(location, varDecls, start, end);
@@ -214,7 +215,7 @@
     int start = tokenLine();
     int end = 0;
     TreeConstDeclList constDecls = parseConstDeclarationList(context);
-    failIfTrue(m_error);
+    failIfTrue(hasError());
     failIfFalse(autoSemiColon());
     
     return context.createConstStatement(location, constDecls, start, end);
@@ -353,9 +354,8 @@
         int initEnd = 0;
         decls = parseVarDeclarationList(context, declarations, forInTarget, forInInitializer, declsStart, initStart, initEnd);
         m_allowsIn = true;
-        if (m_error)
-            fail();
-        
+        failIfTrue(hasError());
+
         // Remainder of a standard for loop is handled identically
         if (match(SEMICOLON))
             goto standardForLoop;
@@ -573,13 +573,13 @@
     consumeOrFail(OPENBRACE);
     startSwitch();
     TreeClauseList firstClauses = parseSwitchClauses(context);
-    failIfTrue(m_error);
+    failIfTrue(hasError());
     
     TreeClause defaultClause = parseSwitchDefaultClause(context);
-    failIfTrue(m_error);
+    failIfTrue(hasError());
     
     TreeClauseList secondClauses = parseSwitchClauses(context);
-    failIfTrue(m_error);
+    failIfTrue(hasError());
     endSwitch();
     consumeOrFail(CLOSEBRACE);
     
@@ -1537,7 +1537,6 @@
         TreeExpression re = context.createRegExp(location, *pattern, *flags, start);
         if (!re) {
             const char* yarrErrorMsg = Yarr::checkSyntax(pattern->string());
-            ASSERT(!m_errorMessage.isNull());
             failWithMessage(yarrErrorMsg);
         }
         return re;

Modified: trunk/Source/_javascript_Core/parser/Parser.h (148166 => 148167)


--- trunk/Source/_javascript_Core/parser/Parser.h	2013-04-11 03:12:14 UTC (rev 148166)
+++ trunk/Source/_javascript_Core/parser/Parser.h	2013-04-11 03:17:08 UTC (rev 148167)
@@ -752,17 +752,16 @@
     
     NEVER_INLINE void updateErrorMessage() 
     {
-        m_error = true;
         const char* name = getTokenName(m_token.m_type);
         if (!name) 
             updateErrorMessageSpecialCase(m_token.m_type);
         else 
             m_errorMessage = String::format("Unexpected token '%s'", name);
+        ASSERT(!m_errorMessage.isNull());
     }
     
     NEVER_INLINE void updateErrorMessage(JSTokenType expectedToken) 
     {
-        m_error = true;
         const char* name = getTokenName(expectedToken);
         if (name)
             m_errorMessage = String::format("Expected token '%s'", name);
@@ -772,18 +771,19 @@
             else
                 updateErrorMessageSpecialCase(expectedToken);
         }
+        ASSERT(!m_errorMessage.isNull());
     }
     
     NEVER_INLINE void updateErrorWithNameAndMessage(const char* beforeMsg, String name, const char* afterMsg)
     {
-        m_error = true;
         m_errorMessage = makeString(beforeMsg, " '", name, "' ", afterMsg);
     }
     
     NEVER_INLINE void updateErrorMessage(const char* msg)
-    {   
-        m_error = true;
+    {
+        ASSERT(msg);
         m_errorMessage = String(msg);
+        ASSERT(!m_errorMessage.isNull());
     }
     
     void startLoop() { currentScope()->startLoop(); }
@@ -889,6 +889,11 @@
         return m_lastTokenEnd;
     }
 
+    bool hasError() const
+    {
+        return !m_errorMessage.isNull();
+    }
+
     JSGlobalData* m_globalData;
     const SourceCode* m_source;
     ParserArena* m_arena;
@@ -896,7 +901,6 @@
     
     StackBounds m_stack;
     bool m_hasStackOverflow;
-    bool m_error;
     String m_errorMessage;
     JSToken m_token;
     bool m_allowsIn;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to