Changeset 86547 in webkit

Timestamp:

May 16, 2011 2:08:07 AM (13 years ago)

Author:

barraclough@apple.com

Message:

​https://bugs.webkit.org/show_bug.cgi?id=60866
Evaluation order broken for empty alternatives in subpatterns

Rubber stamped by Geoff Garen.

Source/JavaScriptCore:

Reverting ​https://bugs.webkit.org/show_bug.cgi?id=51395

• yarr/YarrPattern.cpp:

(JSC::Yarr::YarrPatternConstructor::atomParenthesesEnd):

LayoutTests:

Reverted ​https://bugs.webkit.org/show_bug.cgi?id=51395, and added
test cases for /(|a)/ and /(a|)/, to test the evaluation order of
subpattern matches with empty alternatives.

• fast/regex/parentheses-expected.txt:
• fast/regex/script-tests/parentheses.js:
• fast/regex/script-tests/slow.js:
• fast/regex/slow-expected.txt:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/regex/parentheses-expected.txt (modified) (1 diff)
• LayoutTests/fast/regex/script-tests/parentheses.js (modified) (1 diff)
• LayoutTests/fast/regex/script-tests/slow.js (modified) (1 diff)
• LayoutTests/fast/regex/slow-expected.txt (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/yarr/YarrPattern.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-05-16  Gavin Barraclough  <barraclough@apple.com>
+
+        Rubber stamped by Geoff Garen.
+
+        https://bugs.webkit.org/show_bug.cgi?id=60866
+        Evaluation order broken for empty alternatives in subpatterns
+
+        Reverted https://bugs.webkit.org/show_bug.cgi?id=51395, and added
+        test cases for /(|a)/ and /(a|)/, to test the evaluation order of
+        subpattern matches with empty alternatives.
+
+        * fast/regex/parentheses-expected.txt:
+        * fast/regex/script-tests/parentheses.js:
+        * fast/regex/script-tests/slow.js:
+        * fast/regex/slow-expected.txt:
+
 2011-05-16  Alejandro G. Castro  <alex@igalia.com>
 

trunk/LayoutTests/fast/regex/parentheses-expected.txt

@@ -92 +92 @@
 PASS regexp54.exec('#') is ['','',undefined,undefined,'']
 PASS regexp55.exec('#') is ['','']
+PASS regexp56.exec('a') is ['','']
+PASS regexp57.exec('a') is ['a','a']
 PASS successfullyParsed is true
 

trunk/LayoutTests/fast/regex/script-tests/parentheses.js

@@ -241 +241 @@
 shouldBe("regexp55.exec('#')", "['','']");
 
+// Test evaluation order of empty subpattern alternatives.
+var regexp56 = /(|a)/;
+shouldBe("regexp56.exec('a')", "['','']");
+var regexp57 = /(a|)/;
+shouldBe("regexp57.exec('a')", "['a','a']");
+
 var successfullyParsed = true;
 

trunk/LayoutTests/fast/regex/script-tests/slow.js

@@ -3 +3 @@
 );
 
-shouldBe('/(?:[^(?!)]||){23}z/.test("/(?:[^(?!)]||){23}z/")', 'true');
+shouldBe('/(?:[^(?!)]||){23}z/.test("/(?:[^(?!)]||){23}z/")', 'false');
 
 var successfullyParsed = true;

trunk/LayoutTests/fast/regex/slow-expected.txt

@@ -4 +4 @@
 
 
-PASS /(?:[^(?!)]||){23}z/.test("/(?:[^(?!)]||){23}z/") is true
+PASS /(?:[^(?!)]||){23}z/.test("/(?:[^(?!)]||){23}z/") is false
 PASS successfullyParsed is true
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2011-05-16  Gavin Barraclough  <barraclough@apple.com>
+
+        Rubber stamped by Geoff Garen.
+
+        https://bugs.webkit.org/show_bug.cgi?id=60866
+        Evaluation order broken for empty alternatives in subpatterns
+
+        Reverting https://bugs.webkit.org/show_bug.cgi?id=51395
+
+        * yarr/YarrPattern.cpp:
+        (JSC::Yarr::YarrPatternConstructor::atomParenthesesEnd):
+
 2011-05-15  Gavin Barraclough  <barraclough@apple.com>
 

trunk/Source/JavaScriptCore/yarr/YarrPattern.cpp

@@ -492 +492 @@
         unsigned numParenAlternatives = parenthesesDisjunction->m_alternatives.size();
         unsigned numBOLAnchoredAlts = 0;
-        bool containsEmptyAlternative = false;
 
         for (unsigned i = 0; i < numParenAlternatives; i++) {
-            if (!parenthesesDisjunction->m_alternatives[i]->m_terms.size() && numParenAlternatives > 1) {
-                PatternAlternative* altToRemove = parenthesesDisjunction->m_alternatives[i];
-                parenthesesDisjunction->m_alternatives.remove(i);
-                delete altToRemove;
-                --numParenAlternatives;
-
-                containsEmptyAlternative = true;
-                continue;
-            }
-
             // Bubble up BOL flags
             if (parenthesesDisjunction->m_alternatives[i]->m_startsWithBOL)
@@ -519 +508 @@
         lastTerm.parentheses.lastSubpatternId = m_pattern.m_numSubpatterns;
         m_invertParentheticalAssertion = false;
-
-        if (containsEmptyAlternative) {
-            // Backup and remove the current disjunction's alternatives.
-            Vector<PatternAlternative*> alternatives;
-            alternatives.append(parenthesesDisjunction->m_alternatives);
-            parenthesesDisjunction->m_alternatives.clear();
-            PatternAlternative* alternative = parenthesesDisjunction->addNewAlternative();
-
-            // Insert a new non-capturing parentheses.
-            unsigned subpatternId = m_pattern.m_numSubpatterns + 1;
-            PatternDisjunction* newDisjunction = new PatternDisjunction(alternative);
-            m_pattern.m_disjunctions.append(newDisjunction);
-            alternative->m_terms.append(PatternTerm(PatternTerm::TypeParenthesesSubpattern, subpatternId, newDisjunction, false, false));
-            newDisjunction->m_alternatives.append(alternatives);
-
-            // Set the quantifier of the new parentheses to '?' and set the inherited properties.
-            PatternTerm& disjunctionTerm = alternative->lastTerm();
-            disjunctionTerm.quantify(1, QuantifierGreedy);
-            disjunctionTerm.parentheses.lastSubpatternId = m_pattern.m_numSubpatterns;
-            alternative->m_containsBOL = m_alternative->m_containsBOL;
-            alternative->m_startsWithBOL = m_alternative->m_startsWithBOL;
-        }
     }