nanojit: convert some error() tests to asserts

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
Created attachment 376860 [details] [diff] [review]
patch that converts some error() tests to asserts

In pageValidate() and resourceConsistencyCheck(), abort on prior error, rather than skipping.  This establishes an invariant -- prior error should be impossible due to the calling context.
Attachment #376860 - Flags: review?(edwsmith)

Comment 1

9 years ago
is this valid for all the possible ways error() can be set?  (out of memory, stackfull, etc?)
(Assignee)

Comment 2

9 years ago
It's valid.  Both functions only have one calling context, which looks like this:

        if (error())
            return;

        // check that all is well (don't check in exit paths since its more complicated)
        debug_only( pageValidate(); )
        debug_only( resourceConsistencyCheck();  )
(Assignee)

Comment 3

9 years ago
I should add the reason for this change.  It's because the use of the Assembler._err field, accessed through the error() and setError() methods, is very confusing, and this ties in with the difficulties of OOM handling.  Converting these two cases to assertions effectively removed two uses of error(), and so made a small step towards simplifying the error handling.

Comment 4

9 years ago
Comment on attachment 376860 [details] [diff] [review]
patch that converts some error() tests to asserts

cool
Attachment #376860 - Flags: review?(edwsmith) → review+

Comment 5

9 years ago
http://hg.mozilla.org/tracemonkey/rev/b76004a6fae3
Whiteboard: fixed-in-tracemonkey

Comment 6

9 years ago
http://hg.mozilla.org/mozilla-central/rev/b76004a6fae3
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.