Title: [207705] trunk/Source/WebCore
Revision
207705
Author
cdu...@apple.com
Date
2016-10-21 22:27:25 -0700 (Fri, 21 Oct 2016)

Log Message

[Web ID] Overload resolution is wrong if one of the types is a nullable union
https://bugs.webkit.org/show_bug.cgi?id=163816

Reviewed by Alex Christensen.

Overload resolution was wrong if one of the types was a nullable union. This
is because we never considered the union type itself, only its subtypes.
Therefore, we checked if any of the union's subtypes were nullable but we
failed to check if the union itself was nullable.

See:
- https://heycam.github.io/webidl/#es-overloads (Step 11.3.)

No new tests, extended bindings tests.

* bindings/scripts/CodeGeneratorJS.pm:
(GetOverloadThatMatchesIgnoringUnionSubtypes):
(GenerateOverloadedFunctionOrConstructor):
* bindings/scripts/test/JS/JSTestObj.cpp:
(WebCore::jsTestObjPrototypeFunctionOverloadWithNullableUnion1):
(WebCore::jsTestObjPrototypeFunctionOverloadWithNullableUnion1Caller):
(WebCore::jsTestObjPrototypeFunctionOverloadWithNullableUnion2):
(WebCore::jsTestObjPrototypeFunctionOverloadWithNullableUnion2Caller):
(WebCore::jsTestObjPrototypeFunctionOverloadWithNullableUnion):
(WebCore::jsTestObjPrototypeFunctionOverloadWithOptionalUnion1):
(WebCore::jsTestObjPrototypeFunctionOverloadWithOptionalUnion1Caller):
(WebCore::jsTestObjPrototypeFunctionOverloadWithOptionalUnion2):
(WebCore::jsTestObjPrototypeFunctionOverloadWithOptionalUnion2Caller):
(WebCore::jsTestObjPrototypeFunctionOverloadWithOptionalUnion):
* bindings/scripts/test/TestObj.idl:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (207704 => 207705)


--- trunk/Source/WebCore/ChangeLog	2016-10-22 03:35:04 UTC (rev 207704)
+++ trunk/Source/WebCore/ChangeLog	2016-10-22 05:27:25 UTC (rev 207705)
@@ -1,3 +1,36 @@
+2016-10-21  Chris Dumez  <cdu...@apple.com>
+
+        [Web ID] Overload resolution is wrong if one of the types is a nullable union
+        https://bugs.webkit.org/show_bug.cgi?id=163816
+
+        Reviewed by Alex Christensen.
+
+        Overload resolution was wrong if one of the types was a nullable union. This
+        is because we never considered the union type itself, only its subtypes.
+        Therefore, we checked if any of the union's subtypes were nullable but we
+        failed to check if the union itself was nullable.
+
+        See:
+        - https://heycam.github.io/webidl/#es-overloads (Step 11.3.)
+
+        No new tests, extended bindings tests.
+
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GetOverloadThatMatchesIgnoringUnionSubtypes):
+        (GenerateOverloadedFunctionOrConstructor):
+        * bindings/scripts/test/JS/JSTestObj.cpp:
+        (WebCore::jsTestObjPrototypeFunctionOverloadWithNullableUnion1):
+        (WebCore::jsTestObjPrototypeFunctionOverloadWithNullableUnion1Caller):
+        (WebCore::jsTestObjPrototypeFunctionOverloadWithNullableUnion2):
+        (WebCore::jsTestObjPrototypeFunctionOverloadWithNullableUnion2Caller):
+        (WebCore::jsTestObjPrototypeFunctionOverloadWithNullableUnion):
+        (WebCore::jsTestObjPrototypeFunctionOverloadWithOptionalUnion1):
+        (WebCore::jsTestObjPrototypeFunctionOverloadWithOptionalUnion1Caller):
+        (WebCore::jsTestObjPrototypeFunctionOverloadWithOptionalUnion2):
+        (WebCore::jsTestObjPrototypeFunctionOverloadWithOptionalUnion2Caller):
+        (WebCore::jsTestObjPrototypeFunctionOverloadWithOptionalUnion):
+        * bindings/scripts/test/TestObj.idl:
+
 2016-10-21  Eric Carlson  <eric.carl...@apple.com>
 
         [MediaStream] Dynamically generate media capture sandbox extensions

Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (207704 => 207705)


--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2016-10-22 03:35:04 UTC (rev 207704)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2016-10-22 05:27:25 UTC (rev 207705)
@@ -2037,6 +2037,17 @@
     }
 }
 
+sub GetOverloadThatMatchesIgnoringUnionSubtypes
+{
+    my ($S, $parameterIndex, $matches) = @_;
+
+    for my $tuple (@{$S}) {
+        my $idlType = @{@{$tuple}[1]}[$parameterIndex];
+        my $optionality = @{@{$tuple}[2]}[$parameterIndex];
+        return @{$tuple}[0] if $matches->($idlType, $optionality);
+    }
+}
+
 sub getConditionalForFunctionConsideringOverloads
 {
     my $function = shift;
@@ -2086,9 +2097,17 @@
         my ($idlType, $optionality) = @_;
         return $idlType->name eq "Dictionary" || $codeGenerator->IsDictionaryType($idlType->name);
     };
-    my $isNullableOrDictionaryParameter = sub {
+    my $isNullableOrDictionaryParameterOrUnionContainingOne = sub {
         my ($idlType, $optionality) = @_;
-        return $idlType->isNullable || &$isDictionaryParameter($idlType, $optionality);
+        return 1 if $idlType->isNullable;
+        if ($idlType->isUnion) {
+            for my $idlSubtype (GetFlattenedMemberTypes($idlType)) {
+                return 1 if $idlType->isNullable || &$isDictionaryParameter($idlSubtype, $optionality);
+            }
+            return 0;
+        } else {
+            return &$isDictionaryParameter($idlType, $optionality);
+        }
     };
     my $isRegExpOrObjectParameter = sub {
         my ($idlType, $optionality) = @_;
@@ -2162,10 +2181,10 @@
             my $d = GetDistinguishingArgumentIndex($function, $S);
             push(@implContent, "        JSValue distinguishingArg = state->uncheckedArgument($d);\n");
 
-            my $overload = GetOverloadThatMatches($S, $d, \&$isOptionalParameter);
+            my $overload = GetOverloadThatMatchesIgnoringUnionSubtypes($S, $d, \&$isOptionalParameter);
             &$generateOverloadCallIfNecessary($overload, "distinguishingArg.isUndefined()");
 
-            $overload = GetOverloadThatMatches($S, $d, \&$isNullableOrDictionaryParameter);
+            $overload = GetOverloadThatMatchesIgnoringUnionSubtypes($S, $d, \&$isNullableOrDictionaryParameterOrUnionContainingOne);
             &$generateOverloadCallIfNecessary($overload, "distinguishingArg.isUndefinedOrNull()");
 
             for my $tuple (@{$S}) {

Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp (207704 => 207705)


--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp	2016-10-22 03:35:04 UTC (rev 207704)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp	2016-10-22 05:27:25 UTC (rev 207705)
@@ -960,6 +960,8 @@
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionOverloadedMethodWithDistinguishingUnion(JSC::ExecState*);
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionOverloadedMethodWith2DistinguishingUnions(JSC::ExecState*);
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionOverloadedMethodWithNonDistinguishingUnion(JSC::ExecState*);
+JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionOverloadWithNullableUnion(JSC::ExecState*);
+JSC::EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionOverloadWithOptionalUnion(JSC::ExecState*);
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjConstructorFunctionClassMethod(JSC::ExecState*);
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjConstructorFunctionClassMethodWithOptional(JSC::ExecState*);
 JSC::EncodedJSValue JSC_HOST_CALL jsTestObjConstructorFunctionClassMethod2(JSC::ExecState*);
@@ -1581,6 +1583,8 @@
     { "overloadedMethodWithDistinguishingUnion", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionOverloadedMethodWithDistinguishingUnion), (intptr_t) (1) } },
     { "overloadedMethodWith2DistinguishingUnions", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionOverloadedMethodWith2DistinguishingUnions), (intptr_t) (1) } },
     { "overloadedMethodWithNonDistinguishingUnion", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionOverloadedMethodWithNonDistinguishingUnion), (intptr_t) (2) } },
+    { "overloadWithNullableUnion", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionOverloadWithNullableUnion), (intptr_t) (1) } },
+    { "overloadWithOptionalUnion", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionOverloadWithOptionalUnion), (intptr_t) (0) } },
     { "classMethodWithClamp", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionClassMethodWithClamp), (intptr_t) (2) } },
     { "classMethodWithEnforceRange", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionClassMethodWithEnforceRange), (intptr_t) (2) } },
     { "methodWithUnsignedLongSequence", JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestObjPrototypeFunctionMethodWithUnsignedLongSequence), (intptr_t) (1) } },
@@ -7030,6 +7034,127 @@
     return argsCount < 2 ? throwVMError(state, throwScope, createNotEnoughArgumentsError(state)) : throwVMTypeError(state, throwScope);
 }
 
+static inline JSC::EncodedJSValue jsTestObjPrototypeFunctionOverloadWithNullableUnion1Caller(JSC::ExecState*, JSTestObj*, JSC::ThrowScope&);
+
+static inline EncodedJSValue jsTestObjPrototypeFunctionOverloadWithNullableUnion1(ExecState* state)
+{
+    return BindingCaller<JSTestObj>::callOperation<jsTestObjPrototypeFunctionOverloadWithNullableUnion1Caller>(state, "overloadWithNullableUnion");
+}
+
+static inline JSC::EncodedJSValue jsTestObjPrototypeFunctionOverloadWithNullableUnion1Caller(JSC::ExecState* state, JSTestObj* castedThis, JSC::ThrowScope& throwScope)
+{
+    UNUSED_PARAM(state);
+    UNUSED_PARAM(throwScope);
+    auto& impl = castedThis->wrapped();
+    if (UNLIKELY(state->argumentCount() < 1))
+        return throwVMError(state, throwScope, createNotEnoughArgumentsError(state));
+    auto objectOrNode = convert<IDLNullable<IDLUnion<IDLInterface<TestObj>, IDLInterface<TestNode>>>>(*state, state->uncheckedArgument(0));
+    RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
+    impl.overloadWithNullableUnion(WTFMove(objectOrNode));
+    return JSValue::encode(jsUndefined());
+}
+
+static inline JSC::EncodedJSValue jsTestObjPrototypeFunctionOverloadWithNullableUnion2Caller(JSC::ExecState*, JSTestObj*, JSC::ThrowScope&);
+
+static inline EncodedJSValue jsTestObjPrototypeFunctionOverloadWithNullableUnion2(ExecState* state)
+{
+    return BindingCaller<JSTestObj>::callOperation<jsTestObjPrototypeFunctionOverloadWithNullableUnion2Caller>(state, "overloadWithNullableUnion");
+}
+
+static inline JSC::EncodedJSValue jsTestObjPrototypeFunctionOverloadWithNullableUnion2Caller(JSC::ExecState* state, JSTestObj* castedThis, JSC::ThrowScope& throwScope)
+{
+    UNUSED_PARAM(state);
+    UNUSED_PARAM(throwScope);
+    auto& impl = castedThis->wrapped();
+    if (UNLIKELY(state->argumentCount() < 1))
+        return throwVMError(state, throwScope, createNotEnoughArgumentsError(state));
+    auto index = convert<IDLLong>(*state, state->uncheckedArgument(0), NormalConversion);
+    RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
+    impl.overloadWithNullableUnion(WTFMove(index));
+    return JSValue::encode(jsUndefined());
+}
+
+EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionOverloadWithNullableUnion(ExecState* state)
+{
+    VM& vm = state->vm();
+    auto throwScope = DECLARE_THROW_SCOPE(vm);
+    UNUSED_PARAM(throwScope);
+    size_t argsCount = std::min<size_t>(1, state->argumentCount());
+    if (argsCount == 1) {
+        JSValue distinguishingArg = state->uncheckedArgument(0);
+        if (distinguishingArg.isUndefinedOrNull())
+            return jsTestObjPrototypeFunctionOverloadWithNullableUnion1(state);
+        if (distinguishingArg.isObject() && asObject(distinguishingArg)->inherits(JSTestObj::info()))
+            return jsTestObjPrototypeFunctionOverloadWithNullableUnion1(state);
+        if (distinguishingArg.isObject() && asObject(distinguishingArg)->inherits(JSTestNode::info()))
+            return jsTestObjPrototypeFunctionOverloadWithNullableUnion1(state);
+        if (distinguishingArg.isNumber())
+            return jsTestObjPrototypeFunctionOverloadWithNullableUnion2(state);
+        return jsTestObjPrototypeFunctionOverloadWithNullableUnion2(state);
+    }
+    return argsCount < 1 ? throwVMError(state, throwScope, createNotEnoughArgumentsError(state)) : throwVMTypeError(state, throwScope);
+}
+
+static inline JSC::EncodedJSValue jsTestObjPrototypeFunctionOverloadWithOptionalUnion1Caller(JSC::ExecState*, JSTestObj*, JSC::ThrowScope&);
+
+static inline EncodedJSValue jsTestObjPrototypeFunctionOverloadWithOptionalUnion1(ExecState* state)
+{
+    return BindingCaller<JSTestObj>::callOperation<jsTestObjPrototypeFunctionOverloadWithOptionalUnion1Caller>(state, "overloadWithOptionalUnion");
+}
+
+static inline JSC::EncodedJSValue jsTestObjPrototypeFunctionOverloadWithOptionalUnion1Caller(JSC::ExecState* state, JSTestObj* castedThis, JSC::ThrowScope& throwScope)
+{
+    UNUSED_PARAM(state);
+    UNUSED_PARAM(throwScope);
+    auto& impl = castedThis->wrapped();
+    auto objectOrNode = state->argument(0).isUndefined() ? true : convert<IDLUnion<IDLDOMString, IDLBoolean>>(*state, state->uncheckedArgument(0));
+    RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
+    impl.overloadWithOptionalUnion(WTFMove(objectOrNode));
+    return JSValue::encode(jsUndefined());
+}
+
+static inline JSC::EncodedJSValue jsTestObjPrototypeFunctionOverloadWithOptionalUnion2Caller(JSC::ExecState*, JSTestObj*, JSC::ThrowScope&);
+
+static inline EncodedJSValue jsTestObjPrototypeFunctionOverloadWithOptionalUnion2(ExecState* state)
+{
+    return BindingCaller<JSTestObj>::callOperation<jsTestObjPrototypeFunctionOverloadWithOptionalUnion2Caller>(state, "overloadWithOptionalUnion");
+}
+
+static inline JSC::EncodedJSValue jsTestObjPrototypeFunctionOverloadWithOptionalUnion2Caller(JSC::ExecState* state, JSTestObj* castedThis, JSC::ThrowScope& throwScope)
+{
+    UNUSED_PARAM(state);
+    UNUSED_PARAM(throwScope);
+    auto& impl = castedThis->wrapped();
+    if (UNLIKELY(state->argumentCount() < 1))
+        return throwVMError(state, throwScope, createNotEnoughArgumentsError(state));
+    auto index = convert<IDLLong>(*state, state->uncheckedArgument(0), NormalConversion);
+    RETURN_IF_EXCEPTION(throwScope, encodedJSValue());
+    impl.overloadWithOptionalUnion(WTFMove(index));
+    return JSValue::encode(jsUndefined());
+}
+
+EncodedJSValue JSC_HOST_CALL jsTestObjPrototypeFunctionOverloadWithOptionalUnion(ExecState* state)
+{
+    VM& vm = state->vm();
+    auto throwScope = DECLARE_THROW_SCOPE(vm);
+    UNUSED_PARAM(throwScope);
+    size_t argsCount = std::min<size_t>(1, state->argumentCount());
+    if (argsCount == 0) {
+        return jsTestObjPrototypeFunctionOverloadWithOptionalUnion1(state);
+    }
+    if (argsCount == 1) {
+        JSValue distinguishingArg = state->uncheckedArgument(0);
+        if (distinguishingArg.isUndefined())
+            return jsTestObjPrototypeFunctionOverloadWithOptionalUnion1(state);
+        if (distinguishingArg.isBoolean())
+            return jsTestObjPrototypeFunctionOverloadWithOptionalUnion1(state);
+        if (distinguishingArg.isNumber())
+            return jsTestObjPrototypeFunctionOverloadWithOptionalUnion2(state);
+        return jsTestObjPrototypeFunctionOverloadWithOptionalUnion1(state);
+    }
+    return throwVMTypeError(state, throwScope);
+}
+
 EncodedJSValue JSC_HOST_CALL jsTestObjConstructorFunctionClassMethod(ExecState* state)
 {
     VM& vm = state->vm();

Modified: trunk/Source/WebCore/bindings/scripts/test/TestObj.idl (207704 => 207705)


--- trunk/Source/WebCore/bindings/scripts/test/TestObj.idl	2016-10-22 03:35:04 UTC (rev 207704)
+++ trunk/Source/WebCore/bindings/scripts/test/TestObj.idl	2016-10-22 05:27:25 UTC (rev 207705)
@@ -307,6 +307,12 @@
 
     void overloadedMethodWithNonDistinguishingUnion((TestObj or TestNode) objectOrNode, TestObj object);
     void overloadedMethodWithNonDistinguishingUnion((TestObj or TestNode) objectOrNode, TestNode node);
+
+    void overloadWithNullableUnion((TestObj or TestNode)? objectOrNode);
+    void overloadWithNullableUnion(long index);
+
+    void overloadWithOptionalUnion(optional (DOMString or boolean) objectOrNode = true);
+    void overloadWithOptionalUnion(long index);
 #endif
 
     // Class methods within _javascript_ (like what's used for IDBKeyRange).
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to