966181 - Rename ForkJoinSlice to ForkJoinContext.

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Shu-yu Guo [:shu]]

With the upcoming bug 958370, the name ForkJoinSlice no longer makes sense.
This patch renames it to ForkJoinContext.

Comment 1

[Shu-yu Guo [:shu]]

Attachment: Rename ForkJoinSlice to ForkJoinContext.

Comment 2

[Felix S. Klock II [:pnkfelix, :fklock]]

Comment on attachment 8368424 [details] [diff] [review]
Rename ForkJoinSlice to ForkJoinContext.

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

I think you missed the occurrences of the parameter named |slice| in the formal arguments to LNewPar, LNewCallObjectPar, LCheckOverRecursedPar, etc.

Or was there a reason you left them as |slice|, even though at their use-sites they are now passed a context?

Anyway, I have no problem with the *spirit* of the patch, especially since you did seem to take care to update the comments as well.  I'll r+ since I do trust that you can fix the other instances of the word slice.  (But if you want me to take another pass over the next patch, feel free to post it and r? me.)

Comment 3

[Felix S. Klock II [:pnkfelix, :fklock]]

(also the comment in MIRGraph.h still refers to a |slice| in its second line...)

I would have done a more through job of listing all these cases, but the provided patch did not apply cleanly to my current repo.  Again, if you want me to take another look at the next version, I'll try to start with a repo version where it applies cleanly so that I can grep more effectively for outstanding instances of "slice".

Comment 4

[Shu-yu Guo [:shu]]

(In reply to Felix S. Klock II [:pnkfelix, :fklock] from comment #2)
> Comment on attachment 8368424 [details] [diff] [review]
> Rename ForkJoinSlice to ForkJoinContext.
> 
> Review of attachment 8368424 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think you missed the occurrences of the parameter named |slice| in the
> formal arguments to LNewPar, LNewCallObjectPar, LCheckOverRecursedPar, etc.
> 
> Or was there a reason you left them as |slice|, even though at their
> use-sites they are now passed a context?
> 
> Anyway, I have no problem with the *spirit* of the patch, especially since
> you did seem to take care to update the comments as well.  I'll r+ since I
> do trust that you can fix the other instances of the word slice.  (But if
> you want me to take another pass over the next patch, feel free to post it
> and r? me.)

Nope, I just forgot. Thanks for the catching these cases.

Comment 5

[Shu-yu Guo [:shu]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/48a1442045d6

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/48a1442045d6