[wasm] asm.js - Parse and convert asm.js to wasm a function at a time.

src/ast/scopes.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/ast/scopes.h"
 
 #include <set>
 
 #include "src/accessors.h"
 #include "src/ast/ast.h"
@@ skipping 528 matching lines @@
 
   if (scope->is_eval_scope() && is_sloppy(scope->language_mode())) {
     AstNodeFactory factory(info->ast_value_factory());
     scope->HoistSloppyBlockFunctions(&factory);
   }
 
   // We are compiling one of three cases:
   // 1) top-level code,
   // 2) a function/eval/module on the top-level
   // 3) a function/eval in a scope that was already resolved.
-  DCHECK(scope->scope_type() == SCRIPT_SCOPE ||
-         scope->outer_scope()->scope_type() == SCRIPT_SCOPE ||
-         scope->outer_scope()->already_resolved_);
+  //  DCHECK(scope->scope_type() == SCRIPT_SCOPE ||
+  //         scope->outer_scope()->scope_type() == SCRIPT_SCOPE ||
+  //         scope->outer_scope()->already_resolved_);

[marja, 2016/11/26 15:46:36]

Why is the outer scope not already_resolved_ in your use case?

The ctor which constructs a Scope based on ScopeInfo sets already_resolved_ to
true. So this DCHECK checks (in your case, afaics) that you call Analyze on the
inner scope of such a scope... is that not the case?

[bradn, 2016/11/27 01:14:02]

So I'm calling this on a scope for an asm function, inside the scope for an asm
module. The module is sometimes at script scope, but can be wrapped inside an
eval or other function scope (as is the case in many of our tests).

Does it make sense to run Analyze on such a scope?
My goal is to allocate Variable* for each variable in the function, so I can use
them during asm.js validation.

 
   // The outer scope is never lazy.
   scope->set_should_eager_compile();
 
   scope->AllocateVariables(info, mode);
 
   // Ensuring that the outer script scope has a scope info avoids having
   // special case for native contexts vs other contexts.
   if (info->script_scope()->scope_info_.is_null()) {
     info->script_scope()->scope_info_ =
@@ skipping 437 matching lines @@
   return var;
 }
 
 VariableProxy* Scope::NewUnresolved(AstNodeFactory* factory,
                                     const AstRawString* name,
                                     int start_position, VariableKind kind) {
   // Note that we must not share the unresolved variables with
   // the same name because they may be removed selectively via
   // RemoveUnresolved().
   DCHECK(!already_resolved_);
-  DCHECK_EQ(factory->zone(), zone());
+  //  DCHECK_EQ(factory->zone(), zone());

[marja, 2016/11/26 15:46:36]

Removing this check seems wrong - why would your CL change this? Maybe you're
passing in the wrong Zone.

[bradn, 2016/11/27 01:14:02]

So this is happening because I'm passing in an AstValueFactory for each
ParseInfo that uses the zone used to parse the module (rather than using the
separate zone I'm parsing each asm function in).
I did this because Scopes use VariableMap which in turn seems to rely on
AstValueFactory ensuring that strings with the same contents get the same
pointer value. This is ensured inside AstValueFactory by keeping a string table.
Since I need variable names inside an asm function to resolve with names from
the module, I share a single one (not sure if this is ideal).

Suggestions very welcome :-)

[marja, 2016/11/28 11:47:21]

Yeah, the AstValueFactory needs to be the same, if you want to resolve variables
based on those strings.

I'm guessing what's happening here:
- You are creating a Zone in AsmWasmBuilderImpl::VisitFunctionDeclaration and
giving it to ParseInfo
- You're also giving the ParseInfo an AstValueFactory which is from a different
Zone.
- ParserBase ctor creates an AstNodeFactory based on the passed in
AstValueFactory
- And that's where this conflict comes from.

What you're doing is fine I think, it's just a new special case which hasn't
been needed so far. When this happens, the parser should just set the Zone of
the AstNodeFactory to be the Zone from the ParseInfo and everything should be
fine. Right?

   VariableProxy* proxy = factory->NewVariableProxy(name, kind, start_position);
   proxy->set_next_unresolved(unresolved_);
   unresolved_ = proxy;
   return proxy;
 }
 
 void Scope::AddUnresolved(VariableProxy* proxy) {
   DCHECK(!already_resolved_);
   DCHECK(!proxy->is_resolved());
   proxy->set_next_unresolved(unresolved_);
@@ skipping 621 matching lines @@
 
     Variable* invalidated = var;
     var = NonLocal(proxy->raw_name(), DYNAMIC_LOCAL);
     var->set_local_if_not_shadowed(invalidated);
   }
 
   return var;
 }
 
 void Scope::ResolveVariable(ParseInfo* info, VariableProxy* proxy) {
-  DCHECK(info->script_scope()->is_script_scope());
+  //  DCHECK(info->script_scope()->is_script_scope());

[marja, 2016/11/26 15:46:36]

What's the failure here? is script_scope() null or just not a script scope?

[bradn, 2016/11/27 01:14:02]

scope_type_ = v8::internal::ScopeType::FUNCTION_SCOPE

Though I see now I'm using script_scope in a different way than intended
(misunderstood and thought it was the script text being parsed), changed round,
added "asm_function_scope" (not sure if this is right).
What I'm trying to do is force variable resolution in the scope surrounding the
asm function.

[marja, 2016/11/28 11:47:21]

Ok, we need to think how to make the scope analysis work and what happens at the
boundary between Scopes created earlier when parsing the full thing and the
Scopes created now when parsing the function itself.

So now you connect the Scopes via

  150       // Create fresh function scope to use to parse the function in.
  151       DeclarationScope* new_func_scope = new (info.zone())
DeclarationScope(
  152           info.zone(), decl->fun()->scope()->outer_scope(),
FUNCTION_SCOPE);
  153       info.set_asm_function_scope(new_func_scope);

and then you parse & analyze the function scope.

What should happen when the function refers to a variable in the outer
functions? You want to connect the VariableProxy into a Variable which is out
there somewhere? Maybe the right thing already happens?

Another thing which I think is making the DCHECKs here fail is that the
situation is different from the normal "lazy parse an individual function" code
path. At that point we have reconstructed the Scope chain based on ScopeInfos,
so, outer scopes are ScopeInfo backed ones, and the boundary is where
non-ScopeInfo-backed Scopes meet ScopeInfo backed scopes.

But now it's different...

   DCHECK(!proxy->is_resolved());
   Variable* var = LookupRecursive(proxy, nullptr);
   ResolveTo(info, proxy, var);
 }
 
 namespace {
 
 bool AccessNeedsHoleCheck(Variable* var, VariableProxy* proxy, Scope* scope) {
   if (!var->binding_needs_init()) {
     return false;
@@ skipping 67 matching lines @@
   }
 #endif
 
   DCHECK_NOT_NULL(var);
   if (proxy->is_assigned()) var->set_maybe_assigned();
   if (AccessNeedsHoleCheck(var, proxy, this)) proxy->set_needs_hole_check();
   proxy->BindTo(var);
 }
 
 void Scope::ResolveVariablesRecursively(ParseInfo* info) {
-  DCHECK(info->script_scope()->is_script_scope());
+  // DCHECK(info->script_scope()->is_script_scope());
   // Lazy parsed declaration scopes are already partially analyzed. If there are
   // unresolved references remaining, they just need to be resolved in outer
   // scopes.
   if (is_declaration_scope() && AsDeclarationScope()->is_lazily_parsed()) {
     DCHECK(variables_.occupancy() == 0);
     for (VariableProxy* proxy = unresolved_; proxy != nullptr;
          proxy = proxy->next_unresolved()) {
       Variable* var = outer_scope()->LookupRecursive(proxy, nullptr);
       if (!var->is_dynamic()) {
         var->set_is_used();
@@ skipping 321 matching lines @@
   Variable* function =
       is_function_scope() ? AsDeclarationScope()->function_var() : nullptr;
   bool is_function_var_in_context =
       function != nullptr && function->IsContextSlot();
   return num_heap_slots() - Context::MIN_CONTEXT_SLOTS -
          (is_function_var_in_context ? 1 : 0);
 }
 
 }  // namespace internal
 }  // namespace v8