Last Comment Bug 742213 - isDistinguishableFrom should check whether two interfaces can be implemented by the same object
: isDistinguishableFrom should check whether two interfaces can be implemented ...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla16
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on:
Blocks: ParisBindings
  Show dependency treegraph
 
Reported: 2012-04-03 22:20 PDT by Boris Zbarsky [:bz]
Modified: 2012-06-23 05:45 PDT (History)
3 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
isDistinguishableFrom should correctly check whether two interfaces can be implemented on the same object. (12.26 KB, patch)
2012-06-14 12:06 PDT, Boris Zbarsky [:bz]
justin.lebar+bug: review+
Details | Diff | Review

Description Boris Zbarsky [:bz] 2012-04-03 22:20:51 PDT
Instead of just assuming two different interfaces are always distinguishable.
Comment 1 Boris Zbarsky [:bz] 2012-06-13 21:53:54 PDT
Implementation plan:

On each IDLInterface C, store the following set of interfaces:

  { C, interfaces C is ancestor of, interfaces C is consequential interface of }

then A and B are distinguishable (ignoring typed array stuff and callbacks) if and only if A.getThisSet() intersect B.getThisSet() is empty.
Comment 2 Boris Zbarsky [:bz] 2012-06-14 12:06:15 PDT
Created attachment 633224 [details] [diff] [review]
isDistinguishableFrom should correctly check whether two interfaces can be implemented on the same object.
Comment 3 Justin Lebar (not reading bugmail) 2012-06-20 12:16:55 PDT
Where is test_distinguishability coming from?  It's not in my inbound tree.
Comment 4 Justin Lebar (not reading bugmail) 2012-06-20 13:01:44 PDT
> { C, interfaces C is ancestor of, interfaces C is consequential interface of }

This is the same as

  { C, interfaces C is a consequential interface of },

because if C is an ancestor of X, then C is a consequential interface of X, right?  So this is really just the converse (I don't like "inverse" here) of the consequential relation (union {C}, anyway).
Comment 5 Justin Lebar (not reading bugmail) 2012-06-20 13:41:21 PDT
Comment on attachment 633224 [details] [diff] [review]
isDistinguishableFrom should correctly check whether two interfaces can be implemented on the same object.

>diff --git a/dom/bindings/parser/WebIDL.py b/dom/bindings/parser/WebIDL.py
>--- a/dom/bindings/parser/WebIDL.py
>+++ b/dom/bindings/parser/WebIDL.py
>@@ -367,16 +367,17 @@ class IDLInterface(IDLObjectWithScope):
>         assert not parent or isinstance(parent, IDLIdentifierPlaceholder)
> 
>         self.parent = parent
>         self._callback = False
>         self._finished = False
>         self.members = list(members) # clone the list
>         self.implementedInterfaces = set()
>         self._consequential = False
>+        self.interfacesImplyingSelf = set([self])

I struggled with this for a while, and I think a simple explanation for this field is, the field contains the interfaces that |self| is a part of.  I think it's worth documenting that.  I also think the name could be improved, too; maybe interfacesBasedOnSelf?

>@@ -508,18 +529,19 @@ class IDLInterface(IDLObjectWithScope):
>             specialMembersSeen.add(memberType)
> 
>     def isInterface(self):
>         return True
> 
>     def isExternal(self):
>         return False
> 
>-    def setConsequential(self):
>+    def setConsequential(self, other):
>         self._consequential = True
>+        self.interfacesImplyingSelf.add(other)

Maybe now |setConsequentialTo|?

>+            if other.isSpiderMonkeyInterface():
>+                # Just let |other| handle things
>+                return other.isDistinguishableFrom(self)
>+            assert self.isGeckoInterface() and other.isGeckoInterface()
>+            if self.inner.isExternal() or other.unroll().inner.isExternal():
>+                return self != other
>+            return (len(self.inner.interfacesImplyingSelf &
>+                        other.unroll().inner.interfacesImplyingSelf) == 0 and

So two interfaces A and B are distinguishable if no object implements or inherits-from both A and B.

This creates really tight coupling -- my union or override can break if some bozo implements an object in some completely irrelevant spec.  But I guess we have to do this because we have no concept of an explicit cast.
Comment 6 Justin Lebar (not reading bugmail) 2012-06-20 13:42:05 PDT
> because if C is an ancestor of X, then C is a consequential interface of X, right?

Wrong.  :)
Comment 7 Boris Zbarsky [:bz] 2012-06-20 13:48:13 PDT
> Where is test_distinguishability coming from? 

Bug 764698.

> because if C is an ancestor of X, then C is a consequential interface of X, right?

As you discovered, wrong.

> I also think the name could be improved, too; maybe interfacesBasedOnSelf?

Will do.  And will document.

> Maybe now |setConsequentialTo|?

Hmm.  Or setConsequentialOf?  Preference?  Or even setIsConsequentialInterfaceOf?

Fully agreed with the last two paragraphs of comment 5.
Comment 8 Justin Lebar (not reading bugmail) 2012-06-20 13:51:15 PDT
I like setIsConsequentialInterfaceOf more than setConsequentialOf, and have no preference between setIsConsequentialInterfaceOf and set{Is,}ConsequentialTo.
Comment 9 Boris Zbarsky [:bz] 2012-06-20 22:01:14 PDT
setIsConsequentialInterfaceOf it is.
Comment 11 Ryan VanderMeulen [:RyanVM] 2012-06-23 05:45:21 PDT
https://hg.mozilla.org/mozilla-central/rev/a37c6fb970a4

Note You need to log in before you can comment on or make changes to this bug.