764456 - Need to ensure that the RHS of "implements" (or any other consequential interface) is not flagged as castable if used as an argument

Status: RESOLVED FIXED

Description

[Boris Zbarsky [:bzbarsky, bz on IRC]]

In particular, we need a few changes here:

1)  Consequential interfaces need to be flagged as such.
2)  Codegen should assert if a consequential interface is used as an argument and
    is marked as castable.
3)  This-unwrapping should stop asserting that the interface being unwrapped to is
    castable, since it's possible for a concrete interface to be the RHS of
    implements and hence to require non-castability.
4)  Make a clear differentiation between our various interface types.  This will
    somewhat lay the groundwork for bug 742164.

Comment 1

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Created attachment 632873 [details] [diff] [review]
part 1.  Flag consequential interfaces in the WebIDL data model.

Comment 2

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Created attachment 632874 [details] [diff] [review]
part 2.  Add some tests for callback interfaces.

Comment 3

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Created attachment 632875 [details] [diff] [review]
part 3.  Clean up meaning of 'castable' and use thereof a bit.

Comment 4

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Created attachment 632876 [details] [diff] [review]
part 4.  Make sure consequential interface used as arguments are non-castable.

Comment 5

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Created attachment 632877 [details] [diff] [review]
part 5.  Add a way to flag interfaces as not needing a prototype object so we won't do useless codegen for them.

Comment 6

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Created attachment 634533 [details] [diff] [review]
part 5.  Actually look at hasConcreteDescendant in hasInterfacePrototypeObject() so that we can avoid codegen for [NoInterfaceObject] interfaces hat are only implemented via "implements".

Comment 7

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Part 1 landed as https://hg.mozilla.org/integration/mozilla-inbound/rev/25a6e906d51b because other patches depended on it.  Bug remains open for parts 2-4.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/25a6e906d51b

Comment 9

[Peter Van der Beken [:peterv]]

Comment on attachment 632875 [details] [diff] [review]
part 3.  Clean up meaning of 'castable' and use thereof a bit.

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

Remove castable from addExternalIface.

Comment 10

[Peter Van der Beken [:peterv]]

Comment on attachment 632876 [details] [diff] [review]
part 4.  Make sure consequential interface used as arguments are non-castable.

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

::: dom/bindings/Codegen.py
@@ +2806,5 @@
> +    def __init__(self, descriptor):
> +        self.castable = True
> +        self.workers = descriptor.workers
> +        self.nativeType = descriptor.nativeType
> +        self.name = descriptor.name

There are ways to have proxies in python I believe, but I guess that's a bit much for this one use case.

Comment 11

[Boris Zbarsky [:bzbarsky, bz on IRC]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cec6ea1283ad
https://hg.mozilla.org/integration/mozilla-inbound/rev/8498c31fb92e
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9f685ea9ed0
https://hg.mozilla.org/integration/mozilla-inbound/rev/298c8e888da5

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/cec6ea1283ad
https://hg.mozilla.org/mozilla-central/rev/8498c31fb92e
https://hg.mozilla.org/mozilla-central/rev/f9f685ea9ed0
https://hg.mozilla.org/mozilla-central/rev/298c8e888da5