Bug 742152. Move distinguishing index determination and verification of type-identity of preceding arguments out of codegen and into the WebIDL parser. r=jlebar

dom/bindings/Codegen.py

@@ -2483,29 +2483,6 @@ class CGMethodCall(CGThing):
                 requiredArgs -= 1
             return requiredArgs
 
-        def maxSigLength(signatures):
-            return max([len(s[1]) for s in signatures])
-
-        def signaturesForArgCount(i, signatures):
-            return filter(
-                lambda s: len(s[1]) == i or (len(s[1]) > i and
-                                             s[1][i].optional),
-                    signatures)
-
-        def findDistinguishingIndex(argCount, signatures):
-            def isValidDistinguishingIndex(idx, signatures):
-                for firstSigIndex in range(0, len(signatures)):
-                    for secondSigIndex in range(0, firstSigIndex):
-                        firstType = signatures[firstSigIndex][1][idx].type
-                        secondType = signatures[secondSigIndex][1][idx].type
-                        if not firstType.isDistinguishableFrom(secondType):
-                            return False
-                return True
-            for idx in range(0, argCount):
-                if isValidDistinguishingIndex(idx, signatures):
-                    return idx
-            return -1
-
         def getPerSignatureCall(signature, argConversionStartsAt=0):
             return CGPerSignatureCall(signature[0], argsPre, signature[1],
                                       nativeMethodName, static, descriptor,
@@ -2535,13 +2512,12 @@ class CGMethodCall(CGThing):
             return
 
         # Need to find the right overload
-        maxSigArgs = maxSigLength(signatures)
+        maxArgCount = method.maxArgCount
-        allowedArgCounts = [ i for i in range(0, maxSigArgs+1)
+        allowedArgCounts = method.allowedArgCounts
-                             if len(signaturesForArgCount(i, signatures)) != 0 ]
 
         argCountCases = []
         for argCount in allowedArgCounts:
-            possibleSignatures = signaturesForArgCount(argCount, signatures)
+            possibleSignatures = method.signaturesForArgCount(argCount)
             if len(possibleSignatures) == 1:
                 # easy case!
                 signature = possibleSignatures[0]
@@ -2554,7 +2530,7 @@ class CGMethodCall(CGThing):
                 if (len(signature[1]) > argCount and
                     signature[1][argCount].optional and
                     (argCount+1) in allowedArgCounts and
-                    len(signaturesForArgCount(argCount+1, signatures)) == 1):
+                    len(method.signaturesForArgCount(argCount+1)) == 1):
                     argCountCases.append(
                         CGCase(str(argCount), None, True))
                 else:
@@ -2562,28 +2538,7 @@ class CGMethodCall(CGThing):
                         CGCase(str(argCount), getPerSignatureCall(signature)))
                 continue
 
-            distinguishingIndex = findDistinguishingIndex(argCount,
+            distinguishingIndex = method.distinguishingIndexForArgCount(argCount)
-                                                          possibleSignatures)
-            if distinguishingIndex == -1:
-                raise TypeError(("Signatures with %s arguments for " +
-                                 descriptor.interface.identifier.name + "." +
-                                 method.identifier.name +
-                                 " are not distinguishable") % argCount)
-
-            for idx in range(0, distinguishingIndex):
-                firstSigType = possibleSignatures[0][1][idx].type
-                for sigIdx in range(1, len(possibleSignatures)):
-                    if possibleSignatures[sigIdx][1][idx].type != firstSigType:
-                        raise TypeError(("Signatures with %d arguments for " +
-                                         descriptor.interface.identifier.name +
-                                         "." + method.identifier.name +
-                                         " have different types at index %d" +
-                                         " which is before distinguishing" +
-                                         " index %d.  Types are %s and %s") %
-                                        (argCount, idx,
-                                         distinguishingIndex,
-                                         str(possibleSignatures[sigIdx][1][idx].type),
-                                         str(firstSigType)))
 
             # Convert all our arguments up to the distinguishing index.
             # Doesn't matter which of the possible signatures we use, since
@@ -2723,7 +2678,7 @@ class CGMethodCall(CGThing):
         overloadCGThings = []
         overloadCGThings.append(
             CGGeneric("unsigned argcount = NS_MIN(argc, %du);" %
-                      maxSigArgs))
+                      maxArgCount))
         overloadCGThings.append(
             CGSwitch("argcount",
                      argCountCases,

dom/bindings/parser/WebIDL.py

@@ -51,19 +51,21 @@ def enum(*names):
     return Foo()
 
 class WebIDLError(Exception):
-    def __init__(self, message, location, warning=False, extraLocation=""):
+    def __init__(self, message, location, warning=False, extraLocations=[]):
         self.message = message
         self.location = location
         self.warning = warning
-        self.extraLocation = extraLocation
+        self.extraLocations = [str(loc) for loc in extraLocations]
 
     def __str__(self):
-        return "%s: %s%s%s%s%s" % (self.warning and 'warning' or 'error',
+        extraLocationsStr = (
+            "" if len(self.extraLocations) == 0 else
+            "\n" + "\n".join(self.extraLocations))
+        return "%s: %s%s%s%s" % (self.warning and 'warning' or 'error',
                                  self.message,
                                  ", " if self.location else "",
                                  self.location,
-                                   "\n" if self.extraLocation else "",
+                                 extraLocationsStr)
-                                   self.extraLocation)
 
 class Location(object):
     def __init__(self, lexer, lineno, lexpos, filename):
@@ -345,6 +347,9 @@ class IDLExternalInterface(IDLObjectWithIdentifier):
     def finish(self, scope):
         pass
 
+    def validate(self):
+        pass
+
     def isExternal(self):
         return True
 
@@ -436,7 +441,7 @@ class IDLInterface(IDLObjectWithScope):
                                       (self.identifier.name,
                                        self.parent.identifier.name),
                                       self.location,
-                                      extraLocation=self.parent.location)
+                                      extraLocations=[self.parent.location])
                 if len(self.parent.getConsequentialInterfaces()) != 0:
                     raise WebIDLError("Callback interface %s inheriting from "
                                       "interface %s which has consequential "
@@ -444,7 +449,7 @@ class IDLInterface(IDLObjectWithScope):
                                       (self.identifier.name,
                                        self.parent.identifier.name),
                                       self.location,
-                                      extraLocation=self.parent.location)
+                                      extraLocations=[self.parent.location])
 
         for iface in self.implementedInterfaces:
             iface.finish(scope)
@@ -454,7 +459,7 @@ class IDLInterface(IDLObjectWithScope):
             raise WebIDLError("Interface %s has itself as ancestor or "
                               "implemented interface" % self.identifier.name,
                               self.location,
-                              extraLocation=cycleInGraph.location)
+                              extraLocations=[cycleInGraph.location])
 
         # Now resolve() and finish() our members before importing the
         # ones from our implemented interfaces.
@@ -492,7 +497,7 @@ class IDLInterface(IDLObjectWithScope):
                             "Multiple definitions of %s on %s coming from 'implements' statements" %
                             (member.identifier.name, self),
                             additionalMember.location,
-                            extraLocation=member.location)
+                            extraLocations=[member.location])
             self.members.extend(additionalMembers)
 
         for ancestor in self.getInheritedInterfaces():
@@ -531,6 +536,10 @@ class IDLInterface(IDLObjectWithScope):
 
             specialMembersSeen.add(memberType)
 
+    def validate(self):
+        for member in self.members:
+            member.validate()
+
     def isInterface(self):
         return True
 
@@ -690,7 +699,7 @@ class IDLDictionary(IDLObjectWithScope):
                 raise WebIDLError("Dictionary %s has parent that is not a dictionary" %
                                   self.identifier.name,
                                   oldParent.location,
-                                  extraLocation=self.parent.location)
+                                  extraLocations=[self.parent.location])
 
             # Make sure the parent resolves all its members before we start
             # looking at them.
@@ -724,7 +733,10 @@ class IDLDictionary(IDLObjectWithScope):
                     raise WebIDLError("Dictionary %s has two members with name %s" %
                                       (self.identifier.name, member.identifier.name),
                                       member.location,
-                                      extraLocation=inheritedMember.location)
+                                      extraLocations=[inheritedMember.location])
+
+    def validate(self):
+        pass
 
     def addExtendedAttributes(self, attrs):
         assert len(attrs) == 0
@@ -747,6 +759,9 @@ class IDLEnum(IDLObjectWithIdentifier):
     def finish(self, scope):
         pass
 
+    def validate(self):
+        pass
+
     def isEnum(self):
         return True
 
@@ -1292,11 +1307,11 @@ class IDLWrapperType(IDLType):
                     other.isCallback() or other.isDictionary() or
                     other.isSequence() or other.isArray() or
                     other.isDate())
-        if other.isPrimitive() or other.isString() or other.isEnum():
+        if other.isPrimitive() or other.isString() or other.isEnum() or other.isDate():
             return True
         if self.isDictionary():
             return (other.isNonCallbackInterface() or other.isSequence() or
-                    other.isArray() or other.isDate())
+                    other.isArray())
 
         assert self.isInterface()
         # XXXbz need to check that the interfaces can't be implemented
@@ -1316,6 +1331,10 @@ class IDLWrapperType(IDLType):
             other.isSequence() or other.isArray()):
             return self.isNonCallbackInterface()
 
+        # Not much else |other| can be
+        assert other.isObject()
+        return False
+
 class IDLBuiltinType(IDLType):
 
     Types = enum(
@@ -1682,6 +1701,9 @@ class IDLConst(IDLInterfaceMember):
     def finish(self, scope):
         assert self.type.isComplete()
 
+    def validate(self):
+        pass
+
 class IDLAttribute(IDLInterfaceMember):
     def __init__(self, location, identifier, type, readonly, inherit):
         IDLInterfaceMember.__init__(self, location, identifier,
@@ -1710,6 +1732,9 @@ class IDLAttribute(IDLInterfaceMember):
                                   self.location)
             self.type = t
 
+    def validate(self):
+        pass
+
     def handleExtendedAttribute(self, name, list):
         if name == "TreatNonCallableAsNull":
             self.type.markTreatNonCallableAsNull();
@@ -1783,6 +1808,9 @@ class IDLCallbackType(IDLType, IDLObjectWithScope):
             assert not isinstance(type.name, IDLUnresolvedIdentifier)
             argument.type = type
 
+    def validate(self):
+        pass
+
     def isDistinguishableFrom(self, other):
         return (other.isPrimitive() or other.isString() or other.isEnum() or
                 other.isNonCallbackInterface() or other.isDate())
@@ -1824,6 +1852,9 @@ class IDLMethod(IDLInterfaceMember, IDLScope):
 
         assert isinstance(returnType, IDLType)
         self._returnType = [returnType]
+        # We store a list of all the overload locations, matching our
+        # signature list.
+        self._location = [location]
 
         assert isinstance(static, bool)
         self._static = static
@@ -1870,20 +1901,32 @@ class IDLMethod(IDLInterfaceMember, IDLScope):
             assert self._returnType[0] == BuiltinTypes[IDLBuiltinType.Types.domstring]
 
         inOptionalArguments = False
-        sawVariadicArgument = False
+        variadicArgument = None
+        sawOptionalWithNoDefault = False
 
         assert len(self._arguments) == 1
         arguments = self._arguments[0]
 
         for argument in arguments:
             # Only the last argument can be variadic
-            assert not sawVariadicArgument
+            if variadicArgument:
+                raise WebIDLError("Variadic argument is not last argument",
+                                  variadicArgument.location)
             # Once we see an optional argument, there can't be any non-optional
             # arguments.
-            if inOptionalArguments:
+            if inOptionalArguments and not argument.optional:
-                assert argument.optional
+                raise WebIDLError("Non-optional argument after optional arguments",
+                                  argument.location)
+            # Once we see an argument with no default value, there can
+            # be no more default values.
+            if sawOptionalWithNoDefault and argument.defaultValue:
+                raise WebIDLError("Argument with default value after optional "
+                                  "arguments with no default values",
+                                  argument.location)
             inOptionalArguments = argument.optional
-            sawVariadicArgument = argument.variadic
+            if argument.variadic:
+                variadicArgument = argument
+            sawOptionalWithNoDefault = argument.optional and not argument.defaultValue
 
     def isStatic(self):
         return self._static
@@ -1932,9 +1975,11 @@ class IDLMethod(IDLInterfaceMember, IDLScope):
 
         assert len(method._returnType) == 1
         assert len(method._arguments) == 1
+        assert len(method._location) == 1
 
         self._returnType.extend(method._returnType)
         self._arguments.extend(method._arguments)
+        self._location.extend(method._location)
 
         self._hasOverloads = True
 
@@ -1986,6 +2031,64 @@ class IDLMethod(IDLInterfaceMember, IDLScope):
                 assert not isinstance(type.name, IDLUnresolvedIdentifier)
                 argument.type = type
 
+        # Now compute various information that will be used by the
+        # WebIDL overload resolution algorithm.
+        self.maxArgCount = max(len(s[1]) for s in self.signatures())
+        self.allowedArgCounts = [ i for i in range(self.maxArgCount+1)
+                                  if len(self.signaturesForArgCount(i)) != 0 ]
+
+    def validate(self):
+        # Make sure our overloads are properly distinguishable and don't have
+        # different argument types before the distinguishing args.
+        for argCount in self.allowedArgCounts:
+            possibleSignatures = self.signaturesForArgCount(argCount)
+            if len(possibleSignatures) == 1:
+                continue
+            distinguishingIndex = self.distinguishingIndexForArgCount(argCount)
+            arglists = [ s[1] for s in possibleSignatures ]
+            for idx in range(distinguishingIndex):
+                firstSigType = arglists[0][idx].type
+                for (otherArgList, location) in zip(arglists[1:],
+                                                    self._location[1:]):
+                    if otherArgList[idx].type != firstSigType:
+                        raise WebIDLError(
+                            "Signatures for method '%s' with %d arguments have "
+                            "different types of arguments at index %d, which "
+                            "is before distinguishing index %d" %
+                            (self.identifier.name, argCount, idx,
+                             distinguishingIndex),
+                            self.location,
+                            extraLocations=[location])
+
+    def signaturesForArgCount(self, argc):
+        return [(retval, args) for (retval, args) in self.signatures() if
+                len(args) == argc or (len(args) > argc and args[argc].optional)]
+
+    def locationsForArgCount(self, argc):
+        return [ self._location[i] for (i, args) in enumerate(self._arguments) if
+                 len(args) == argc or
+                 (len(args) > argc and args[argc].optional)]
+
+    def distinguishingIndexForArgCount(self, argc):
+        def isValidDistinguishingIndex(idx, signatures):
+            for (firstSigIndex, (firstRetval, firstArgs)) in enumerate(signatures[:-1]):
+                for (secondRetval, secondArgs) in signatures[firstSigIndex+1:]:
+                    firstType = firstArgs[idx].type
+                    secondType = secondArgs[idx].type
+                    if not firstType.isDistinguishableFrom(secondType):
+                        return False
+            return True
+        signatures = self.signaturesForArgCount(argc)
+        for idx in range(argc):
+            if isValidDistinguishingIndex(idx, signatures):
+                return idx
+        # No valid distinguishing index.  Time to throw
+        locations = self.locationsForArgCount(argc)
+        raise WebIDLError("Signatures with %d arguments for method '%s' are not "
+                          "distinguishable" % (argc, self.identifier.name),
+                          locations[0],
+                          extraLocations=locations[1:])
+
 class IDLImplementsStatement(IDLObject):
     def __init__(self, location, implementor, implementee):
         IDLObject.__init__(self, location)
@@ -1999,6 +2102,9 @@ class IDLImplementsStatement(IDLObject):
         implementee = self.implementee.finish(scope)
         implementor.addImplementedInterface(implementee)
 
+    def validate(self):
+        pass
+
     def addExtendedAttributes(self, attrs):
         assert len(attrs) == 0
 
@@ -3245,6 +3351,10 @@ class Parser(Tokenizer):
         for production in otherStatements:
             production.finish(self.globalScope())
 
+        # Do any post-finish validation we need to do
+        for production in self._productions:
+            production.validate()
+
         # De-duplicate self._productions, without modifying its order.
         seen = set()
         result = []

dom/bindings/parser/tests/test_distinguishability.py

@@ -76,3 +76,57 @@ def WebIDLTest(parser, harness):
                           distinguishable,
                           "Type %s should %sbe distinguishable from type %s" %
                           (type2, "" if distinguishable else "not ", type1))
+
+    parser = parser.reset()
+    parser.parse("""
+      interface Dummy {};
+      interface TestIface {
+        void method(long arg1, TestIface arg2);
+        void method(long arg1, long arg2);
+        void method(long arg1, Dummy arg2);
+        void method(DOMString arg1, DOMString arg2, DOMString arg3);
+      };
+    """)
+    results = parser.finish()
+    harness.check(len(results[1].members), 1,
+                  "Should look like we have one method")
+    harness.check(len(results[1].members[0].signatures()), 4,
+                  "Should have foid signatures")
+
+    parser = parser.reset()
+    threw = False
+    try:
+        parser.parse("""
+          interface Dummy {};
+          interface TestIface {
+            void method(long arg1, TestIface arg2);
+            void method(long arg1, long arg2);
+            void method(any arg1,  Dummy arg2);
+            void method(DOMString arg1, DOMString arg2, DOMString arg3);
+          };
+        """)
+        results = parser.finish()
+    except:
+        threw = True
+
+    harness.ok(threw,
+               "Should throw when args before the distinguishing arg are not "
+               "all the same type")
+
+    parser = parser.reset()
+    threw = False
+    try:
+        parser.parse("""
+          interface Dummy {};
+          interface TestIface {
+            void method(long arg1, TestIface arg2);
+            void method(long arg1, long arg2);
+            void method(any arg1,  DOMString arg2);
+            void method(DOMString arg1, DOMString arg2, DOMString arg3);
+          };
+        """)
+        results = parser.finish()
+    except:
+        threw = True
+
+    harness.ok(threw, "Should throw when there is no distinguishing index")

dom/bindings/parser/tests/test_overload.py

@@ -5,9 +5,9 @@ def WebIDLTest(parser, harness):
         interface TestOverloads {
           void basic();
           void basic(long arg1);
-          boolean abitharder(unsigned long foo);
+          boolean abitharder(TestOverloads foo);
           boolean abitharder(boolean foo);
-          void abitharder(long? foo);
+          void abitharder(ArrayBuffer? foo);
         };
     """)