1118978 - Add a more fine-grained mechanism for specifying alias sets in IDL

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

The plan is to add these two annotations:

[DependsOn=DOMState|DeviceState|Nothing].  If not specified, the default value is "Everything".

[Affects=Nothing].  If not specified, the default value is "Everything".

Then [Pure] desugars to [Affects=Nothing, DependsOn=DOMState] and [Constant] desugars to [Affects=Nothing, DependsOn=Nothing].

On the interaction-with-jit side, DependsOn=DeviceState means "not movable", but otherwise has the same alias set as DependsOn=Nothing.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Introduce "Affects" and "DependsOn" state for IDL attributes and operations and desugar [Pure] and [Constant] into that state

This does not change the generated binding code in any way for our existing IDL files.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Introduce [Affects] and [DependsOn] extended attributes in Web IDL

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Add support for DependsOn=DeviceState

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Mark performance.now() as being side-effect free but dependent on device state

Comment 5

[Peter Van der Beken [:peterv]]

Comment on attachment 8545591 [details] [diff] [review]
part 1.  Introduce "Affects" and "DependsOn" state for IDL attributes and operations and desugar [Pure] and [Constant] into that state

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

::: dom/bindings/Codegen.py
@@ +8215,5 @@
> +    def aliasSet(self):
> +        """
> +        Returns the alias set to store in the jitinfo.  This may not be the
> +        effective alias set the JIT uses, depending on whether we have enough
> +        information about our args that the JIT can prove effectful argument

Not sure if it'd be correct English, but s/that/so that/ or s/prove/prove that/ would be slightly clearer to me.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Rewrote as:

        depending on whether we have enough
        information about our args to allow the JIT to prove that effectful
        argument conversions won't happen

Comment 7

[Peter Van der Beken [:peterv]]

Comment on attachment 8545597 [details] [diff] [review]
part 4.  Mark performance.now() as being side-effect free but dependent on device state

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

::: dom/webidl/Performance.webidl
@@ +14,5 @@
>  typedef sequence <PerformanceEntry> PerformanceEntryList;
>  
>  [Exposed=(Window,Worker)]
>  interface Performance {
> +  [DependsOn=DeviceState,Affects=Nothing]

I'd put a space after comma.

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

These landed and broke b2g builds, so backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/5feaa9fcb009

https://treeherder.mozilla.org/logviewer.html#?job_id=5518001&repo=mozilla-inbound

Comment 9

[Boris Zbarsky [:bzbarsky]]

Ugh.  I was sure I'd done a try run on this.

The problem is that the IDL added in bug 1115465 is bogus and these patches added detection of that sort of bogosity.  :(

Comment 10

[Boris Zbarsky [:bzbarsky]]

   https://hg.mozilla.org/integration/mozilla-inbound/rev/ded8921cf389
   https://hg.mozilla.org/integration/mozilla-inbound/rev/c20c84c93efc
   https://hg.mozilla.org/integration/mozilla-inbound/rev/28487aeabd0a
   https://hg.mozilla.org/integration/mozilla-inbound/rev/8685f427b0bb

Comment 11

[Boris Zbarsky [:bzbarsky]]

Documented at:

https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings#DependsOn
https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings#Affects

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/ded8921cf389
https://hg.mozilla.org/mozilla-central/rev/c20c84c93efc
https://hg.mozilla.org/mozilla-central/rev/28487aeabd0a
https://hg.mozilla.org/mozilla-central/rev/8685f427b0bb