Bug 1687428 - Part 29: Return TaggedParserAtomIndex from ParserAtomsTable::concatAtoms. r=nbp
authorTooru Fujisawa <arai_a@mac.com>
Fri, 29 Jan 2021 19:43:05 +0000
changeset 565244 21b2349db370939b1edc3c321dfccd220774c3c6
parent 565243 2fa38f16f81a998968667e876b2b3282c3e4741e
child 565245 5d9fb33286089d22e274c81bf31f8a69caafc760
push id38154
push usernbeleuzu@mozilla.com
push dateSat, 30 Jan 2021 05:43:48 +0000
treeherdermozilla-central@059539aeb485 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp
bugs1687428
milestone87.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1687428 - Part 29: Return TaggedParserAtomIndex from ParserAtomsTable::concatAtoms. r=nbp ParserAtomsTable::concatAtoms still receives ParserAtom* range, because we need character access, and InflatedChar16Sequence needs ParserAtom* range. Differential Revision: https://phabricator.services.mozilla.com/D102560
js/src/frontend/FoldConstants.cpp
js/src/frontend/Parser.cpp
js/src/frontend/ParserAtom.cpp
js/src/frontend/ParserAtom.h
js/src/jsapi-tests/testParserAtom.cpp
--- a/js/src/frontend/FoldConstants.cpp
+++ b/js/src/frontend/FoldConstants.cpp
@@ -1245,25 +1245,25 @@ static bool FoldAdd(FoldInfo info, Parse
         next = &(*current)->pn_next;
 
         node->unsafeDecrementCount();
       } while (*next);
 
       // Replace with concatenation if we multiple nodes.
       if (accum.length() > 1) {
         // Construct the concatenated atom.
-        const ParserAtom* combination = info.parserAtoms.concatAtoms(
+        auto combination = info.parserAtoms.concatAtoms(
             info.cx, mozilla::Range(accum.begin(), accum.length()));
         if (!combination) {
           return false;
         }
 
         // Replace |current|'s string with the entire combination.
         MOZ_ASSERT((*current)->isKind(ParseNodeKind::StringExpr));
-        (*current)->as<NameNode>().setAtom(combination->toIndex());
+        (*current)->as<NameNode>().setAtom(combination);
       }
 
       // If we're out of nodes, we're done.
       if (!*next) {
         break;
       }
 
       current = next;
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -2441,22 +2441,17 @@ TaggedParserAtomIndex ParserBase::prefix
   } else {
     MOZ_ASSERT(propType == PropertyType::Getter);
     prefix = cx_->parserNames().getPrefix;
   }
 
   const ParserAtom* atoms[2] = {
       prefix, this->compilationState_.parserAtoms.getParserAtom(propAtom)};
   auto atomsRange = mozilla::Range(atoms, 2);
-  auto result =
-      this->compilationState_.parserAtoms.concatAtoms(cx_, atomsRange);
-  if (!result) {
-    return TaggedParserAtomIndex::null();
-  }
-  return result->toIndex();
+  return this->compilationState_.parserAtoms.concatAtoms(cx_, atomsRange);
 }
 
 template <class ParseHandler, typename Unit>
 void GeneralParser<ParseHandler, Unit>::setFunctionStartAtPosition(
     FunctionBox* funbox, TokenPos pos) const {
   uint32_t startLine, startColumn;
   tokenStream.computeLineAndColumn(pos.begin, &startLine, &startColumn);
 
--- a/js/src/frontend/ParserAtom.cpp
+++ b/js/src/frontend/ParserAtom.cpp
@@ -547,64 +547,72 @@ const ParserAtom* ParserAtomsTable::inte
   }
 
   // We should (infallibly) map back to the same JSAtom.
   MOZ_ASSERT(parserAtom->toJSAtom(cx, stencil.input.atomCache) == atom);
 
   return parserAtom;
 }
 
-const ParserAtom* ParserAtomsTable::concatAtoms(
+TaggedParserAtomIndex ParserAtomsTable::concatAtoms(
     JSContext* cx, mozilla::Range<const ParserAtom*> atoms) {
   MOZ_ASSERT(atoms.length() >= 2,
              "concatAtoms should only be used for multiple inputs");
 
   // Compute final length and encoding.
   bool catLatin1 = true;
   uint32_t catLen = 0;
   for (const ParserAtom* atom : atoms) {
     if (atom->hasTwoByteChars()) {
       catLatin1 = false;
     }
     // Overflow check here, length
     if (atom->length() >= (ParserAtomEntry::MAX_LENGTH - catLen)) {
       js::ReportOutOfMemory(cx);
-      return nullptr;
+      return TaggedParserAtomIndex::null();
     }
     catLen += atom->length();
   }
 
   // Short Latin1 strings must check for both Tiny and WellKnown atoms so simple
   // concatenate onto stack and use `internLatin1`.
   if (catLatin1 && (catLen <= WellKnownParserAtoms::MaxWellKnownLength)) {
     Latin1Char buf[WellKnownParserAtoms::MaxWellKnownLength];
     size_t offset = 0;
     for (const ParserAtom* atom : atoms) {
       mozilla::PodCopy(buf + offset, atom->latin1Chars(), atom->length());
       offset += atom->length();
     }
-    return internLatin1(cx, buf, catLen);
+    const auto* result = internLatin1(cx, buf, catLen);
+    if (!result) {
+      return TaggedParserAtomIndex::null();
+    }
+    return result->toIndex();
   }
 
   // NOTE: We have ruled out Tiny and WellKnown atoms and can ignore below.
 
   InflatedChar16Sequence<const ParserAtom*> seq(atoms);
   SpecificParserAtomLookup<const ParserAtom*> lookup(seq);
 
   // Check for existing atom.
   auto addPtr = entryMap_.lookupForAdd(lookup);
   if (addPtr) {
-    return addPtr->key()->asAtom();
+    return addPtr->value();
   }
 
   // Otherwise, add new entry.
-  return catLatin1 ? internChar16Seq<Latin1Char>(cx, addPtr, lookup.hash(), seq,
-                                                 catLen)
-                   : internChar16Seq<char16_t>(cx, addPtr, lookup.hash(), seq,
-                                               catLen);
+  const auto* result =
+      catLatin1
+          ? internChar16Seq<Latin1Char>(cx, addPtr, lookup.hash(), seq, catLen)
+          : internChar16Seq<char16_t>(cx, addPtr, lookup.hash(), seq, catLen);
+  if (!result) {
+    return TaggedParserAtomIndex::null();
+  }
+  return result->toIndex();
 }
 
 const ParserAtom* WellKnownParserAtoms::getWellKnown(
     WellKnownAtomId atomId) const {
 #define ASSERT_OFFSET_(idpart, id, text)              \
   static_assert(offsetof(WellKnownParserAtoms, id) == \
                 int32_t(WellKnownAtomId::id) * sizeof(ParserAtom*));
   FOR_EACH_COMMON_PROPERTYNAME(ASSERT_OFFSET_);
--- a/js/src/frontend/ParserAtom.h
+++ b/js/src/frontend/ParserAtom.h
@@ -784,18 +784,18 @@ class ParserAtomsTable {
                                uint32_t nbyte);
 
   const ParserAtom* internChar16(JSContext* cx, const char16_t* char16Ptr,
                                  uint32_t length);
 
   const ParserAtom* internJSAtom(JSContext* cx, CompilationStencil& stencil,
                                  JSAtom* atom);
 
-  const ParserAtom* concatAtoms(JSContext* cx,
-                                mozilla::Range<const ParserAtom*> atoms);
+  TaggedParserAtomIndex concatAtoms(JSContext* cx,
+                                    mozilla::Range<const ParserAtom*> atoms);
 
   const ParserAtom* getWellKnown(WellKnownAtomId atomId) const;
   const ParserAtom* getStatic1(StaticParserString1 s) const;
   const ParserAtom* getStatic2(StaticParserString2 s) const;
   const ParserAtom* getParserAtom(ParserAtomIndex index) const;
   const ParserAtom* getParserAtom(TaggedParserAtomIndex index) const;
 
   void markUsedByStencil(TaggedParserAtomIndex index) const;
--- a/js/src/jsapi-tests/testParserAtom.cpp
+++ b/js/src/jsapi-tests/testParserAtom.cpp
@@ -28,28 +28,30 @@ BEGIN_TEST(testParserAtom_empty) {
   const char16_t char16[] = {};
 
   const uint8_t bytes[] = {};
   const js::LittleEndianChars leTwoByte(bytes);
 
   // Check that the well-known empty atom matches for different entry points.
   const ParserAtom* ref = cx->parserNames().empty;
   CHECK(ref);
+  auto refIndex = ref->toIndex();
+  CHECK(refIndex);
   CHECK(atomTable.internAscii(cx, ascii, 0) == ref);
   CHECK(atomTable.internLatin1(cx, latin1, 0) == ref);
   CHECK(atomTable.internUtf8(cx, utf8, 0) == ref);
   CHECK(atomTable.internChar16(cx, char16, 0) == ref);
 
   // Check concatenation works on empty atoms.
   const ParserAtom* concat[] = {
       cx->parserNames().empty,
       cx->parserNames().empty,
   };
   mozilla::Range<const ParserAtom*> concatRange(concat, 2);
-  CHECK(atomTable.concatAtoms(cx, concatRange) == ref);
+  CHECK(atomTable.concatAtoms(cx, concatRange) == refIndex);
 
   return true;
 }
 END_TEST(testParserAtom_empty)
 
 // Test length-1 fast-path is consistent across entry points.
 BEGIN_TEST(testParserAtom_tiny1) {
   using js::frontend::ParserAtom;
@@ -65,27 +67,29 @@ BEGIN_TEST(testParserAtom_tiny1) {
   const mozilla::Utf8Unit utf8[] = {mozilla::Utf8Unit('a')};
   char16_t char16[] = {'a'};
 
   const uint8_t bytes[] = {'a', 0};
   const js::LittleEndianChars leTwoByte(bytes);
 
   const ParserAtom* ref = cx->parserNames().lookupTiny(&a, 1);
   CHECK(ref);
+  auto refIndex = ref->toIndex();
+  CHECK(refIndex);
   CHECK(atomTable.internAscii(cx, ascii, 1) == ref);
   CHECK(atomTable.internLatin1(cx, latin1, 1) == ref);
   CHECK(atomTable.internUtf8(cx, utf8, 1) == ref);
   CHECK(atomTable.internChar16(cx, char16, 1) == ref);
 
   const ParserAtom* concat[] = {
       ref,
       cx->parserNames().empty,
   };
   mozilla::Range<const ParserAtom*> concatRange(concat, 2);
-  CHECK(atomTable.concatAtoms(cx, concatRange) == ref);
+  CHECK(atomTable.concatAtoms(cx, concatRange) == refIndex);
 
   // Note: If Latin1-Extended characters become supported, then UTF-8 behaviour
   // should be tested.
   char16_t ae = 0x00E6;
   CHECK(cx->parserNames().lookupTiny(&ae, 1) == nullptr);
 
   return true;
 }
@@ -106,27 +110,29 @@ BEGIN_TEST(testParserAtom_tiny2) {
                                     mozilla::Utf8Unit('0')};
   char16_t char16[] = {'a', '0'};
 
   const uint8_t bytes[] = {'a', 0, '0', 0};
   const js::LittleEndianChars leTwoByte(bytes);
 
   const ParserAtom* ref = cx->parserNames().lookupTiny(ascii, 2);
   CHECK(ref);
+  auto refIndex = ref->toIndex();
+  CHECK(refIndex);
   CHECK(atomTable.internAscii(cx, ascii, 2) == ref);
   CHECK(atomTable.internLatin1(cx, latin1, 2) == ref);
   CHECK(atomTable.internUtf8(cx, utf8, 2) == ref);
   CHECK(atomTable.internChar16(cx, char16, 2) == ref);
 
   const ParserAtom* concat[] = {
       cx->parserNames().lookupTiny(ascii + 0, 1),
       cx->parserNames().lookupTiny(ascii + 1, 1),
   };
   mozilla::Range<const ParserAtom*> concatRange(concat, 2);
-  CHECK(atomTable.concatAtoms(cx, concatRange) == ref);
+  CHECK(atomTable.concatAtoms(cx, concatRange) == refIndex);
 
   // Note: If Latin1-Extended characters become supported, then UTF-8 behaviour
   // should be tested.
   char16_t ae0[] = {0x00E6, '0'};
   CHECK(cx->parserNames().lookupTiny(ae0, 2) == nullptr);
 
   return true;
 }
@@ -147,25 +153,27 @@ BEGIN_TEST(testParserAtom_concat) {
     for (const char16_t* arg : args) {
       size_t len = std::char_traits<char16_t>::length(arg);
       const ParserAtom* atom = atomTable.internChar16(cx, arg, len);
       inputs.push_back(atom);
     }
 
     // Concatenate twice to test new vs existing pathways.
     mozilla::Range<const ParserAtom*> range(inputs.data(), inputs.size());
-    const ParserAtom* once = atomTable.concatAtoms(cx, range);
-    const ParserAtom* twice = atomTable.concatAtoms(cx, range);
+    auto once = atomTable.concatAtoms(cx, range);
+    auto twice = atomTable.concatAtoms(cx, range);
 
     // Intern expected value literal _after_ the concat code to allow
     // allocation pathways a chance to be tested.
     size_t exp_len = std::char_traits<char16_t>::length(exp);
     const ParserAtom* ref = atomTable.internChar16(cx, exp, exp_len);
+    auto refIndex = ref->toIndex();
+    CHECK(refIndex);
 
-    return (once == ref) && (twice == ref);
+    return (once == refIndex) && (twice == refIndex);
   };
 
   // Checks empty strings
   CHECK(CheckConcat(u"", {u"", u""}));
   CHECK(CheckConcat(u"", {u"", u"", u"", u""}));
   CHECK(CheckConcat(u"A", {u"", u"", u"A", u""}));
   CHECK(CheckConcat(u"AAAA", {u"", u"", u"AAAA", u""}));