Implement proper module linking.

Implement proper module linking.

Specifically:

- In parser, check that all exports are defined.
- Move JSModule allocation from parser to scope resolution.
- Move JSModule linking from full codegen to scope resolution.
- Implement module accessors for exported value members.
- Allocate module contexts statically along with JSModules
  (to allow static linking), but chain them when module literal is evaluated.
- Make module contexts' extension slot refer to resp. JSModule
  (makes modules' ScopeInfo accessible from context).
- Some other tweaks to context handling in general.
- Make any code containing module literals (and thus embedding
  static references to JSModules) non-cacheable.

This enables accessing module instance objects as expected.
Import declarations are a separate feature and do not work yet.

R=mstarzinger@chromium.org
BUG=v8:1569
TEST=

Committed: https://code.google.com/p/v8/source/detail?r=12010

[rossberg, 2012-06-29 15:10:36]

Sorry for the size of this CL, but it is difficult to break into meaningful
pieces.

[Michael Starzinger, 2012-07-06 10:53:22]

LGTM (with nits and comments). Just one real comment at the end of scopes.cc,
the rest are mainly nits.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/accessors.cc
File src/accessors.cc (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/accessors.cc#newco...
src/accessors.cc:855: Factory* factory = Isolate::Current()->factory();
Use name->GetIsolate() instead.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/accessors.h
File src/accessors.h (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/accessors.h#newcode89
src/accessors.h:89: static Handle<AccessorInfo> MakeModuleExport(
Let's name that "MakeModuleExportAccessor()"

https://chromiumcodereview.appspot.com/10690043/diff/1/src/accessors.h#newcode90
src/accessors.h:90: Handle<String> property, int index, PropertyAttributes
attributes);
Rename "property" to "name" here.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/ast.cc
File src/ast.cc (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/ast.cc#newcode1083
src/ast.cc:1083: DONT_CACHE_NODE(ModuleLiteral)
Can we move that out into a separate group instead of interleaving it with the
non-optimized nodes.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/ast.cc#newcode1119
src/ast.cc:1119: #undef DONT_SELFOPTIMIZE_NODE
Also #undef DONT_CACHE_NODE here.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/ast.h
File src/ast.h (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/ast.h#newcode590
src/ast.h:590: ExportDeclaration(VariableProxy* proxy, Scope* scope) :
The colon should be on the next line.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/ast.h#newcode598
src/ast.h:598: 
For consistency we should drop this empty newline.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/ast.h#newcode602
src/ast.h:602: explicit Module(Zone* zone) :
The colon should be on the next line.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/ast.h#newcode605
src/ast.h:605: explicit Module(Interface* interface, Block* body = NULL) :
The colon should be on the next line.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/contexts.h
File src/contexts.h (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/contexts.h#newcode434
src/contexts.h:434: bool IsBootstrappingOrValidContext(Object* object);
Can we make these two debug methods static again and pass in both contexts as
arguments. Because currently it's not intuitive what the difference between
"this" and "object" actually is.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/heap.cc
File src/heap.cc (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/heap.cc#newcode4928
src/heap.cc:4928: context->set_previous(NULL);
I would prefer Smi::FromInt(0) instead of NULL here.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/messages.js
File src/messages.js (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/messages.js#newcod...
src/messages.js:259: "export_undefined",             ["Export '", "%0", "' is
not defined in module"],
Should we maybe name this "module_export_undefined"?

https://chromiumcodereview.appspot.com/10690043/diff/1/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/objects.h#newcode5861
src/objects.h:5861: // [scope_info]: Scope info.
Empty newline between accessors.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/scopes.cc
File src/scopes.cc (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/scopes.cc#newcode646
src/scopes.cc:646: if (is_global_scope() || is_module_scope()) {
Can we merge that into one condition?

https://chromiumcodereview.appspot.com/10690043/diff/1/src/scopes.cc#newcode1377
src/scopes.cc:1377: USE(instance->PreventExtensions());
This might request a GC, so you should use JSObject::PreventExtensions(instance)
here.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/scopes.h
File src/scopes.h (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/scopes.h#newcode599
src/scopes.h:599: // may potentially requires (forward) references to others.
Drop one "may" and "s/requires/require".

https://chromiumcodereview.appspot.com/10690043/diff/1/test/mjsunit/harmony/m...
File test/mjsunit/harmony/module-recompile.js (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/test/mjsunit/harmony/m...
test/mjsunit/harmony/module-recompile.js:81: if (i > N) print("impossible");
I am not sure if this will force recompilation. Unless you made sure that
top-level code was deoptimized after the last optimization loop.

[rossberg, 2012-07-06 15:39:28]

https://chromiumcodereview.appspot.com/10690043/diff/1/src/accessors.cc
File src/accessors.cc (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/accessors.cc#newco...
src/accessors.cc:855: Factory* factory = Isolate::Current()->factory();
On 2012/07/06 10:53:22, Michael Starzinger wrote:
> Use name->GetIsolate() instead.

Done.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/accessors.h
File src/accessors.h (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/accessors.h#newcode89
src/accessors.h:89: static Handle<AccessorInfo> MakeModuleExport(
On 2012/07/06 10:53:22, Michael Starzinger wrote:
> Let's name that "MakeModuleExportAccessor()"

Hm, I'm following the naming convention of the existing accessors here. This one
is a factory for a "ModuleExport" accessor, thus the name. (Note that the class
is already named Accessors, so it's implied.)

https://chromiumcodereview.appspot.com/10690043/diff/1/src/accessors.h#newcode90
src/accessors.h:90: Handle<String> property, int index, PropertyAttributes
attributes);
On 2012/07/06 10:53:22, Michael Starzinger wrote:
> Rename "property" to "name" here.

Done.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/ast.cc
File src/ast.cc (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/ast.cc#newcode1083
src/ast.cc:1083: DONT_CACHE_NODE(ModuleLiteral)
On 2012/07/06 10:53:22, Michael Starzinger wrote:
> Can we move that out into a separate group instead of interleaving it with the
> non-optimized nodes.

Done.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/ast.cc#newcode1119
src/ast.cc:1119: #undef DONT_SELFOPTIMIZE_NODE
On 2012/07/06 10:53:22, Michael Starzinger wrote:
> Also #undef DONT_CACHE_NODE here.

Done.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/ast.h
File src/ast.h (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/ast.h#newcode590
src/ast.h:590: ExportDeclaration(VariableProxy* proxy, Scope* scope) :
On 2012/07/06 10:53:22, Michael Starzinger wrote:
> The colon should be on the next line.

Done.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/ast.h#newcode598
src/ast.h:598: 
On 2012/07/06 10:53:22, Michael Starzinger wrote:
> For consistency we should drop this empty newline.

Done.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/ast.h#newcode602
src/ast.h:602: explicit Module(Zone* zone) :
On 2012/07/06 10:53:22, Michael Starzinger wrote:
> The colon should be on the next line.

Done.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/ast.h#newcode605
src/ast.h:605: explicit Module(Interface* interface, Block* body = NULL) :
On 2012/07/06 10:53:22, Michael Starzinger wrote:
> The colon should be on the next line.

Done.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/contexts.h
File src/contexts.h (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/contexts.h#newcode434
src/contexts.h:434: bool IsBootstrappingOrValidContext(Object* object);
On 2012/07/06 10:53:22, Michael Starzinger wrote:
> Can we make these two debug methods static again and pass in both contexts as
> arguments. Because currently it's not intuitive what the difference between
> "this" and "object" actually is.

Done.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/heap.cc
File src/heap.cc (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/heap.cc#newcode4928
src/heap.cc:4928: context->set_previous(NULL);
On 2012/07/06 10:53:22, Michael Starzinger wrote:
> I would prefer Smi::FromInt(0) instead of NULL here.

Done (also in old code below).

https://chromiumcodereview.appspot.com/10690043/diff/1/src/messages.js
File src/messages.js (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/messages.js#newcod...
src/messages.js:259: "export_undefined",             ["Export '", "%0", "' is
not defined in module"],
On 2012/07/06 10:53:22, Michael Starzinger wrote:
> Should we maybe name this "module_export_undefined"?

Done.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/objects.h
File src/objects.h (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/objects.h#newcode5861
src/objects.h:5861: // [scope_info]: Scope info.
On 2012/07/06 10:53:22, Michael Starzinger wrote:
> Empty newline between accessors.

Done.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/scopes.cc
File src/scopes.cc (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/scopes.cc#newcode646
src/scopes.cc:646: if (is_global_scope() || is_module_scope()) {
On 2012/07/06 10:53:22, Michael Starzinger wrote:
> Can we merge that into one condition?

Done.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/scopes.cc#newcode1377
src/scopes.cc:1377: USE(instance->PreventExtensions());
On 2012/07/06 10:53:22, Michael Starzinger wrote:
> This might request a GC, so you should use
JSObject::PreventExtensions(instance)
> here.

Done.

https://chromiumcodereview.appspot.com/10690043/diff/1/src/scopes.h
File src/scopes.h (right):

https://chromiumcodereview.appspot.com/10690043/diff/1/src/scopes.h#newcode599
src/scopes.h:599: // may potentially requires (forward) references to others.
On 2012/07/06 10:53:22, Michael Starzinger wrote:
> Drop one "may" and "s/requires/require".

Done.

[Jakob Kummerow, 2012-07-09 11:03:12]

This breaks compilation on newer GCCs:

../src/scopes.cc: In member function 'void
v8::internal::Scope::LinkModules(v8::internal::CompilationInfo*)':
../src/scopes.cc:1379:24: error: variable 'result' set but not used
[-Werror=unused-but-set-variable]

../src/full-codegen.cc: In member function 'virtual void
v8::internal::FullCodeGenerator::VisitModuleLiteral(v8::internal::ModuleLiteral*)':
../src/full-codegen.cc:596:21: error: variable 'scope_info' set but not used
[-Werror=unused-but-set-variable]

https://chromiumcodereview.appspot.com/10690043/diff/4004/src/full-codegen.cc
File src/full-codegen.cc (right):

https://chromiumcodereview.appspot.com/10690043/diff/4004/src/full-codegen.cc...
src/full-codegen.cc:596: Handle<ScopeInfo> scope_info = scope_->GetScopeInfo();
unused variable.

https://chromiumcodereview.appspot.com/10690043/diff/4004/src/scopes.cc
File src/scopes.cc (right):

https://chromiumcodereview.appspot.com/10690043/diff/4004/src/scopes.cc#newco...
src/scopes.cc:1371: Handle<Object> result = SetAccessor(instance, info);
unused variable (in release mode).