Closed Bug 1285710 Opened 5 years ago Closed 5 years ago

The JavaScript parser is misunderstanding the cause of error and showing inaccurate message in the console.

Categories

(Core :: JavaScript Engine, defect)

49 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: devashishjaiswal86, Assigned: arai)

References

Details

Attachments

(2 files)

Attached image Captursbvse.JPG
User Agent: Mozilla/5.0 (Windows NT 6.1; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID: 20160707004018

Steps to reproduce:

I wrote some code in JavaScript. I declared a variable 'aVar' and initialized it to a value. Following that, I declared a block. Note that the block is not part of any function. It is just a simple code enclosing block. I needed the block to limit the scope of a variables declared using 'let'. Then, just to play a bit, I tried to access a variable declared using 'let' before it is actually declared. If my English confuses, then following is the code:

var aVar = 56;
				
{
	console.log(aLet);
	let aLet = 56;
}




Actual results:

A ReferenceError was thrown as expected, but the error message in the console says:

"ReferenceError: can't access lexical declaration 'aVar' before initialization"

The error indicates that we tried to access 'aVar' without initializing it.

Another thing that should be noted is that if I declare another variable in top of my script, then the JS parser thinks that something is wrong with that variable. For example, If I do this:

var anotherVar = 20;
var aVar = 56;
				
{
	console.log(aLet);
	let aLet = 56;
}

then, the error message changes to:

"ReferenceError: can't access lexical declaration 'anotherVar' before initialization"



Expected results:

'aVar' is innocent. Everything is fine with it. So, the error message should show the name of the variable declared using 'let' that is inside the block. In my code example, it is 'aLet'. 

Note: I am using the latest version of Firefox Developer Edition. But the problem exists in earlier versions. Have a look at the comments here: http://stackoverflow.com/questions/38258719/is-it-a-bug-in-firefox-developer-edition#
Status: UNCONFIRMED → NEW
Component: Untriaged → JavaScript Engine
Ever confirmed: true
Product: Firefox → Core
https://dxr.mozilla.org/mozilla-central/rev/23dc78b7b57e9f91798ea44c242a04e112c37db0/js/src/vm/Interpreter.cpp#4917
>         // First search for a name among body-level lets.
>         for (BindingIter bi(script); bi; bi++) {
>             if (bi->kind() != Binding::ARGUMENT && !bi->aliased() && bi.frameIndex() == slot) {
>                 name = bi->name();
>                 break;
>             }
>         }

iiuc, this code shouldn't be executed for global.
This has to be something touched by the frontend rewrite, ami(w)rite.  Ergo, it depends on that happening first.
Depends on: 1263355
Confirmed this is fixed in today's m-c (presumably by bug 1263355).
It would be better adding the testcase.
So, it's already fixed.
Just added testcase.
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Attachment #8809694 - Flags: review?(shu)
Comment on attachment 8809694 [details] [diff] [review]
Add testcase for error message for redeclaration error.

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

::: js/src/jit-test/tests/parser/redeclaration-message.js
@@ +4,5 @@
> +    let aLet = 56;
> +} catch (e) {
> +    assertEq(e.message.includes("aLet"), true);
> +    assertEq(e.message.includes("aVar"), false);
> +}

I'm totally fine with checking this in as-is. Would be nice to also assert that an error was actually thrown.
Attachment #8809694 - Flags: review?(shu) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/e725cf00b32e304c50e4df8614cf01dfb49c095d
Bug 1285710 - Add testcase for error message for redeclaration error. r=shu
https://hg.mozilla.org/mozilla-central/rev/e725cf00b32e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.