[JSC] Clean up RegExp flag code

https://bugs.webkit.org/show_bug.cgi?id=227786

Reviewed by Filip Pizlo.

I found a bug in regexpToSourceString (used for RegExp's dump code), and it motivates me to clean up
all RegExp flag handling code. We define RegExp flags as a macro and updating that macro updates all
RegExp flag handling code too.

* runtime/RegExp.cpp:
(JSC::RegExpFunctionalTestCollector::outputOneTest):
(JSC::regexpToSourceString):
* runtime/RegExp.h:
* runtime/RegExpPrototype.cpp:
(JSC::flagsString):
* yarr/YarrFlags.cpp:
(JSC::Yarr::parseFlags):
(JSC::Yarr::flagsString):
* yarr/YarrFlags.h:

Canonical link: https://commits.webkit.org/239501@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@279709 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog

@@ -1,3 +1,25 @@
+2021-07-07  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Clean up RegExp flag code
+        https://bugs.webkit.org/show_bug.cgi?id=227786
+
+        Reviewed by Filip Pizlo.
+
+        I found a bug in regexpToSourceString (used for RegExp's dump code), and it motivates me to clean up
+        all RegExp flag handling code. We define RegExp flags as a macro and updating that macro updates all
+        RegExp flag handling code too.
+
+        * runtime/RegExp.cpp:
+        (JSC::RegExpFunctionalTestCollector::outputOneTest):
+        (JSC::regexpToSourceString):
+        * runtime/RegExp.h:
+        * runtime/RegExpPrototype.cpp:
+        (JSC::flagsString):
+        * yarr/YarrFlags.cpp:
+        (JSC::Yarr::parseFlags):
+        (JSC::Yarr::flagsString):
+        * yarr/YarrFlags.h:
+
 2021-07-07  Saam Barati  <sbarati@apple.com>
 
         JSArrayBufferView::byteOffsetConcurrently has a race when using PAC

Source/JavaScriptCore/runtime/RegExp.cpp

@@ -51,21 +51,7 @@ void RegExpFunctionalTestCollector::outputOneTest(RegExp* regExp, const String&
         fputc('/', m_file);
         outputEscapedString(regExp->pattern(), true);
         fputc('/', m_file);
-        if (regExp->global())
-            fputc('g', m_file);
-        if (regExp->ignoreCase())
-            fputc('i', m_file);
-        if (regExp->hasIndices())
-            fputc('d', m_file);
-        if (regExp->multiline())
-            fputc('m', m_file);
-        if (regExp->dotAll())
-            fputc('s', m_file);
-        if (regExp->unicode())
-            fputc('u', m_file);
-        if (regExp->sticky())
-            fputc('y', m_file);
-        fprintf(m_file, "\n");
+        fprintf(m_file, "%s\n", Yarr::flagsString(regExp->flags()).data());
     }
 
     fprintf(m_file, " \"");
@@ -484,24 +470,7 @@ void RegExp::matchCompareWithInterpreter(const String& s, int startOffset, int*
 
 static CString regexpToSourceString(const RegExp* regExp)
 {
-    char postfix[7] = { '/', 0, 0, 0, 0, 0, 0 };
-    int index = 1;
-    if (regExp->global())
-        postfix[index++] = 'g';
-    if (regExp->ignoreCase())
-        postfix[index++] = 'i';
-    if (regExp->hasIndices())
-        postfix[index] = 'd';
-    if (regExp->multiline())
-        postfix[index] = 'm';
-    if (regExp->dotAll())
-        postfix[index++] = 's';
-    if (regExp->unicode())
-        postfix[index++] = 'u';
-    if (regExp->sticky())
-        postfix[index++] = 'y';
-
-    return toCString("/", regExp->pattern().impl(), postfix);
+    return toCString("/", regExp->pattern().impl(), "/", Yarr::flagsString(regExp->flags()).data());
 }
 
 void RegExp::dumpToStream(const JSCell* cell, PrintStream& out)

Source/JavaScriptCore/runtime/RegExp.h

@@ -57,14 +57,11 @@ class RegExp final : public JSCell {
     static size_t estimatedSize(JSCell*, VM&);
     JS_EXPORT_PRIVATE static void dumpToStream(const JSCell*, PrintStream&);
 
-    bool global() const { return m_flags.contains(Yarr::Flags::Global); }
-    bool ignoreCase() const { return m_flags.contains(Yarr::Flags::IgnoreCase); }
-    bool multiline() const { return m_flags.contains(Yarr::Flags::Multiline); }
-    bool sticky() const { return m_flags.contains(Yarr::Flags::Sticky); }
+    OptionSet<Yarr::Flags> flags() const { return m_flags; }
+#define JSC_DEFINE_REGEXP_FLAG_ACCESSOR(key, name, lowerCaseName, index) bool lowerCaseName() const { return m_flags.contains(Yarr::Flags::name); }
+    JSC_REGEXP_FLAGS(JSC_DEFINE_REGEXP_FLAG_ACCESSOR)
+#undef JSC_DEFINE_REGEXP_FLAG_ACCESSOR
     bool globalOrSticky() const { return global() || sticky(); }
-    bool unicode() const { return m_flags.contains(Yarr::Flags::Unicode); }
-    bool dotAll() const { return m_flags.contains(Yarr::Flags::DotAll); }
-    bool hasIndices() const { return m_flags.contains(Yarr::Flags::HasIndices); }
 
     const String& pattern() const { return m_patternString; }
 

Source/JavaScriptCore/runtime/RegExpPrototype.cpp

@@ -162,49 +162,24 @@ JSC_DEFINE_HOST_FUNCTION(regExpProtoFuncCompile, (JSGlobalObject* globalObject,
     return JSValue::encode(thisRegExp);
 }
 
-typedef std::array<char, 7 + 1> FlagsString; // 6 different flags and a null character terminator.
-
-static inline FlagsString flagsString(JSGlobalObject* globalObject, JSObject* regexp)
+static inline Yarr::FlagsString flagsString(JSGlobalObject* globalObject, JSObject* regexp)
 {
-    FlagsString string;
-    string[0] = 0;
-
     VM& vm = globalObject->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    JSValue indicesValue = regexp->get(globalObject, vm.propertyNames->hasIndices);
-    RETURN_IF_EXCEPTION(scope, string);
-    JSValue globalValue = regexp->get(globalObject, vm.propertyNames->global);
-    RETURN_IF_EXCEPTION(scope, string);
-    JSValue ignoreCaseValue = regexp->get(globalObject, vm.propertyNames->ignoreCase);
-    RETURN_IF_EXCEPTION(scope, string);
-    JSValue multilineValue = regexp->get(globalObject, vm.propertyNames->multiline);
-    RETURN_IF_EXCEPTION(scope, string);
-    JSValue dotAllValue = regexp->get(globalObject, vm.propertyNames->dotAll);
-    RETURN_IF_EXCEPTION(scope, string);
-    JSValue unicodeValue = regexp->get(globalObject, vm.propertyNames->unicode);
-    RETURN_IF_EXCEPTION(scope, string);
-    JSValue stickyValue = regexp->get(globalObject, vm.propertyNames->sticky);
-    RETURN_IF_EXCEPTION(scope, string);
-
-    unsigned index = 0;
-    if (indicesValue.toBoolean(globalObject))
-        string[index++] = 'd';
-    if (globalValue.toBoolean(globalObject))
-        string[index++] = 'g';
-    if (ignoreCaseValue.toBoolean(globalObject))
-        string[index++] = 'i';
-    if (multilineValue.toBoolean(globalObject))
-        string[index++] = 'm';
-    if (dotAllValue.toBoolean(globalObject))
-        string[index++] = 's';
-    if (unicodeValue.toBoolean(globalObject))
-        string[index++] = 'u';
-    if (stickyValue.toBoolean(globalObject))
-        string[index++] = 'y';
-    ASSERT(index < string.size());
-    string[index] = 0;
-    return string;
+    OptionSet<Yarr::Flags> flags;
+
+#define JSC_RETRIEVE_REGEXP_FLAG(key, name, lowerCaseName, index) \
+    JSValue lowerCaseName##Value = regexp->get(globalObject, vm.propertyNames->lowerCaseName); \
+    RETURN_IF_EXCEPTION(scope, { }); \
+    if (lowerCaseName##Value.toBoolean(globalObject)) \
+        flags.add(Yarr::Flags::name);
+
+    JSC_REGEXP_FLAGS(JSC_RETRIEVE_REGEXP_FLAG)
+
+#undef JSC_RETRIEVE_REGEXP_FLAG
+
+    return Yarr::flagsString(flags);
 }
 
 JSC_DEFINE_HOST_FUNCTION(regExpProtoFuncToString, (JSGlobalObject* globalObject, CallFrame* callFrame))

Source/JavaScriptCore/yarr/YarrFlags.cpp

@@ -36,47 +36,16 @@ std::optional<OptionSet<Flags>> parseFlags(StringView string)
     OptionSet<Flags> flags;
     for (auto character : string.codeUnits()) {
         switch (character) {
-        case 'd':
-            if (flags.contains(Flags::HasIndices))
-                return std::nullopt;
-            flags.add(Flags::HasIndices);
+#define JSC_HANDLE_REGEXP_FLAG(key, name, lowerCaseName, _) \
+        case key: \
+            if (flags.contains(Flags::name)) \
+                return std::nullopt; \
+            flags.add(Flags::name); \
             break;
 
-        case 'g':
-            if (flags.contains(Flags::Global))
-                return std::nullopt;
-            flags.add(Flags::Global);
-            break;
-
-        case 'i':
-            if (flags.contains(Flags::IgnoreCase))
-                return std::nullopt;
-            flags.add(Flags::IgnoreCase);
-            break;
-
-        case 'm':
-            if (flags.contains(Flags::Multiline))
-                return std::nullopt;
-            flags.add(Flags::Multiline);
-            break;
+        JSC_REGEXP_FLAGS(JSC_HANDLE_REGEXP_FLAG)
 
-        case 's':
-            if (flags.contains(Flags::DotAll))
-                return std::nullopt;
-            flags.add(Flags::DotAll);
-            break;
-
-        case 'u':
-            if (flags.contains(Flags::Unicode))
-                return std::nullopt;
-            flags.add(Flags::Unicode);
-            break;
-
-        case 'y':
-            if (flags.contains(Flags::Sticky))
-                return std::nullopt;
-            flags.add(Flags::Sticky);
-            break;
+#undef JSC_HANDLE_REGEXP_FLAG
 
         default:
             return std::nullopt;
@@ -86,4 +55,24 @@ std::optional<OptionSet<Flags>> parseFlags(StringView string)
     return std::make_optional(flags);
 }
 
+FlagsString flagsString(OptionSet<Flags> flags)
+{
+    FlagsString string;
+    unsigned index = 0;
+
+#define JSC_WRITE_REGEXP_FLAG(key, name, lowerCaseName, _) \
+    do { \
+        if (flags.contains(Flags::name)) \
+            string[index++] = key; \
+    } while (0);
+
+    JSC_REGEXP_FLAGS(JSC_WRITE_REGEXP_FLAG)
+
+#undef JSC_WRITE_REGEXP_FLAG
+
+    ASSERT(index < string.size());
+    string[index] = 0;
+    return string;
+}
+
 } } // namespace JSC::Yarr

Source/JavaScriptCore/yarr/YarrFlags.h

@@ -30,17 +30,29 @@
 
 namespace JSC { namespace Yarr {
 
+// Flags must be ordered in alphabet ordering.
+#define JSC_REGEXP_FLAGS(macro) \
+    macro('d', HasIndices, hasIndices, 0) \
+    macro('g', Global, global, 1) \
+    macro('i', IgnoreCase, ignoreCase, 2) \
+    macro('m', Multiline, multiline, 3) \
+    macro('s', DotAll, dotAll, 4) \
+    macro('u', Unicode, unicode, 5) \
+    macro('y', Sticky, sticky, 6) \
+
+#define JSC_COUNT_REGEXP_FLAG(key, name, lowerCaseName, index) + 1
+static constexpr unsigned numberOfFlags = 0 JSC_REGEXP_FLAGS(JSC_COUNT_REGEXP_FLAG);
+#undef JSC_COUNT_REGEXP_FLAG
+
 enum class Flags : uint8_t {
-    Global = 1 << 0,
-    IgnoreCase = 1 << 1,
-    Multiline = 1 << 2,
-    Sticky = 1 << 3,
-    Unicode = 1 << 4,
-    DotAll = 1 << 5,
-    HasIndices = 1 << 6,
-    DeletedValue = 1 << 7
+#define JSC_DEFINE_REGEXP_FLAG(key, name, lowerCaseName, index) name = 1 << index,
+    JSC_REGEXP_FLAGS(JSC_DEFINE_REGEXP_FLAG)
+#undef JSC_DEFINE_REGEXP_FLAG
+    DeletedValue = 1 << numberOfFlags,
 };
 
 JS_EXPORT_PRIVATE std::optional<OptionSet<Flags>> parseFlags(StringView);
+using FlagsString = std::array<char, Yarr::numberOfFlags + 1>; // numberOfFlags + null-terminator
+FlagsString flagsString(OptionSet<Flags>);
 
 } } // namespace JSC::Yarr