Closed Bug 1144630 Opened 5 years ago Closed 4 years ago

Methods defined in classes should not be enumerable

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: evilpie, Assigned: efaust)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

14.5.14 Step 21.a.i and 21.b.i pass `false` for the enumerable parameter of PropertyDefinitionEvaluation.
Flags: needinfo?(efaustbmo)
This is one verbose approach. Let's see what you think.
Assignee: nobody → efaustbmo
Status: NEW → ASSIGNED
Flags: needinfo?(efaustbmo)
Attachment #8675250 - Flags: review?(evilpies)
Comment on attachment 8675250 [details] [diff] [review]
Make class methods non-enumerable.

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

\o/ This was actually the last missing piece for 100% on kangax's ES6 compat table.

::: js/src/frontend/BytecodeEmitter.cpp
@@ +7124,5 @@
>              if (!makeAtomIndex(key->pn_atom, &index))
>                  return false;
>  
>              if (objp) {
> +                MOZ_ASSERT(!IsHiddenInitOp(op));

No idea why this is true.

::: js/src/jit/BaselineIC.cpp
@@ +3695,5 @@
>          oldCapacity = GetAnyBoxedOrUnboxedCapacity(obj);
>          oldInitLength = GetAnyBoxedOrUnboxedInitializedLength(obj);
>      }
>  
> +    if (op == JSOP_INITELEM || op == JSOP_INITHIDDENELEM) {

Right now this can't happen I think, but I assume the IC we emit doesn't care about JSOP_INITHIDDENELEM. So maybe we should return early?

::: js/src/vm/Interpreter-inl.h
@@ +776,1 @@
>  {

Please assert the op like InitArrayElemOperation below.
Attachment #8675250 - Flags: review?(evilpies) → review+
https://hg.mozilla.org/mozilla-central/rev/3fe3b12859ef
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
> Right now this can't happen I think, but I assume the IC we emit doesn't
> care about JSOP_INITHIDDENELEM. So maybe we should return early?
Didn't you mentiond on IRC that you would address this?
Flags: needinfo?(efaustbmo)
(In reply to Tom Schuster [:evilpie] from comment #5)
> > Right now this can't happen I think, but I assume the IC we emit doesn't
> > care about JSOP_INITHIDDENELEM. So maybe we should return early?
> Didn't you mentiond on IRC that you would address this?

Yes, thanks. Pushed above.
Flags: needinfo?(efaustbmo)
You need to log in before you can comment on or make changes to this bug.