The default bug view has changed. See this FAQ.

The WebIDL parser should detect interfaces that inherit from or implement themselves

RESOLVED FIXED in mozilla16

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

(Blocks: 1 bug)

unspecified
mozilla16
x86
Mac OS X
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Right now I think we go into an infinite loop or something.  We should throw.  "implements" makes this a bit of a pain, possibly.
Created attachment 632893 [details] [diff] [review]
Detect cycles in the interface inheritance and implementation graph.
Attachment #632893 - Flags: review?(khuey)
Assignee: nobody → bzbarsky
Whiteboard: [need review]
Comment on attachment 632893 [details] [diff] [review]
Detect cycles in the interface inheritance and implementation graph.

It looks like my distinguishability-checking move depends on this.  :(
Attachment #632893 - Flags: review?(khuey) → review?(justin.lebar+bug)
>+        cycleInGraph = self.inheritsFromOrImplements(self)
>+        if cycleInGraph:
>+            raise WebIDLError("Interface %s has itself as ancestor or "
>+                              "implemented interface" % self.identifier.name,
>+                              self.location,
>+                              extraLocation=cycleInGraph.location)

I'd prefer |if self.inheritsFromOrImplements(self)|, but either way is fine.

>@@ -603,16 +610,31 @@ class IDLInterface(IDLObjectWithScope):
> 
>         # And now collect up the consequential interfaces of all of those
>         temp = set()
>         for iface in consequentialInterfaces:
>             temp |= iface.getConsequentialInterfaces()
> 
>         return consequentialInterfaces | temp
> 
>+    def inheritsFromOrImplements(self, otherInterface):

Based on its name, it seems to me that this should return a bool.  Perhaps we should rename this method, or at least document the behavior.

>+        if self.parent:
>+            if self.parent is otherInterface:
>+                return self

Why |is| and not |==|?
Attachment #632893 - Flags: review?(justin.lebar+bug) → review+
> I'd prefer |if self.inheritsFromOrImplements(self)|

I can do that.

> Perhaps we should rename this method, or at least document the behavior.

Will try to find a better name, for sure.

> Why |is| and not |==|?

No reason.  Can do ==.
> I can do that.

Actually, no, since I use cycleInGraph.location in the error message.

I renamed to findInterfaceLoopPoint and added some docs.
> Actually, no, since I use cycleInGraph.location in the error message.

Ah, right!  Sounds like a plan.
https://hg.mozilla.org/integration/mozilla-inbound/rev/5de10f87c56e
Flags: in-testsuite+
Whiteboard: [need review]
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/5de10f87c56e
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.