774702 - Remove dead code in BindVarOrConst

Status: NEW

(Core :: JavaScript Engine, defect)

Description

[Eddy Bruel [:ejpbruel]]

Attachment: Patch to be reviewed

http://mxr.mozilla.org/mozilla-central/source/js/src/frontend/Parser.cpp#2228

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).

Comment 1

[Nicholas Nethercote [inactive]]

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 jstests.py to get better test coverage.