Use NonNullObject in Debugger.cpp

RESOLVED FIXED in Firefox 42

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: jorendorff, Assigned: jorendorff)

Tracking

unspecified
mozilla42
Points:
---

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Calling NonNullObject() is shorter than what we have now.

Also:
-   inline NonNullObject's fast path
-   remove the ^L characters from Debugger.cpp (finally) in favor of
    double blank lines.
(In reply to Jason Orendorff [:jorendorff] from comment #0)
> -   remove the ^L characters from Debugger.cpp (finally) in favor of
>     double blank lines.

Whoa whoa whoa -- these are super useful for code navigation!
(Assignee)

Comment 2

3 years ago
More useful than searching for two blank lines in a row (\n\n\n)? Do you use the ^L support built into emacs?

I'd rather have navigation aids that don't look like some kind of ridiculous mistake to other hackers. They remove them; they don't add new ones when they add new sections...

This is properly a separate bug. though. I'll figure it out with whoever added the form feeds to gc/Marking.cpp and gc/Tracer.cpp.
(Assignee)

Comment 3

3 years ago
Created attachment 8627827 [details] [diff] [review]
Inline the common path of NonNullObject; use it instead of ReportObjectRequired in the Debugger
Attachment #8627827 - Flags: review?(shu)
(Assignee)

Updated

3 years ago
Assignee: nobody → jorendorff
Status: NEW → ASSIGNED

Comment 4

3 years ago
Comment on attachment 8627827 [details] [diff] [review]
Inline the common path of NonNullObject; use it instead of ReportObjectRequired in the Debugger

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

This is nice. Would like the message thing cleared up though.

::: addon-sdk/source/test/test-weak-set.js
@@ +117,5 @@
>  
>    assert.throws(() => {
>      add(items, 'foo');
>    },
> +  /^\w+ is not an object/,

Why the changes to these exception messages? I don't see any changes to messages in this patch.
Attachment #8627827 - Flags: review?(shu) → review+
(Assignee)

Comment 5

3 years ago
I reverted the changes in the addon sdk, as those had to do with changes to js.msg that aren't really necessary.

The current message is dumb, but I can fix it in a later bug.
(Assignee)

Comment 6

3 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e2824a3861dd
(Assignee)

Comment 7

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9e868f5f61c
https://hg.mozilla.org/mozilla-central/rev/f9e868f5f61c
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.