Closed Bug 1118978 Opened 7 years ago Closed 7 years ago

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

Categories

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

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla38

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(4 files)

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.
This does not change the generated binding code in any way for our existing IDL files.
Attachment #8545591 - Flags: review?(peterv)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
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.
Attachment #8545591 - Flags: review?(peterv) → review+
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
Attachment #8545594 - Flags: review?(peterv) → review+
Attachment #8545596 - Flags: review?(peterv) → review+
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.
Attachment #8545597 - Flags: review?(peterv) → review+
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.  :(
Flags: needinfo?(bzbarsky)
Depends on: 1122357
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.