Bug 1534593 part 2. Factor out the "determine the default initializer" code from dictionary member init and reuse it for sequence member init. r=qdot
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 16 Apr 2019 19:18:09 +0000
changeset 469841 988efe7f290a677c37ecdd9cf5450632d91e7759
parent 469840 606bba3b09248942c5cdbfb44e119d40f9dd054c
child 469842 7e2bcc5233ff4910b859b06b8f8d4fd61c34de57
push id35883
push userbtara@mozilla.com
push dateWed, 17 Apr 2019 21:47:29 +0000
treeherdermozilla-central@02b89c29412b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersqdot
bugs1534593
milestone68.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 1534593 part 2. Factor out the "determine the default initializer" code from dictionary member init and reuse it for sequence member init. r=qdot The other option would be to implement nsTArrayElementTraits for JSObject* and null-initialize there. Differential Revision: https://phabricator.services.mozilla.com/D27561
dom/bindings/Codegen.py
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -4570,16 +4570,32 @@ def recordKeyType(recordType):
         return "nsCString"
     return "nsString"
 
 
 def recordKeyDeclType(recordType):
     return CGGeneric(recordKeyType(recordType))
 
 
+def initializerForType(type):
+    """
+    Get the right initializer for the given type for a data location where we
+    plan to then initialize it from a JS::Value.  Some types need to always be
+    initialized even before we start the JS::Value-to-IDL-value conversion.
+
+    Returns a string or None if no initialization is needed.
+    """
+    if type.isObject():
+        return "nullptr"
+    # We could probably return CGDictionary.getNonInitializingCtorArg() for the
+    # dictionary case, but code outside DictionaryBase subclasses can't use
+    # that, so we can't do it across the board.
+    return None
+
+
 # If this function is modified, modify CGNativeMember.getArg and
 # CGNativeMember.getRetvalInfo accordingly.  The latter cares about the decltype
 # and holdertype we end up using, because it needs to be able to return the code
 # that will convert those to the actual return value of the callback function.
 def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
                                 isDefinitelyObject=False,
                                 isMember=False,
                                 isOptional=False,
@@ -4877,16 +4893,22 @@ def getJSToNativeConversionInfo(type, de
             "declName": "slot" + str(nestingLevel),
             # We only need holderName here to handle isExternal()
             # interfaces, which use an internal holder for the
             # conversion even when forceOwningType ends up true.
             "holderName": "tempHolder" + str(nestingLevel),
             "passedToJSImpl": "${passedToJSImpl}"
         })
 
+        elementInitializer = initializerForType(elementType)
+        if elementInitializer is None:
+            elementInitializer = ""
+        else:
+            elementInitializer = elementInitializer + ", "
+
         # NOTE: Keep this in sync with variadic conversions as needed
         templateBody = fill(
             """
             JS::ForOfIterator iter${nestingLevel}(cx);
             if (!iter${nestingLevel}.init($${val}, JS::ForOfIterator::AllowNonIterable)) {
               $*{exceptionCode}
             }
             if (!iter${nestingLevel}.valueIsIterable()) {
@@ -4897,31 +4919,32 @@ def getJSToNativeConversionInfo(type, de
             while (true) {
               bool done${nestingLevel};
               if (!iter${nestingLevel}.next(&temp${nestingLevel}, &done${nestingLevel})) {
                 $*{exceptionCode}
               }
               if (done${nestingLevel}) {
                 break;
               }
-              ${elementType}* slotPtr${nestingLevel} = arr${nestingLevel}.AppendElement(mozilla::fallible);
+              ${elementType}* slotPtr${nestingLevel} = arr${nestingLevel}.AppendElement(${elementInitializer}mozilla::fallible);
               if (!slotPtr${nestingLevel}) {
                 JS_ReportOutOfMemory(cx);
                 $*{exceptionCode}
               }
               ${elementType}& slot${nestingLevel} = *slotPtr${nestingLevel};
               $*{elementConversion}
             }
             """,
             exceptionCode=exceptionCode,
             notSequence=notSequence,
             sequenceType=sequenceType,
             arrayRef=arrayRef,
             elementType=elementInfo.declType.define(),
             elementConversion=elementConversion,
+            elementInitializer=elementInitializer,
             nestingLevel=str(nestingLevel))
 
         templateBody = wrapObjectTemplate(templateBody, type,
                                           "${declName}.SetNull();\n", notSequence)
         if isinstance(defaultValue, IDLEmptySequenceValue):
             if type.nullable():
                 codeToSetEmpty = "${declName}.SetValue();\n"
             else:
@@ -6488,28 +6511,31 @@ class CGArgumentConverter(CGThing):
                                               typeConversion.declType).define()
         if typeNeedsRooting(self.argument.type):
             rooterDecl = ("SequenceRooter<%s> ${holderName}(cx, &${declName});\n" %
                           typeConversion.declType.define())
         else:
             rooterDecl = ""
         replacer["elemType"] = typeConversion.declType.define()
 
+        replacer["elementInitializer"] = initializerForType(self.argument.type) or ""
+
         # NOTE: Keep this in sync with sequence conversions as needed
         variadicConversion = string.Template(
             "${seqType} ${declName};\n" +
             rooterDecl +
             dedent("""
                 if (${argc} > ${index}) {
                   if (!${declName}.SetCapacity(${argc} - ${index}, mozilla::fallible)) {
                     JS_ReportOutOfMemory(cx);
                     return false;
                   }
                   for (uint32_t variadicArg = ${index}; variadicArg < ${argc}; ++variadicArg) {
-                    ${elemType}& slot = *${declName}.AppendElement(mozilla::fallible);
+                    // OK to do infallible append here, since we ensured capacity already.
+                    ${elemType}& slot = *${declName}.AppendElement(${elementInitializer});
                 """)
         ).substitute(replacer)
 
         val = string.Template("args[variadicArg]").substitute(replacer)
         variadicConversion += indent(
             string.Template(typeConversion.template).substitute({
                 "val": val,
                 "maybeMutableVal": val,
@@ -14025,35 +14051,34 @@ class CGDictionary(CGThing):
         if member.canHaveMissingValue():
             trace = CGIfWrapper(trace, "%s.WasPassed()" % memberLoc)
 
         return trace.define()
 
     def getMemberInitializer(self, memberInfo):
         """
         Get the right initializer for the member.  Most members don't need one,
-        but we need to pre-initialize 'any' and 'object' that have a default
-        value, so they're safe to trace at all times.
+        but we need to pre-initialize 'object' that have a default value or are
+        required (and hence are not inside Optional), so they're safe to trace
+        at all times.  And we can optimize a bit for dictionary-typed members.
         """
         member, _ = memberInfo
         if member.canHaveMissingValue():
             # Allowed missing value means no need to set it up front, since it's
             # inside an Optional and won't get traced until it's actually set
             # up.
             return None
         type = member.type
-        if type.isObject():
-            return "nullptr"
         if type.isDictionary():
             # When we construct ourselves, we don't want to init our member
             # dictionaries.  Either we're being constructed-but-not-initialized
             # ourselves (and then we don't want to init them) or we're about to
             # init ourselves and then we'll init them anyway.
             return CGDictionary.getNonInitializingCtorArg()
-        return None
+        return initializerForType(type)
 
     def getMemberSourceDescription(self, member):
         return ("'%s' member of %s" %
                 (member.identifier.name, self.dictionary.identifier.name))
 
     @staticmethod
     def makeIdName(name):
         return IDLToCIdentifier(name) + "_id"