Closed
Bug 1460126
Opened 6 years ago
Closed 6 years ago
Simplify TryEmitter member
Categories
(Core :: JavaScript Engine, enhancement, P3)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: arai, Assigned: arai)
References
Details
Attachments
(4 files)
13.25 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
13.31 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
5.87 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
4.80 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
similar to bug 1459845, TryEmitter can be simpified. * TryEmitter::ShouldUseRetVal and TryEmitter::ShouldUseControl can be merged * TryEmitter::State can become debug-only
Assignee | ||
Updated•6 years ago
|
Updated•6 years ago
|
Priority: -- → P3
Assignee | ||
Comment 1•6 years ago
|
||
simply changed enum to enum class.
Attachment #8979160 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 2•6 years ago
|
||
TryEmitter::ShouldUseRetVal and TryEmitter::ShouldUseControl are added to control the emitted bytecode, for syntactic try-catch and implicit internal try-catch. and apparently they can be merged (there's no other usage)
Attachment #8979161 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 3•6 years ago
|
||
the branch based on TryEmitter::State can be converted into TryEmitter.kind_, and now State is debug-only. (State type is going to be debug-only also in all other helper classes)
Attachment #8979162 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 4•6 years ago
|
||
Just added MOZ_MUST_USE
Attachment #8979163 -
Flags: review?(jwalden+bmo)
Updated•6 years ago
|
Attachment #8979160 -
Flags: review?(jwalden+bmo) → review+
Updated•6 years ago
|
Attachment #8979161 -
Flags: review?(jwalden+bmo) → review+
Comment 5•6 years ago
|
||
Comment on attachment 8979162 [details] [diff] [review] Part 3: Make TryEmitter::State debug only. Review of attachment 8979162 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/frontend/BytecodeEmitter.cpp @@ +1689,5 @@ > + , depth_(0) > + , noteIndex_(0) > + , tryStart_(0) > +#ifdef DEBUG > + , state_(State::Start) SpiderMonkey hasn't switched to this complete style for member-initializers, so keep everything up to noteIndex_ the same, remove the trailing comma from tryStart_, and then have the #ifdef DEBUG as you have it here.
Attachment #8979162 -
Flags: review?(jwalden+bmo) → review+
Updated•6 years ago
|
Attachment #8979163 -
Flags: review?(jwalden+bmo) → review+
Assignee | ||
Comment 6•6 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ad0b4d1b0f34ca4a8059b2f814be317ea5865d5 Bug 1460126 - Part 1: Make TryEmitter::Kind and TryEmitter::State enum classes. r=jwalden https://hg.mozilla.org/integration/mozilla-inbound/rev/e2aabb9c50162fc250cabbbfe9c6c69ecdede6b9 Bug 1460126 - Part 2: Merge TryEmitter::{ShouldUseRetVal,ShouldUseControl} into TryEmitter::ControlKind. r=jwalden https://hg.mozilla.org/integration/mozilla-inbound/rev/325ac4e5f005b00b3a9ec68b59cdb22180eb5725 Bug 1460126 - Part 3: Make TryEmitter::State debug only. r=jwalden https://hg.mozilla.org/integration/mozilla-inbound/rev/05084c58cd39f65c141e39d2d28528be29056e96 Bug 1460126 - Part 4: Add MOZ_MUST_USE to TryEmitter methods. r=jwalden
Comment 7•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0ad0b4d1b0f3 https://hg.mozilla.org/mozilla-central/rev/e2aabb9c5016 https://hg.mozilla.org/mozilla-central/rev/325ac4e5f005 https://hg.mozilla.org/mozilla-central/rev/05084c58cd39
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in
before you can comment on or make changes to this bug.
Description
•