Closed Bug 678201 Opened 13 years ago Closed 13 years ago

[jsdbg2] Use CallArgs throughout Debugger.cpp

Categories

(Core :: JavaScript Engine, defect)

Other Branch
x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jorendorff, Assigned: jorendorff)

Details

Attachments

(6 files)

Assignee: general → jorendorff
Attached patch part 1 - preliminary - v1 (deleted) — Splinter Review
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)
Attached patch part 2 - move - v1 (deleted) — Splinter Review
Move the newly specialized code just before its first use.
Attachment #552444 - Flags: review?(luke)
Attached patch part 3 - Debugger - v1 (deleted) — Splinter Review
Attachment #552446 - Flags: review?(luke)
Attached patch part 4 - Script - v1 (deleted) — Splinter Review
Attachment #552447 - Flags: review?(luke)
Attached patch part 5 - Frame - v1 (deleted) — Splinter Review
Attachment #552449 - Flags: review?(luke)
Attached patch part 6 - Object - v1 (deleted) — Splinter Review
That's it, 6 patches. It looks nicer this way.
Attachment #552487 - Flags: review?(luke)
Attachment #552442 - Flags: review?(luke) → review+
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 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+
Attachment #552447 - Flags: review?(luke) → review+
Attachment #552449 - Flags: review?(luke) → review+
Attachment #552487 - Flags: review?(luke) → review+
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.

Attachment

General

Created:
Updated:
Size: