auto merge of #2244 : Ms2ger/servo/optional-rewrite, r=jdm

This moves all handling of `isOptional` (except `type.isCallback()`, which I believe @jdm is rewriting in #2204) into one place.

3 files changed, 44 insertions, 57 deletions

diff --git a/src/components/script/dom/bindings/codegen/CodegenRust.py b/src/components/script/dom/bindings/codegen/CodegenRust.py
index 0912c6fdf96..b0c6879deab 100644
--- a/src/components/script/dom/bindings/codegen/CodegenRust.py
+++ b/src/components/script/dom/bindings/codegen/CodegenRust.py
@@ -92,19 +92,18 @@ class CastableObjectUnwrapper():
 
     codeOnFailure is the code to run if unwrapping fails.
     """
-    def __init__(self, descriptor, source, codeOnFailure, isOptional=False):
+    def __init__(self, descriptor, source, codeOnFailure):
         self.substitution = { "type" : descriptor.nativeType,
                               "depth": descriptor.interface.inheritanceDepth(),
                               "prototype": "PrototypeList::id::" + descriptor.name,
                               "protoID" : "PrototypeList::id::" + descriptor.name + " as uint",
                               "source" : source,
-                              "codeOnFailure" : CGIndenter(CGGeneric(codeOnFailure), 4).define(),
-                              "unwrapped_val" : "Some(val)" if isOptional else "val"}
+                              "codeOnFailure" : CGIndenter(CGGeneric(codeOnFailure), 4).define()}
 
     def __str__(self):
         return string.Template(
 """match unwrap_jsmanaged(${source}, ${prototype}, ${depth}) {
-  Ok(val) => ${unwrapped_val},
+  Ok(val) => val,
   Err(()) => {
     ${codeOnFailure}
   }
@@ -121,10 +120,9 @@ class FailureFatalCastableObjectUnwrapper(CastableObjectUnwrapper):
     """
     As CastableObjectUnwrapper, but defaulting to throwing if unwrapping fails
     """
-    def __init__(self, descriptor, source, isOptional):
+    def __init__(self, descriptor, source):
         CastableObjectUnwrapper.__init__(self, descriptor, source,
-                                         "return 0; //XXXjdm return Throw(cx, rv);",
-                                         isOptional)
+                                         "return 0; //XXXjdm return Throw(cx, rv);")
 
 class CGThing():
     """
@@ -494,6 +492,16 @@ def getJSToNativeConversionTemplate(type, descriptorProvider, failureCode=None,
     if exceptionCode is None:
         exceptionCode = "return 0;"
 
+    def handleOptional(template, declType, isOptional):
+        if isOptional:
+            template = "Some(%s)" % template
+            declType = CGWrapper(declType, pre="Option<", post=">")
+            initialValue = "None"
+        else:
+            initialValue = None
+
+        return (template, declType, isOptional, initialValue)
+
     # Unfortunately, .capitalize() on a string will lowercase things inside the
     # string, which we do not want.
     def firstCap(string):
@@ -587,7 +595,7 @@ def getJSToNativeConversionTemplate(type, descriptorProvider, failureCode=None,
         templateBody = handleDefaultNull(templateBody.define(),
                                          "None")
 
-        return (templateBody, declType, isOptional, "None" if isOptional else None)
+        return handleOptional(templateBody, declType, isOptional)
 
     if type.isGeckoInterface():
         assert not isEnforceRange and not isClamp
@@ -602,34 +610,32 @@ def getJSToNativeConversionTemplate(type, descriptorProvider, failureCode=None,
 
             template = wrapObjectTemplate(conversion, isDefinitelyObject, type,
                                           failureCode)
-            return (template, declType, isOptional, None)
+            return handleOptional(template, declType, isOptional)
 
         templateBody = ""
         if descriptor.interface.isConsequential():
             raise TypeError("Consequential interface %s being used as an "
                             "argument" % descriptor.interface.identifier.name)
 
-
         if failureCode is not None:
             templateBody = str(CastableObjectUnwrapper(
                     descriptor,
                     "(${val}).to_object()",
-                    failureCode,
-                    isOptional or type.nullable()))
+                    failureCode))
         else:
             templateBody = str(FailureFatalCastableObjectUnwrapper(
                     descriptor,
-                    "(${val}).to_object()",
-                    isOptional or type.nullable()))
-
-        templateBody = wrapObjectTemplate(templateBody, isDefinitelyObject,
-                                          type, failureCode)
+                    "(${val}).to_object()"))
 
         declType = CGGeneric(descriptor.nativeType)
-        if type.nullable() or isOptional:
+        if type.nullable():
+            templateBody = "Some(%s)" % templateBody
             declType = CGWrapper(declType, pre="Option<", post=">")
 
-        return (templateBody, declType, isOptional, "None" if isOptional else None)
+        templateBody = wrapObjectTemplate(templateBody, isDefinitelyObject,
+                                          type, failureCode)
+
+        return handleOptional(templateBody, declType, isOptional)
 
     if type.isSpiderMonkeyInterface():
         raise TypeError("Can't handle SpiderMonkey interface arguments yet")
@@ -648,16 +654,12 @@ def getJSToNativeConversionTemplate(type, descriptorProvider, failureCode=None,
         else:
             nullBehavior = treatAs[treatNullAs]
 
-        def getConversionCode(isOptional=False):
-            strval = "strval"
-            if isOptional:
-                strval = "Some(%s)" % strval
-
+        def getConversionCode():
             conversionCode = (
                 "match FromJSValConvertible::from_jsval(cx, ${val}, %s) {\n"
-                "  Ok(strval) => %s,\n"
+                "  Ok(strval) => strval,\n"
                 "  Err(_) => { %s },\n"
-                "}" % (nullBehavior, strval, exceptionCode))
+                "}" % (nullBehavior, exceptionCode))
 
             if defaultValue is None:
                 return conversionCode
@@ -680,15 +682,10 @@ def getJSToNativeConversionTemplate(type, descriptorProvider, failureCode=None,
             return handleDefault(conversionCode, default)
 
         declType = "DOMString"
-        initialValue = None
         if type.nullable():
             declType = "Option<%s>" % declType
 
-        if isOptional:
-            declType = "Option<%s>" % declType
-            initialValue = "None"
-
-        return (getConversionCode(isOptional), CGGeneric(declType), False, initialValue)
+        return handleOptional(getConversionCode(), CGGeneric(declType), isOptional)
 
     if type.isEnum():
         assert not isEnforceRange and not isClamp
@@ -721,7 +718,7 @@ def getJSToNativeConversionTemplate(type, descriptorProvider, failureCode=None,
                                       (enum,
                                        getEnumValueName(defaultValue.value))))
 
-        return (template, CGGeneric(enum), isOptional, None)
+        return handleOptional(template, CGGeneric(enum), isOptional)
 
     if type.isCallback():
         assert not isEnforceRange and not isClamp
@@ -747,13 +744,8 @@ def getJSToNativeConversionTemplate(type, descriptorProvider, failureCode=None,
         assert not isEnforceRange and not isClamp
 
         declType = CGGeneric("JSVal")
-        value = CGGeneric("${val}")
-        if isOptional:
-            declType = CGWrapper(declType, pre="Option<", post=">")
-            value = CGWrapper(value, pre="Some(", post=")")
-
-        templateBody = handleDefaultNull(value.define(), "NullValue()")
-        return (templateBody, declType, isOptional, "None" if isOptional else None)
+        templateBody = handleDefaultNull("${val}", "NullValue()")
+        return handleOptional(templateBody, declType, isOptional)
 
     if type.isObject():
         raise TypeError("Can't handle object arguments yet")
@@ -784,7 +776,7 @@ def getJSToNativeConversionTemplate(type, descriptorProvider, failureCode=None,
                     "  Err(_) => return 0,\n"
                     "}" % (typeName, val))
 
-        return (template, declType, False, None)
+        return handleOptional(template, declType, isOptional)
 
     if type.isVoid():
         assert not isOptional
@@ -800,21 +792,16 @@ def getJSToNativeConversionTemplate(type, descriptorProvider, failureCode=None,
     if failureCode is None:
         failureCode = 'return 0'
 
-    value = "v"
     declType = CGGeneric(builtinNames[type.tag()])
     if type.nullable():
         declType = CGWrapper(declType, pre="Option<", post=">")
 
-    if isOptional:
-        value = "Some(%s)" % value
-        declType = CGWrapper(declType, pre="Option<", post=">")
-
     #XXXjdm support conversionBehavior here
     template = (
         "match FromJSValConvertible::from_jsval(cx, ${val}, ()) {\n"
-        "  Ok(v) => %s,\n"
+        "  Ok(v) => v,\n"
         "  Err(_) => { %s }\n"
-        "}" % (value, exceptionCode))
+        "}" % exceptionCode)
 
     if defaultValue is not None:
         if isinstance(defaultValue, IDLNullValue):
@@ -835,7 +822,7 @@ def getJSToNativeConversionTemplate(type, descriptorProvider, failureCode=None,
                                    CGGeneric(template),
                                    CGGeneric(defaultStr)).define()
 
-    return (template, declType, isOptional, "None" if isOptional else None)
+    return handleOptional(template, declType, isOptional)
 
 def instantiateJSToNativeConversionTemplate(templateTuple, replacements,
                                             argcAndIndex=None):
diff --git a/src/components/script/dom/testbinding.rs b/src/components/script/dom/testbinding.rs
index ade79ec93b3..76056272338 100644
--- a/src/components/script/dom/testbinding.rs
+++ b/src/components/script/dom/testbinding.rs
@@ -127,9 +127,9 @@ impl TestBinding {
     pub fn PassOptionalFloat(&self, _: Option<f32>) {}
     pub fn PassOptionalDouble(&self, _: Option<f64>) {}
     pub fn PassOptionalString(&self, _: Option<DOMString>) {}
-    // pub fn PassOptionalEnum(&self, _: Option<TestEnum>) {}
+    pub fn PassOptionalEnum(&self, _: Option<TestEnum>) {}
     pub fn PassOptionalInterface(&self, _: Option<JS<Blob>>) {}
-    // pub fn PassOptionalUnion(&self, _: Option<HTMLElementOrLong>) {}
+    pub fn PassOptionalUnion(&self, _: Option<HTMLElementOrLong>) {}
     pub fn PassOptionalAny(&self, _: *JSContext, _: Option<JSVal>) {}
 
     pub fn PassOptionalNullableBoolean(&self, _: Option<Option<bool>>) {}
@@ -145,8 +145,8 @@ impl TestBinding {
     pub fn PassOptionalNullableDouble(&self, _: Option<Option<f64>>) {}
     pub fn PassOptionalNullableString(&self, _: Option<Option<DOMString>>) {}
     // pub fn PassOptionalNullableEnum(&self, _: Option<Option<TestEnum>>) {}
-    // pub fn PassOptionalNullableInterface(&self, _: Option<Option<JS<Blob>>>) {}
-    // pub fn PassOptionalNullableUnion(&self, _: Option<Option<HTMLElementOrLong>>) {}
+    pub fn PassOptionalNullableInterface(&self, _: Option<Option<JS<Blob>>>) {}
+    pub fn PassOptionalNullableUnion(&self, _: Option<Option<HTMLElementOrLong>>) {}
 
     pub fn PassOptionalBooleanWithDefault(&self, _: bool) {}
     pub fn PassOptionalByteWithDefault(&self, _: i8) {}
diff --git a/src/components/script/dom/webidls/TestBinding.webidl b/src/components/script/dom/webidls/TestBinding.webidl
index 78e4d146897..5749214fba3 100644
--- a/src/components/script/dom/webidls/TestBinding.webidl
+++ b/src/components/script/dom/webidls/TestBinding.webidl
@@ -132,9 +132,9 @@ interface TestBinding {
   void passOptionalFloat(optional float arg);
   void passOptionalDouble(optional double arg);
   void passOptionalString(optional DOMString arg);
-  // void passOptionalEnum(optional TestEnum arg);
+  void passOptionalEnum(optional TestEnum arg);
   void passOptionalInterface(optional Blob arg);
-  // void passOptionalUnion(optional (HTMLElement or long) arg);
+  void passOptionalUnion(optional (HTMLElement or long) arg);
   void passOptionalAny(optional any arg);
 
   void passOptionalNullableBoolean(optional boolean? arg);
@@ -150,8 +150,8 @@ interface TestBinding {
   void passOptionalNullableDouble(optional double? arg);
   void passOptionalNullableString(optional DOMString? arg);
   // void passOptionalNullableEnum(optional TestEnum? arg);
-  // void passOptionalNullableInterface(optional Blob? arg);
-  // void passOptionalNullableUnion(optional (HTMLElement or long)? arg);
+  void passOptionalNullableInterface(optional Blob? arg);
+  void passOptionalNullableUnion(optional (HTMLElement or long)? arg);
 
   void passOptionalBooleanWithDefault(optional boolean arg = false);
   void passOptionalByteWithDefault(optional byte arg = 0);