Closed
Bug 966181
Opened 10 years ago
Closed 10 years ago
Rename ForkJoinSlice to ForkJoinContext.
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: shu, Assigned: shu)
References
Details
Attachments
(1 file)
179.08 KB,
patch
|
pnkfelix
:
review+
|
Details | Diff | Splinter Review |
With the upcoming bug 958370, the name ForkJoinSlice no longer makes sense. This patch renames it to ForkJoinContext.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8368424 -
Flags: review?(pnkfelix)
Comment 2•10 years ago
|
||
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.)
Attachment #8368424 -
Flags: review?(pnkfelix) → review+
Comment 3•10 years ago
|
||
(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".
Assignee | ||
Comment 4•10 years ago
|
||
(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.
Assignee | ||
Comment 5•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/48a1442045d6
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/48a1442045d6
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in
before you can comment on or make changes to this bug.
Description
•