856819 - Can't declare a JS-implemented iface that inherits from another JS-implemented iface declared in the same webidl file if their order happens to be wrong

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Reuben Morais [:reuben]]

<bz> So we can fix this in the codegen
<bz> we have something like that already for dictionaries
<bz> I mean, it's not compiling because C++ cares about ordering
<bz> and wants to see superclass decls before subclasses.  ;)

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Refactor the dependency-sorting we do for dictionaries so that we can also use it for JS-implemented interfaces.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Sort js-implemented interfaces in a given file so that the declarations in C++ come in the right order.

Comment 3

[Andrew McCreight [:mccr8]]

Comment on attachment 732151 [details] [diff] [review]
part 1.  Refactor the dependency-sorting we do for dictionaries so that we can also use it for JS-implemented interfaces.

Review of attachment 732151 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bindings/Codegen.py
@@ +7138,5 @@
> +    dependencyGetter is something that, given an object, should return
> +    the set of objects it depends on.
> +    """
> +    # XXXbz this will fail if we have two webidl files F1 and F2 such that F1
> +    # declares an object which depends on an object in F2 and F2 declares an

Would you mind changing this to "in F2, and F2"?  My mind wants to put "F2 and F2" together. (That's a little accidental pun for you...)

Comment 4

[Andrew McCreight [:mccr8]]

Comment on attachment 732153 [details] [diff] [review]
part 2.  Sort js-implemented interfaces in a given file so that the declarations in C++ come in the right order.

Review of attachment 732153 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bindings/test/TestJSImplInheritanceGen.webidl
@@ +11,5 @@
>  [Constructor, JSImplementation="@mozilla.org/test-js-impl-interface3;1"]
>  interface TestJSImplInterface3 : TestCImplementedInterface2 {
>  };
>  
> +// Important: TestJSImplInterface5 needs to come before TestJSImplInterface6 to test what it's trying to test.

Maybe for these comments specify "before TestJSImplInterface6 in the file" and likewise for the other one?

Comment 5

[Boris Zbarsky [:bzbarsky]]

> Would you mind changing this to "in F2, and F2"? 

Done.

> Maybe for these comments specify "before TestJSImplInterface6 in the file"

Done.

Comment 6

[Boris Zbarsky [:bzbarsky]]

  https://hg.mozilla.org/integration/mozilla-inbound/rev/7e28ac04452a
  https://hg.mozilla.org/integration/mozilla-inbound/rev/3455bab4adc7

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7e28ac04452a
https://hg.mozilla.org/mozilla-central/rev/3455bab4adc7