chakra-core / ChakraCore

ChakraCore is an open source Javascript engine with a C API.
MIT License
9.1k stars 1.19k forks source link

Inconsistent output compared with other JS engines #5511

Open sunlili opened 6 years ago

sunlili commented 6 years ago

Hello, The following code behaves strangely (inconsistent with other engines).

function test() {
    x = 4;
    do {
    var ss = 100;
    function ss() {
        print("in func");
    }
    x--;
    ss = 101; }
    while(x>0)
}

test();
print("BT_FLAG")

In ChakraCore, the output is BT_FLAG However, in V8 and SpiderMonkey, output is SyntaxError: redeclaration of var ss:

BT group 2018.7.24

rhuanjl commented 4 years ago

I'm unsure what behaviour is correct per spec here - JSC and CC both run to completion whilst spidermonkey and v8 error.

BUT the error is a redeclaration - seemingly of a function overwriting a var? And if you manually pull the function definition ss upto the top of test - where it should assumedly be hoisted anyway - the error goes away in v8 and spidermonkey. Is the error here in the other engines?

rhuanjl commented 4 years ago

@chicoxyzzy do you know what the answer should be per spec here? (I don't want to "fix" it if we're already right)

chicoxyzzy commented 4 years ago

I can't check right now, but it seems that this is caused by ChakraCore and JSC not implementing Annex B 3.3 properly.

Minimal example to reproduce:

{
  var foo;
  function foo() {}
}
rhuanjl commented 4 years ago

OK so per Annex B 3.3.4

It is a Syntax Error if the LexicallyDeclaredNames of StatementList contains any duplicate entries, unless the source code matching this production is not strict mode code and the duplicate entries are only bound by FunctionDeclarations.

That seems to be this case - I can make this throw if we want to implement 3.3.4 - it seems we don't at the moment.

pleath commented 4 years ago

The rule we implemented was an early version of the lexical-scoping spec. We treated hoisted declarations as not colliding with block-scoped ones. The real fix for this is actually kind of non-trivial. Consider this:

{ { var foo; } function foo(){}; }

We need to do some work at binding time to detect redeclaration in all the scopes that enclose the one from which a declaration is being hoisted.

-- Paul

From: Richard notifications@github.com Sent: Monday, April 13, 2020 7:50 AM To: microsoft/ChakraCore ChakraCore@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [microsoft/ChakraCore] Inconsistent output compared with other JS engines (#5511)

OK so per Annex B 3.3.4

It is a Syntax Error if the LexicallyDeclaredNames of StatementList contains any duplicate entries, unless the source code matching this production is not strict mode code and the duplicate entries are only bound by FunctionDeclarations.

That seems to be this case - I can make this throw if we want to implement 3.3.4 - it seems we don't at the moment.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2FChakraCore%2Fissues%2F5511%23issuecomment-612930902&data=02%7C01%7Cpleath%40microsoft.com%7C01a142d0fc8e471e750808d7dfb9f3c1%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637223862067379899&sdata=A3W3Amoq7jkSDDnJV3kSIN9SSUjHqJDjpAK5b1zolcE%3D&reserved=0, or unsubscribehttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FADYXYRB2QDEHSYRI23XNELLRMMRBZANCNFSM4FLTEGDA&data=02%7C01%7Cpleath%40microsoft.com%7C01a142d0fc8e471e750808d7dfb9f3c1%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637223862067379899&sdata=dRUrHIKiMUhQxJQ0jgb3LhuYbiyw2K9nWOU2ZDCcU2Q%3D&reserved=0.