Closed
Bug 678201
Opened 13 years ago
Closed 13 years ago
[jsdbg2] Use CallArgs throughout Debugger.cpp
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jorendorff, Assigned: jorendorff)
Details
Attachments
(6 files)
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
Assignee | ||
Updated•13 years ago
|
Assignee: general → jorendorff
Assignee | ||
Comment 1•13 years ago
|
||
The CheckThis code is fake generality. It turns out only one class uses it. So I go ahead and specialize it.
Attachment #552442 -
Flags: review?(luke)
Assignee | ||
Comment 2•13 years ago
|
||
Move the newly specialized code just before its first use.
Attachment #552444 -
Flags: review?(luke)
Assignee | ||
Comment 3•13 years ago
|
||
Attachment #552446 -
Flags: review?(luke)
Assignee | ||
Comment 4•13 years ago
|
||
Attachment #552447 -
Flags: review?(luke)
Assignee | ||
Comment 5•13 years ago
|
||
Attachment #552449 -
Flags: review?(luke)
Assignee | ||
Comment 6•13 years ago
|
||
That's it, 6 patches. It looks nicer this way.
Attachment #552487 -
Flags: review?(luke)
Updated•13 years ago
|
Attachment #552442 -
Flags: review?(luke) → review+
Comment 7•13 years ago
|
||
Comment on attachment 552444 [details] [diff] [review]
part 2 - move - v1
Review of attachment 552444 [details] [diff] [review]:
-----------------------------------------------------------------
I admire your patch-fu. I should emulate this.
Attachment #552444 -
Flags: review?(luke) → review+
Comment 8•13 years ago
|
||
Comment on attachment 552446 [details] [diff] [review]
part 3 - Debugger - v1
Review of attachment 552446 [details] [diff] [review]:
-----------------------------------------------------------------
So far we have been writing "CallArgs args = ...". This is two chars more, but, as a nice side effect, "args[0]" reads nicely. Grepping, there also seems to be "CallArgs call = ..." in a few places... I should fix this; it would be nice to have a canonical name. r+ with s/ca/args/, or we can continue discussion.
Attachment #552446 -
Flags: review?(luke) → review+
Updated•13 years ago
|
Attachment #552447 -
Flags: review?(luke) → review+
Updated•13 years ago
|
Attachment #552449 -
Flags: review?(luke) → review+
Updated•13 years ago
|
Attachment #552487 -
Flags: review?(luke) → review+
Assignee | ||
Comment 9•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•