JavaScriptCore:
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Nov 2007 17:18:39 +0000 (17:18 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Nov 2007 17:18:39 +0000 (17:18 +0000)
        Reviewed by Darin Adler.

        Fixed part of http://bugs.webkit.org/show_bug.cgi?id=15861
        15% of string-validate-input.js is spent compiling the same regular expression.

        Put RegExpImp properties into a static hashtable to avoid a slew of
        PropertyMap churn when creating a RegExpImp.

        Factored important bits of regular expression implementation out of
        RegExpImp (the JS object) and into RegExp (the PCRE wrapper class),
        making RegExp a ref-counted class. (This will help later.)

        Removed PCRE_POSIX support because I didn't quite know how to test it
        and keep it working with these changes.

        1.1% SunSpider speedup. 5.8% speedup on string-validate-input.js.

        * kjs/regexp.h: A few interface changes:
        1. Renamed "subpatterns()" => "numSubpatterns()"
        2. Made flag enumeration private and replaced it with public getters for
        specific flags.
        3. Made RegExp ref-counted so RegExps can be shared by RegExpImps.
        4. Made RegExp take a string of flags instead of an int, eliminating
        duplicated flag parsing code elsewhere.

        * kjs/regexp_object.cpp:
        (KJS::RegExpProtoFunc::callAsFunction): For RegExp.compile:
        - Fixed a bug where compile(undefined) would throw an exception.
        - Removed some now-redundant code.
        - Used RegExp sharing to eliminate an allocation and a bunch of
        PropertyMap thrash. (Not a big win since compile is a deprecated
        function. I mainly did this to test the plubming.)

LayoutTests:

        Reviewed by Darin Adler.

        Beefed up the RegExp.compile testcase to cover a mistake in the
        original check-in and a mistake I made while developing my new patch.

        * fast/js/resources/regexp-compile.js:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@27571 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JavaScriptCore/ChangeLog
JavaScriptCore/kjs/regexp.cpp
JavaScriptCore/kjs/regexp.h
JavaScriptCore/kjs/regexp_object.cpp
JavaScriptCore/kjs/regexp_object.h
JavaScriptCore/kjs/string_object.cpp
LayoutTests/ChangeLog
LayoutTests/fast/js/regexp-compile-expected.txt
LayoutTests/fast/js/resources/regexp-compile.js

index 3c135bbe38df69107be2b855bcfb7970f851c094..528694b03162423ffd4ce99dabd383fa9f58fbe3 100644 (file)
@@ -1,3 +1,38 @@
+2007-11-06  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Darin Adler.
+        
+        Fixed part of http://bugs.webkit.org/show_bug.cgi?id=15861 
+        15% of string-validate-input.js is spent compiling the same regular expression.
+
+        Put RegExpImp properties into a static hashtable to avoid a slew of
+        PropertyMap churn when creating a RegExpImp.
+        
+        Factored important bits of regular expression implementation out of
+        RegExpImp (the JS object) and into RegExp (the PCRE wrapper class), 
+        making RegExp a ref-counted class. (This will help later.)
+
+        Removed PCRE_POSIX support because I didn't quite know how to test it 
+        and keep it working with these changes.
+        
+        1.1% SunSpider speedup. 5.8% speedup on string-validate-input.js.
+
+        * kjs/regexp.h: A few interface changes:
+        1. Renamed "subpatterns()" => "numSubpatterns()"
+        2. Made flag enumeration private and replaced it with public getters for
+        specific flags.
+        3. Made RegExp ref-counted so RegExps can be shared by RegExpImps.
+        4. Made RegExp take a string of flags instead of an int, eliminating 
+        duplicated flag parsing code elsewhere.
+
+        * kjs/regexp_object.cpp:
+        (KJS::RegExpProtoFunc::callAsFunction): For RegExp.compile: 
+        - Fixed a bug where compile(undefined) would throw an exception. 
+        - Removed some now-redundant code.
+        - Used RegExp sharing to eliminate an allocation and a bunch of 
+        PropertyMap thrash. (Not a big win since compile is a deprecated 
+        function. I mainly did this to test the plubming.)
+
 2007-11-07  Simon Hausmann  <hausmann@kde.org>
 
         Reviewed by nobody, Qt/Windows build fix.
index a39b641df2a08ff8a6bbcdf12a2b7bbc5f5fcd01..8a332b7c85d61584e3e8c4da4f6ff34459575cc7 100644 (file)
 
 namespace KJS {
 
-RegExp::RegExp(const UString &p, int flags)
-  : m_flags(flags), m_constructionError(0), m_numSubPatterns(0)
+RegExp::RegExp(const UString& pattern)
+  : m_refCount(0)
+  , m_pattern(pattern)
+  , m_flags(0)
+  , m_constructionError(0)
+  , m_numSubpatterns(0)
 {
-#if !USE(POSIX_REGEX)
-
-  int options = 0;
-  if (flags & IgnoreCase)
-    options |= JS_REGEXP_CASELESS;
-  if (flags & Multiline)
-    options |= JS_REGEXP_MULTILINE;
-
-  const char* errorMessage;
-  m_regex = jsRegExpCompile(reinterpret_cast<const JSRegExpChar*>(p.data()), p.size(), options,
-    &m_numSubPatterns, &errorMessage);
-  if (!m_regex) {
-    m_constructionError = strdup(errorMessage);
-    return;
-  }
-
-#else /* USE(POSIX_REGEX) */
-
-  int regflags = 0;
-#ifdef REG_EXTENDED
-  regflags |= REG_EXTENDED;
-#endif
-#ifdef REG_ICASE
-  if (flags & IgnoreCase)
-    regflags |= REG_ICASE;
-#endif
-
-  //NOTE: Multiline is not feasible with POSIX regex.
-  //if ( f & Multiline )
-  //    ;
-  // Note: the Global flag is already handled by RegExpProtoFunc::execute
-
-  // FIXME: support \u Unicode escapes.
-
-  int errorCode = regcomp(&m_regex, p.ascii(), regflags);
-  if (errorCode != 0) {
-    char errorMessage[80];
-    regerror(errorCode, &m_regex, errorMessage, sizeof errorMessage);
-    m_constructionError = strdup(errorMessage);
-  }
+    const char* errorMessage;
+    m_regExp = jsRegExpCompile(reinterpret_cast<const JSRegExpChar*>(m_pattern.data()), m_pattern.size(), 0, &m_numSubpatterns, &errorMessage);
+    if (!m_regExp)
+        m_constructionError = strdup(errorMessage);
+}
 
-#endif
+RegExp::RegExp(const UString& pattern, const UString& flags)
+  : m_refCount(0)
+  , m_pattern(pattern)
+  , m_flags(0)
+  , m_constructionError(0)
+  , m_numSubpatterns(0)
+{
+    // NOTE: The global flag is handled on a case-by-case basis by functions like
+    // String::match and RegExpImp::match.
+    if (flags.find('g') != -1)
+        m_flags |= Global;
+
+    // FIXME: Eliminate duplication by adding a way ask a JSRegExp what its flags are.
+    int options = 0;
+    if (flags.find('i') != -1) {
+        m_flags |= IgnoreCase;
+        options |= JS_REGEXP_CASELESS;
+    }
+
+    if (flags.find('m') != -1) {
+        m_flags |= Multiline;
+        options |= JS_REGEXP_MULTILINE;
+    }
+    
+    const char* errorMessage;
+    m_regExp = jsRegExpCompile(reinterpret_cast<const JSRegExpChar*>(m_pattern.data()), m_pattern.size(), options, &m_numSubpatterns, &errorMessage);
+    if (!m_regExp)
+        m_constructionError = strdup(errorMessage);
 }
 
 RegExp::~RegExp()
 {
-#if !USE(POSIX_REGEX)
-  jsRegExpFree(m_regex);
-#else
-  /* TODO: is this really okay after an error ? */
-  regfree(&m_regex);
-#endif
+  jsRegExpFree(m_regExp);
   free(m_constructionError);
 }
 
@@ -98,9 +89,7 @@ int RegExp::match(const UString& s, int i, OwnArrayPtr<int>* ovector)
   if (i > s.size() || s.isNull())
     return -1;
 
-#if !USE(POSIX_REGEX)
-
-  if (!m_regex)
+  if (!m_regExp)
     return -1;
 
   // Set up the offset vector for the result.
@@ -112,12 +101,12 @@ int RegExp::match(const UString& s, int i, OwnArrayPtr<int>* ovector)
     offsetVectorSize = 3;
     offsetVector = fixedSizeOffsetVector;
   } else {
-    offsetVectorSize = (m_numSubPatterns + 1) * 3;
+    offsetVectorSize = (m_numSubpatterns + 1) * 3;
     offsetVector = new int [offsetVectorSize];
     ovector->set(offsetVector);
   }
 
-  int numMatches = jsRegExpExecute(m_regex, reinterpret_cast<const JSRegExpChar*>(s.data()), s.size(), i, offsetVector, offsetVectorSize);
+  int numMatches = jsRegExpExecute(m_regExp, reinterpret_cast<const JSRegExpChar*>(s.data()), s.size(), i, offsetVector, offsetVectorSize);
 
   if (numMatches < 0) {
 #ifndef NDEBUG
@@ -130,41 +119,6 @@ int RegExp::match(const UString& s, int i, OwnArrayPtr<int>* ovector)
   }
 
   return offsetVector[0];
-
-#else
-
-  const unsigned maxMatch = 10;
-  regmatch_t rmatch[maxMatch];
-
-  char *str = strdup(s.ascii()); // TODO: why ???
-  if (regexec(&m_regex, str + i, maxMatch, rmatch, 0)) {
-    free(str);
-    return UString::null();
-  }
-  free(str);
-
-  if (!ovector) {
-    *pos = rmatch[0].rm_so + i;
-    return s.substr(rmatch[0].rm_so + i, rmatch[0].rm_eo - rmatch[0].rm_so);
-  }
-
-  // map rmatch array to ovector used in PCRE case
-  m_numSubPatterns = 0;
-  for(unsigned j = 1; j < maxMatch && rmatch[j].rm_so >= 0 ; j++)
-      m_numSubPatterns++;
-  int ovecsize = (m_numSubPatterns+1)*3; // see above
-  *ovector = new int[ovecsize];
-  for (unsigned j = 0; j < m_numSubPatterns + 1; j++) {
-    if (j>maxMatch)
-      break;
-    (*ovector)[2*j] = rmatch[j].rm_so + i;
-    (*ovector)[2*j+1] = rmatch[j].rm_eo + i;
-  }
-
-  *pos = (*ovector)[0];
-  return s.substr((*ovector)[0], (*ovector)[1] - (*ovector)[0]);
-
-#endif
 }
 
 } // namespace KJS
index b50ce48d348a2f909e633dbc926011fa44ecaeed..a0420825014a96cf9e8c65b12392389e1739fc7b 100644 (file)
 #ifndef KJS_REGEXP_H
 #define KJS_REGEXP_H
 
-#include <sys/types.h>
-
-#if !USE(POSIX_REGEX)
-#include <pcre.h>
-#else
-// POSIX regex - not so good.
-extern "C" { // bug with some libc5 distributions
-#include <regex.h>
-}
-#endif
-
 #include "ustring.h"
+#include <pcre.h>
+#include <sys/types.h>
 #include <wtf/OwnArrayPtr.h>
 
 namespace KJS {
 
   class RegExp : Noncopyable {
-  public:
-    enum { None = 0, Global = 1, IgnoreCase = 2, Multiline = 4 };
+  private:
+    enum { 
+        Global = 1, 
+        IgnoreCase = 2, 
+        Multiline = 4 
+    };
 
-    RegExp(const UString& pattern, int flags = None);
+  public:
+    RegExp(const UString& pattern);
+    RegExp(const UString& pattern, const UString& flags);
     ~RegExp();
+    
+    void ref() { ++m_refCount; }
+    void deref() { if (--m_refCount == 0) delete this; }
+    int refCount() { return m_refCount; }
+
+    bool global() const { return m_flags & Global; }
+    bool ignoreCase() const { return m_flags & IgnoreCase; }
+    bool multiline() const { return m_flags & Multiline; }
+    const UString& pattern() const { return m_pattern; }
 
-    int flags() const { return m_flags; }
     bool isValid() const { return !m_constructionError; }
     const char* errorMessage() const { return m_constructionError; }
 
     int match(const UString&, int offset, OwnArrayPtr<int>* ovector = 0);
-    unsigned subPatterns() const { return m_numSubPatterns; }
+    unsigned numSubpatterns() const { return m_numSubpatterns; }
 
   private:
-#if !USE(POSIX_REGEX)
-    JSRegExp* m_regex;
-#else
-    regex_t m_regex;
-#endif
+    void compile();
+    
+    int m_refCount;
+    
+    // Data supplied by caller.
+    UString m_pattern;
     int m_flags;
+
+    // Data supplied by PCRE.
+    JSRegExp* m_regExp;
     char* m_constructionError;
-    unsigned m_numSubPatterns;
+    unsigned m_numSubpatterns;
   };
 
 } // namespace
index 5fc0365ecfbe2a124990027e34bb2362d64d94a8..143cc543a139f3b71e26c118ccd470ce2d64552f 100644 (file)
@@ -84,52 +84,25 @@ JSValue *RegExpProtoFunc::callAsFunction(ExecState *exec, JSObject *thisObj, con
             return static_cast<RegExpImp*>(thisObj)->exec(exec, args);
   case Compile:
   {
-    UString source;
-    bool global = false;
-    bool ignoreCase = false;
-    bool multiline = false;
-    if (args.size() > 0) {
-      if (args[0]->isObject(&RegExpImp::info)) {
-        if (args.size() != 1)
-          return throwError(exec, TypeError, "cannot supply flags when constructing one RegExp from another.");
-
-        // Flags are mirrored on the JS object and in the implementation, while source is only preserved on the JS object.
-        RegExp* rhsRegExp = static_cast<RegExpImp*>(args[0])->regExp();
-        global = rhsRegExp->flags() & RegExp::Global;
-        ignoreCase = rhsRegExp->flags() & RegExp::IgnoreCase;
-        multiline = rhsRegExp->flags() & RegExp::Multiline;
-        source = static_cast<RegExpImp*>(args[0])->get(exec, exec->propertyNames().source)->toString(exec);
-      } else
-        source = args[0]->toString(exec);
-
-      if (!args[1]->isUndefined()) {
-        UString flags = args[1]->toString(exec);
-
-        global = (flags.find("g") >= 0);
-        ignoreCase = (flags.find("i") >= 0);
-        multiline = (flags.find("m") >= 0);
-      }
+    RefPtr<RegExp> regExp;
+    JSValue* arg0 = args[0];
+    JSValue* arg1 = args[1];
+    
+    if (arg0->isObject(&RegExpImp::info)) {
+      if (!arg1->isUndefined())
+        return throwError(exec, TypeError, "Cannot supply flags when constructing one RegExp from another.");
+      regExp = static_cast<RegExpImp*>(arg0)->regExp();
+    } else {
+      UString pattern = args.isEmpty() ? UString("") : arg0->toString(exec);
+      UString flags = arg1->isUndefined() ? UString("") : arg1->toString(exec);
+      regExp = new RegExp(pattern, flags);
     }
 
-    int reflags = RegExp::None;
-    if (global)
-        reflags |= RegExp::Global;
-    if (ignoreCase)
-        reflags |= RegExp::IgnoreCase;
-    if (multiline)
-        reflags |= RegExp::Multiline;
-
-    OwnPtr<RegExp> newRegExp(new RegExp(source, reflags));
-    if (!newRegExp->isValid())
-        return throwError(exec, SyntaxError, UString("Invalid regular expression: ").append(newRegExp->errorMessage()));
-
-    thisObj->putDirect(exec->propertyNames().global, jsBoolean(global), DontDelete | ReadOnly | DontEnum);
-    thisObj->putDirect(exec->propertyNames().ignoreCase, jsBoolean(ignoreCase), DontDelete | ReadOnly | DontEnum);
-    thisObj->putDirect(exec->propertyNames().multiline, jsBoolean(multiline), DontDelete | ReadOnly | DontEnum);
-    thisObj->putDirect(exec->propertyNames().source, jsString(source), DontDelete | ReadOnly | DontEnum);
-    thisObj->putDirect(exec->propertyNames().lastIndex, jsNumber(0), DontDelete | DontEnum);
-
-    static_cast<RegExpImp*>(thisObj)->setRegExp(newRegExp.release());
+    if (!regExp->isValid())
+      return throwError(exec, SyntaxError, UString("Invalid regular expression: ").append(regExp->errorMessage()));
+
+    static_cast<RegExpImp*>(thisObj)->setRegExp(regExp.release());
+    static_cast<RegExpImp*>(thisObj)->put(exec, exec->propertyNames().lastIndex, jsNumber(0), DontDelete|DontEnum);
     return jsUndefined();
   }
   case ToString:
@@ -151,11 +124,22 @@ JSValue *RegExpProtoFunc::callAsFunction(ExecState *exec, JSObject *thisObj, con
 
 // ------------------------------ RegExpImp ------------------------------------
 
-const ClassInfo RegExpImp::info = { "RegExp", 0, 0 };
+const ClassInfo RegExpImp::info = { "RegExp", 0, &RegExpImpTable };
+
+/* Source for regexp_object.lut.h
+@begin RegExpImpTable 5
+    global        RegExpImp::Global       DontDelete|ReadOnly|DontEnum
+    ignoreCase    RegExpImp::IgnoreCase   DontDelete|ReadOnly|DontEnum
+    multiline     RegExpImp::Multiline    DontDelete|ReadOnly|DontEnum
+    source        RegExpImp::Source       DontDelete|ReadOnly|DontEnum
+    lastIndex     RegExpImp::LastIndex    DontDelete|DontEnum
+@end
+*/
 
-RegExpImp::RegExpImp(RegExpPrototype* regexpProto, RegExp* exp)
+RegExpImp::RegExpImp(RegExpPrototype* regexpProto, PassRefPtr<RegExp> regExp)
   : JSObject(regexpProto)
-  , m_regExp(exp)
+  , m_regExp(regExp)
+  , m_lastIndex(0)
 {
 }
 
@@ -163,6 +147,42 @@ RegExpImp::~RegExpImp()
 {
 }
 
+bool RegExpImp::getOwnPropertySlot(ExecState* exec, const Identifier& propertyName, PropertySlot& slot)
+{
+  return getStaticValueSlot<RegExpImp, JSObject>(exec, &RegExpImpTable, this, propertyName, slot);
+}
+
+JSValue* RegExpImp::getValueProperty(ExecState*, int token) const
+{
+    switch (token) {
+        case Global:
+            return jsBoolean(m_regExp->global());
+        case IgnoreCase:
+            return jsBoolean(m_regExp->ignoreCase());
+        case Multiline:
+            return jsBoolean(m_regExp->multiline());
+        case Source:
+            return jsString(m_regExp->pattern());
+        case LastIndex:
+            return jsNumber(m_lastIndex);
+    }
+    
+    ASSERT_NOT_REACHED();
+    return 0;
+}
+
+void RegExpImp::put(ExecState* exec, const Identifier& propertyName, JSValue* value, int attributes)
+{
+    lookupPut<RegExpImp, JSObject>(exec, propertyName, value, attributes, &RegExpImpTable, this);
+}
+
+void RegExpImp::putValueProperty(ExecState* exec, int token, JSValue* value, int)
+{
+    UNUSED_PARAM(token);
+    ASSERT(token == LastIndex);
+    m_lastIndex = value->toInteger(exec);
+}
+
 bool RegExpImp::match(ExecState* exec, const List& args)
 {
     RegExpObjectImp* regExpObj = exec->lexicalInterpreter()->builtinRegExp();
@@ -225,10 +245,10 @@ JSValue* RegExpImp::callAsFunction(ExecState* exec, JSObject*, const List& args)
 
 // ------------------------------ RegExpObjectImp ------------------------------
 
-const ClassInfo RegExpObjectImp::info = { "Function", &InternalFunctionImp::info, &RegExpTable };
+const ClassInfo RegExpObjectImp::info = { "Function", &InternalFunctionImp::info, &RegExpObjectImpTable };
 
 /* Source for regexp_object.lut.h
-@begin RegExpTable 20
+@begin RegExpObjectImpTable 21
   input           RegExpObjectImp::Input          None
   $_              RegExpObjectImp::Input          DontEnum
   multiline       RegExpObjectImp::Multiline      None
@@ -293,7 +313,7 @@ void RegExpObjectImp::performMatch(RegExp* r, const UString& s, int startOffset,
 
     d->lastInput = s;
     d->lastOvector.set(tmpOvector.release());
-    d->lastNumSubPatterns = r->subPatterns();
+    d->lastNumSubPatterns = r->numSubpatterns();
   }
 }
 
@@ -346,7 +366,7 @@ JSValue *RegExpObjectImp::getRightContext() const
 
 bool RegExpObjectImp::getOwnPropertySlot(ExecState *exec, const Identifier& propertyName, PropertySlot& slot)
 {
-  return getStaticValueSlot<RegExpObjectImp, InternalFunctionImp>(exec, &RegExpTable, this, propertyName, slot);
+  return getStaticValueSlot<RegExpObjectImp, InternalFunctionImp>(exec, &RegExpObjectImpTable, this, propertyName, slot);
 }
 
 JSValue *RegExpObjectImp::getValueProperty(ExecState*, int token) const
@@ -391,7 +411,7 @@ JSValue *RegExpObjectImp::getValueProperty(ExecState*, int token) const
 
 void RegExpObjectImp::put(ExecState *exec, const Identifier &propertyName, JSValue *value, int attr)
 {
-  lookupPut<RegExpObjectImp, InternalFunctionImp>(exec, propertyName, value, attr, &RegExpTable, this);
+  lookupPut<RegExpObjectImp, InternalFunctionImp>(exec, propertyName, value, attr, &RegExpObjectImpTable, this);
 }
 
 void RegExpObjectImp::putValueProperty(ExecState *exec, int token, JSValue *value, int)
@@ -419,44 +439,19 @@ JSObject *RegExpObjectImp::construct(ExecState *exec, const List &args)
   JSValue* arg0 = args[0];
   JSValue* arg1 = args[1];
   
-  JSObject* o = arg0->getObject();
-  if (o && o->inherits(&RegExpImp::info)) {
+  if (arg0->isObject(&RegExpImp::info)) {
     if (!arg1->isUndefined())
-      return throwError(exec, TypeError);
-    return o;
+      return throwError(exec, TypeError, "Cannot supply flags when constructing one RegExp from another.");
+    return static_cast<JSObject*>(arg0);
   }
   
-  UString p = arg0->isUndefined() ? UString("") : arg0->toString(exec);
+  UString pattern = arg0->isUndefined() ? UString("") : arg0->toString(exec);
   UString flags = arg1->isUndefined() ? UString("") : arg1->toString(exec);
+  RefPtr<RegExp> regExp = new RegExp(pattern, flags);
 
-  RegExpPrototype* proto = static_cast<RegExpPrototype*>(exec->lexicalInterpreter()->builtinRegExpPrototype());
-
-  bool global = (flags.find('g') >= 0);
-  bool ignoreCase = (flags.find('i') >= 0);
-  bool multiline = (flags.find('m') >= 0);
-
-  int reflags = RegExp::None;
-  if (global)
-      reflags |= RegExp::Global;
-  if (ignoreCase)
-      reflags |= RegExp::IgnoreCase;
-  if (multiline)
-      reflags |= RegExp::Multiline;
-
-  OwnPtr<RegExp> re(new RegExp(p, reflags));
-  if (!re->isValid())
-      return throwError(exec, SyntaxError, UString("Invalid regular expression: ").append(re->errorMessage()));
-
-  RegExpImp* dat = new RegExpImp(proto, re.release());
-
-  dat->putDirect(exec->propertyNames().global, jsBoolean(global), DontDelete | ReadOnly | DontEnum);
-  dat->putDirect(exec->propertyNames().ignoreCase, jsBoolean(ignoreCase), DontDelete | ReadOnly | DontEnum);
-  dat->putDirect(exec->propertyNames().multiline, jsBoolean(multiline), DontDelete | ReadOnly | DontEnum);
-
-  dat->putDirect(exec->propertyNames().source, jsString(p), DontDelete | ReadOnly | DontEnum);
-  dat->putDirect(exec->propertyNames().lastIndex, jsNumber(0), DontDelete | DontEnum);
-
-  return dat;
+  return regExp->isValid()
+    ? new RegExpImp(static_cast<RegExpPrototype*>(exec->lexicalInterpreter()->builtinRegExpPrototype()), regExp.release())
+    : throwError(exec, SyntaxError, UString("Invalid regular expression: ").append(regExp->errorMessage()));
 }
 
 // ECMA 15.10.3
index 27aaefb74bcbc5f4e13f5e21b9e2054e16e0e0a8..38a9954ca43b233032615e085961c682499f05ad 100644 (file)
@@ -48,10 +48,12 @@ namespace KJS {
 
     class RegExpImp : public JSObject {
     public:
-        RegExpImp(RegExpPrototype*, RegExp*);
+        enum { Global, IgnoreCase, Multiline, Source, LastIndex };
+
+        RegExpImp(RegExpPrototype*, PassRefPtr<RegExp>);
         virtual ~RegExpImp();
 
-        void setRegExp(RegExp* r) { m_regExp.set(r); }
+        void setRegExp(PassRefPtr<RegExp> r) { m_regExp = r; }
         RegExp* regExp() const { return m_regExp.get(); }
 
         JSValue* test(ExecState*, const List& args);
@@ -59,13 +61,19 @@ namespace KJS {
 
         virtual bool implementsCall() const;
         virtual JSValue* callAsFunction(ExecState*, JSObject*, const List&);
+        bool getOwnPropertySlot(ExecState*, const Identifier&, PropertySlot&);
+        JSValue* getValueProperty(ExecState*, int token) const;
+        void put(ExecState*, const Identifier&, JSValue*, int attributes = None);
+        void putValueProperty(ExecState*, int token, JSValue*, int attributes);
+
         virtual const ClassInfo* classInfo() const { return &info; }
         static const ClassInfo info;
 
     private:
         bool match(ExecState*, const List& args);
 
-        OwnPtr<RegExp> m_regExp;
+        RefPtr<RegExp> m_regExp;
+        double m_lastIndex;
     };
 
     class RegExpObjectImp : public InternalFunctionImp {
index 39dd76161b67ac509bb1de2854c807b8f62a99d7..e59694e2b2163e7e49a0bd030f66fc86a4bca13e 100644 (file)
@@ -276,13 +276,13 @@ static inline UString substituteBackreferences(const UString &replacement, const
     } else if (ref >= '0' && ref <= '9') {
         // 1- and 2-digit back references are allowed
         unsigned backrefIndex = ref - '0';
-        if (backrefIndex > (unsigned)reg->subPatterns())
+        if (backrefIndex > reg->numSubpatterns())
             continue;
         if (substitutedReplacement.size() > i + 2) {
             ref = substitutedReplacement[i+2].unicode();
             if (ref >= '0' && ref <= '9') {
                 backrefIndex = 10 * backrefIndex + ref - '0';
-                if (backrefIndex > (unsigned)reg->subPatterns())
+                if (backrefIndex > reg->numSubpatterns())
                     backrefIndex = backrefIndex / 10;   // Fall back to the 1-digit reference
                 else
                     advance = 1;
@@ -334,7 +334,7 @@ static JSValue *replace(ExecState *exec, StringImp* sourceVal, JSValue *pattern,
 
   if (pattern->isObject() && static_cast<JSObject *>(pattern)->inherits(&RegExpImp::info)) {
     RegExp *reg = static_cast<RegExpImp *>(pattern)->regExp();
-    bool global = reg->flags() & RegExp::Global;
+    bool global = reg->global();
 
     RegExpObjectImp* regExpObj = static_cast<RegExpObjectImp*>(exec->lexicalInterpreter()->builtinRegExp());
 
@@ -364,7 +364,7 @@ static JSValue *replace(ExecState *exec, StringImp* sourceVal, JSValue *pattern,
           int completeMatchStart = ovector[0];
           List args;
 
-          for (unsigned i = 0; i < reg->subPatterns() + 1; i++) {
+          for (unsigned i = 0; i < reg->numSubpatterns() + 1; i++) {
               int matchStart = ovector[i * 2];
               int matchLen = ovector[i * 2 + 1] - matchStart;
 
@@ -519,7 +519,7 @@ JSValue* StringProtoFunc::callAsFunction(ExecState* exec, JSObject* thisObj, con
        *  If regexp is not an object whose [[Class]] property is "RegExp", it is
        *  replaced with the result of the expression new RegExp(regexp).
        */
-      reg = tmpReg = new RegExp(a0->toString(exec), RegExp::None);
+      reg = tmpReg = new RegExp(a0->toString(exec));
     }
     RegExpObjectImp* regExpObj = static_cast<RegExpObjectImp*>(exec->lexicalInterpreter()->builtinRegExp());
     int pos;
@@ -529,7 +529,7 @@ JSValue* StringProtoFunc::callAsFunction(ExecState* exec, JSObject* thisObj, con
       result = jsNumber(pos);
     } else {
       // Match
-      if ((reg->flags() & RegExp::Global) == 0) {
+      if (!(reg->global())) {
         // case without 'g' flag is handled like RegExp.prototype.exec
         if (pos < 0)
           result = jsNull();
@@ -546,7 +546,7 @@ JSValue* StringProtoFunc::callAsFunction(ExecState* exec, JSObject* thisObj, con
           regExpObj->performMatch(reg, u, pos, pos, matchLength);
         }
         if (imp)
-          imp->put(exec, "lastIndex", jsNumber(lastIndex), DontDelete|DontEnum);
+          imp->put(exec, exec->propertyNames().lastIndex, jsNumber(lastIndex), DontDelete|DontEnum);
         if (list.isEmpty()) {
           // if there are no matches at all, it's important to return
           // Null instead of an empty array, because this matches
@@ -613,7 +613,7 @@ JSValue* StringProtoFunc::callAsFunction(ExecState* exec, JSObject* thisObj, con
           p0 = mpos + mlen;
           i++;
         }
-        for (unsigned si = 1; si <= reg->subPatterns(); ++si) {
+        for (unsigned si = 1; si <= reg->numSubpatterns(); ++si) {
           int spos = ovector[si * 2];
           if (spos < 0)
             res->put(exec, i++, jsUndefined());
index 0250ba8e5190b7001d40b0226ec92ed8f8a0a38c..20bcdcc1607bc36bdf4a9ff46b852dc12822f933 100644 (file)
@@ -1,3 +1,12 @@
+2007-11-06  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Darin Adler.
+        
+        Beefed up the RegExp.compile testcase to cover a mistake in the 
+        original check-in and a mistake I made while developing my new patch.
+
+        * fast/js/resources/regexp-compile.js:
+
 2007-11-07  Lars Knoll  <lars@trolltech.com>
 
         Reviewed by Simon.
index 4b85c3efebd385d8372aece2a2849482ae63f1e4..854d50a8d2529df36b48ff9cbc6151dad136d68a 100644 (file)
@@ -14,12 +14,15 @@ PASS re.toString() is '/c/'
 PASS re.ignoreCase is true
 PASS re.test('C') is true
 PASS re.toString() is '/c/i'
-PASS re.compile(new RegExp('c'), 'i'); threw exception TypeError: cannot supply flags when constructing one RegExp from another..
+PASS re.compile(new RegExp('c'), 'i'); threw exception TypeError: Cannot supply flags when constructing one RegExp from another..
+PASS re.toString() is '/c/i'
 PASS re.compile(new RegExp('+')); threw exception SyntaxError: Invalid regular expression: nothing to repeat.
 PASS re.toString() is '/undefined/'
 PASS re.toString() is '/null/'
 PASS re.toString() is '//'
 PASS re.toString() is '/z/'
+PASS re.lastIndex is 0
+PASS re.lastIndex is 1
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 6fc2783b54246f33a9e49d25cfd9fbea2eebb4c2..eae57872278014bf132265e3defe97b97dd46240 100644 (file)
@@ -25,6 +25,10 @@ shouldBe("re.toString()", "'/c/i'");
 
 shouldThrow("re.compile(new RegExp('c'), 'i');");
 
+// It's OK to supply a second argument, as long as the argument is "undefined".
+re.compile(re, undefined);
+shouldBe("re.toString()", "'/c/i'");
+
 shouldThrow("re.compile(new RegExp('+'));");
 
 re.compile(undefined);
@@ -39,4 +43,11 @@ shouldBe("re.toString()", "'//'"); // /(?:)/ in Firefox
 re.compile("z", undefined);
 shouldBe("re.toString()", "'/z/'");
 
+// Compiling should reset lastIndex.
+re.lastIndex = 100;
+re.compile(/a/g);
+shouldBe("re.lastIndex", "0");
+re.exec("aaa");
+shouldBe("re.lastIndex", "1");
+
 var successfullyParsed = true;