Remove dead code in BindVarOrConst




JavaScript Engine
6 years ago
4 years ago


(Reporter: ejpbruel, Unassigned)


Firefox Tracking Flags

(Not tracked)


(Whiteboard: [js:t])


(1 attachment)



6 years ago
Created attachment 642986 [details] [diff] [review]
Patch to be reviewed

I believe this code is redundant. stmt != NULL implies that an enclosing
statement has an associated scope object on which a binding for the given name
exists. However, when that binding was created, we also did a call to Define to
add the corresponding definition to tc->decls.

The only exception to this seems to be BindLet, which doesn't call Define until
after all bindings for the letBlock have been parsed (that is, until after we
have pushed the statement for the letBlock on the list of enclosing statements,
see letBlock for details). However, no variable bindings can occur during this
window, so for all intents and purposes, stmt != NULL here implies !defs.empty()

I've double checked this with an JS_ASSERT_IF(stmt, !defs.empty()). This doesn't
break any jit-tests. We should thus be able to change this condition to simply
!defs.empty(). Furthermore, we can get rid of lines 2137-2138 completely, which
misleadingly suggest that it is possible for defs.empty() to be true inside this
block (leading to me unnecessarily spending considerable time pondering how that
could be possible).
Attachment #642986 - Flags: review?(n.nethercote)


6 years ago
QA Contact: ejpbruel
Comment on attachment 642986 [details] [diff] [review]
Patch to be reviewed

Again, sorry, I'm not comfortable reviewing this :(  But it looks pretty plausible.  Again, use to get better test coverage.
Attachment #642986 - Flags: review?(n.nethercote)
Whiteboard: [js:t]


4 years ago
Assignee: general → nobody
You need to log in before you can comment on or make changes to this bug.