Bug 143038 - ES6: Classes: Program level class statement throws exception in strict mode
Summary: ES6: Classes: Program level class statement throws exception in strict mode
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joseph Pecoraro
Depends on:
Blocks: 140491
  Show dependency treegraph
Reported: 2015-03-24 22:04 PDT by Joseph Pecoraro
Modified: 2015-03-25 14:36 PDT (History)
4 users (show)

See Also:

[PATCH] Proposed Fix (32.67 KB, patch)
2015-03-25 09:35 PDT, Joseph Pecoraro
rniwa: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2015-03-24 22:04:54 PDT
Program level class expression throws exception in strict mode.

"use strict";
class MyClass { constructor() { } };

All is fine.

[Error] ReferenceError: Can't find variable: BaseNoReturn (foo.html, line 3)
Comment 1 Joseph Pecoraro 2015-03-24 22:05:31 PDT
Err, the exception should have had the name "MyClass" instead of "BaseNoReturn". Meant to update after pasting ;)
Comment 2 Joseph Pecoraro 2015-03-24 22:22:42 PDT
In parseStatement, parseFunctionDeclaration declares a declareVariable at this top level before pushing a new scope. I wonder if parseClassDeclaration should be doing the same, putting the class name at the top level.
Comment 3 Joseph Pecoraro 2015-03-24 22:27:35 PDT
My guess is that it should. When I compare to Firefox, a class statement requires a name, and the name is put into the global scope. I'll be back later to investigate this and look up the relevant parts of the spec. I'll take this bug.
Comment 4 Joseph Pecoraro 2015-03-25 00:52:18 PDT
Actually this seems to be accomplished a different way.

My next guess is that AST ScopeNodes have a VarStack and FunctionStack. BytecodeGenerator walks these and adds Function Declaration to the UnlinkedProgramCodeBlock. Seems promising.
Comment 5 Joseph Pecoraro 2015-03-25 02:00:11 PDT
Reading the spec more, this has to deal with Lexical Environments / Environment Records. I don't fully understand the terminology of this yet.

It looks like this is meant to be the same as `let` in scoping, but we could probably get away with this as an uninitialized `var` for now. (That seems to be what Firefox does with a basic test, which returned undefined, not a ReferenceError for use of a class name above the class definition).

The special handling for functions seems to be defined in B.3.3 "Block-Level Function Declarations Web Legacy Compatibility Semantics", which doesn't seem to apply to Classes.

My current understanding is that the class statement of "class Foo { ... }" can be considered syntactic sugar for "let Foo = class Foo { ... }".
Comment 6 Joseph Pecoraro 2015-03-25 09:35:07 PDT
Created attachment 249410 [details]
[PATCH] Proposed Fix
Comment 7 Ryosuke Niwa 2015-03-25 12:26:16 PDT
Comment on attachment 249410 [details]
[PATCH] Proposed Fix

Thanks for the fix!
Comment 8 Joseph Pecoraro 2015-03-25 13:41:57 PDT
I keep re-reading the runtime semantics of class declarations:

> 14.5.15 Runtime Semantics: BindingClassDeclarationEvaluation
> ClassDeclaration : class BindingIdentifier ClassTail
>     1. Let className be StringValue of BindingIdentifier.
>     2. Let value be the result of ClassDefinitionEvaluation of ClassTail with argument className.
>     3. ReturnIfAbrupt(value).
>     4. Let hasNameProperty be HasOwnProperty(value, "name").
>     5. ReturnIfAbrupt(hasNameProperty).
>     6. If hasNameProperty is false, then perform SetFunctionName(value, className).
>     7. Let env be the running execution context’s LexicalEnvironment.
>     8. Let status be InitializeBoundName(className, value, env).
>     9. ReturnIfAbrupt(status).
>     10. Return value.

So in here the InitializeBoundName -> InitializeBinding is what we are fixing.

However, I can't find anywhere that actually creates the mutable/immutable binding.

I'm trying to figure out for certain if class statements should be treated like `var` or `let`. Knowing where the binding was created would know the behavior of using the binding identifier before the evaluation of the class statement.
Comment 9 Joseph Pecoraro 2015-03-25 13:58:54 PDT
Geoff sorted this out for me!

The key is ClassDeclaration is a LexicalDeclaration and Global Declaration Initiation creates the Mutable/Immutable bindings for all LexicalDeclarations:  

So, the uninitialized binding will exist, and access to it before it is initialized should throw an error. So it is correct to leave that one test failing and I'll file a bug on that.
Comment 10 Joseph Pecoraro 2015-03-25 14:36:13 PDT
> The key is ClassDeclaration is a LexicalDeclaration and Global Declaration
> Initiation creates the Mutable/Immutable bindings for all
> LexicalDeclarations:

Well, not "all", but all the ones in its scope ;)
Comment 11 Joseph Pecoraro 2015-03-25 14:36:54 PDT