Simplify TryEmitter member

RESOLVED FIXED in Firefox 62

Status

()

enhancement
P3
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: arai, Assigned: arai)

Tracking

(Blocks 1 bug)

Trunk
mozilla62
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox62 fixed)

Details

Attachments

(4 attachments)

Assignee

Description

a year ago
similar to bug 1459845, TryEmitter can be simpified.
  * TryEmitter::ShouldUseRetVal and TryEmitter::ShouldUseControl can be merged
  * TryEmitter::State can become debug-only
Priority: -- → P3
Assignee

Comment 1

a year ago
simply changed enum to enum class.
Attachment #8979160 - Flags: review?(jwalden+bmo)
Assignee

Comment 2

a year 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

a year 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

a year ago
Just added MOZ_MUST_USE
Attachment #8979163 - Flags: review?(jwalden+bmo)
Attachment #8979160 - Flags: review?(jwalden+bmo) → review+
Attachment #8979161 - Flags: review?(jwalden+bmo) → review+
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+
Attachment #8979163 - Flags: review?(jwalden+bmo) → review+
Assignee

Comment 6

a year 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
You need to log in before you can comment on or make changes to this bug.