remove WasmFunc.params_and_locals

Keeping it in sync w/ params and locals is a lot of work, and it doesn't
really help much anyway. Often we can just do whatever lookup we need to
do on one array, then the other.

8 files changed, 103 insertions, 117 deletions

diff --git a/src/wasm-ast-checker.c b/src/wasm-ast-checker.c
index 4d391b9c..cc12f4c6 100644
--- a/src/wasm-ast-checker.c
+++ b/src/wasm-ast-checker.c
@@ -219,16 +219,29 @@ static WasmResult check_local_var(WasmContext* ctx,
                                   const WasmFunc* func,
                                   const WasmVar* var,
                                   WasmType* out_type) {
-  int index;
-  if (WASM_FAILED(check_var(ctx, &func->params_and_locals.bindings,
-                func->params_and_locals.types.size, var, "local",
-                &index))) {
-    return WASM_ERROR;
+  int max_index = wasm_get_num_params_and_locals(func);
+  int index = wasm_get_local_index_by_var(func, var);
+  if (index >= 0 && index < max_index) {
+    if (out_type) {
+      int num_params = func->params.types.size;
+      if (index < num_params)  {
+        *out_type = func->params.types.data[index];
+      } else {
+        *out_type = func->locals.types.data[index - num_params];
+      }
+    }
+    return WASM_OK;
   }
 
-  if (out_type)
-    *out_type = func->params_and_locals.types.data[index];
-  return WASM_OK;
+  if (var->type == WASM_VAR_TYPE_NAME) {
+    print_error(ctx, &var->loc,
+                "undefined local variable \"" PRIstringslice "\"",
+                WASM_PRINTF_STRING_SLICE_ARG(var->name));
+  } else {
+    print_error(ctx, &var->loc, "local variable out of range (max %d)",
+                max_index);
+  }
+  return WASM_ERROR;
 }
 
 static void check_align(WasmContext* ctx,
diff --git a/src/wasm-ast-writer.c b/src/wasm-ast-writer.c
index ed492acc..72899217 100644
--- a/src/wasm-ast-writer.c
+++ b/src/wasm-ast-writer.c
@@ -614,11 +614,9 @@ static void write_exprs(WasmContext* ctx, const WasmExprPtrVector* exprs) {
 static void write_type_bindings(WasmContext* ctx,
                                 const char* prefix,
                                 const WasmFunc* func,
-                                const WasmTypeBindings* type_bindings,
-                                uint32_t index_offset) {
+                                const WasmTypeBindings* type_bindings) {
   CHECK_ALLOC(ctx, wasm_make_type_binding_reverse_mapping(
-                       ctx->allocator, type_bindings, index_offset,
-                       &ctx->index_to_name));
+                       ctx->allocator, type_bindings, &ctx->index_to_name));
 
   /* named params/locals must be specified by themselves, but nameless
    * params/locals can be compressed, e.g.:
@@ -664,7 +662,7 @@ static void write_func(WasmContext* ctx,
   }
   if (func->flags & WASM_FUNC_FLAG_HAS_SIGNATURE) {
     num_params = func->params.types.size;
-    write_type_bindings(ctx, "param", func, &func->params, 0);
+    write_type_bindings(ctx, "param", func, &func->params);
     if (func->result_type != WASM_TYPE_VOID) {
       out_open_space(ctx, "result");
       out_type(ctx, func->result_type, WASM_NEXT_CHAR_NONE);
@@ -673,7 +671,7 @@ static void write_func(WasmContext* ctx,
   }
   out_newline(ctx, NO_FORCE_NEWLINE);
   if (func->locals.types.size) {
-    write_type_bindings(ctx, "local", func, &func->locals, -num_params);
+    write_type_bindings(ctx, "local", func, &func->locals);
   }
   out_newline(ctx, NO_FORCE_NEWLINE);
   write_exprs(ctx, &func->exprs);
diff --git a/src/wasm-ast.c b/src/wasm-ast.c
index 604e9af6..917c4a06 100644
--- a/src/wasm-ast.c
+++ b/src/wasm-ast.c
@@ -200,7 +200,19 @@ int wasm_get_import_index_by_var(const WasmModule* module, const WasmVar* var) {
 }
 
 int wasm_get_local_index_by_var(const WasmFunc* func, const WasmVar* var) {
-  return wasm_get_index_from_var(&func->params_and_locals.bindings, var);
+  if (var->type == WASM_VAR_TYPE_INDEX)
+    return var->index;
+
+  int result = find_binding_index_by_name(&func->params.bindings, &var->name);
+  if (result != -1)
+    return result;
+  /* the locals start after all the params */
+  return func->params.types.size +
+         find_binding_index_by_name(&func->locals.bindings, &var->name);
+}
+
+size_t wasm_get_num_params_and_locals(const WasmFunc* func) {
+  return func->params.types.size + func->locals.types.size;
 }
 
 WasmFuncPtr wasm_get_func_by_var(const WasmModule* module, const WasmVar* var) {
@@ -253,7 +265,6 @@ WasmResult wasm_extend_type_bindings(WasmAllocator* allocator,
 WasmResult wasm_make_type_binding_reverse_mapping(
     struct WasmAllocator* allocator,
     const WasmTypeBindings* type_bindings,
-    uint32_t index_offset,
     WasmStringSliceVector* out_reverse_mapping) {
   uint32_t num_names = type_bindings->types.size;
   if (WASM_FAILED(wasm_reserve_string_slices(allocator, out_reverse_mapping,
@@ -271,7 +282,7 @@ WasmResult wasm_make_type_binding_reverse_mapping(
     if (wasm_hash_entry_is_free(entry))
       continue;
 
-    uint32_t index = entry->binding.index + index_offset;
+    uint32_t index = entry->binding.index;
     assert(index < out_reverse_mapping->size);
     out_reverse_mapping->data[index] = entry->binding.name;
   }
@@ -490,10 +501,6 @@ void wasm_destroy_func(WasmAllocator* allocator, WasmFunc* func) {
   wasm_destroy_var(allocator, &func->type_var);
   wasm_destroy_type_bindings(allocator, &func->params);
   wasm_destroy_type_bindings(allocator, &func->locals);
-  /* params_and_locals shares binding data with params and locals */
-  wasm_destroy_type_vector(allocator, &func->params_and_locals.types);
-  wasm_destroy_binding_hash_entry_vector(
-      allocator, &func->params_and_locals.bindings.entries);
   WASM_DESTROY_VECTOR_AND_ELEMENTS(allocator, func->exprs, expr_ptr);
 }
 
diff --git a/src/wasm-ast.h b/src/wasm-ast.h
index f822b357..7adf5e5c 100644
--- a/src/wasm-ast.h
+++ b/src/wasm-ast.h
@@ -208,10 +208,6 @@ typedef struct WasmFunc {
   WasmType result_type;
   WasmTypeBindings locals;
   WasmExprPtrVector exprs;
-
-  /* combined from params and locals; the binding names share memory with the
-   originals */
-  WasmTypeBindings params_and_locals;
 } WasmFunc;
 typedef WasmFunc* WasmFuncPtr;
 WASM_DEFINE_VECTOR(func_ptr, WasmFuncPtr);
@@ -472,6 +468,7 @@ int wasm_get_func_type_index_by_var(const WasmModule* module,
                                     const WasmVar* var);
 int wasm_get_import_index_by_var(const WasmModule* module, const WasmVar* var);
 int wasm_get_local_index_by_var(const WasmFunc* func, const WasmVar* var);
+size_t wasm_get_num_params_and_locals(const WasmFunc* func);
 
 WasmFuncPtr wasm_get_func_by_var(const WasmModule* module, const WasmVar* var);
 WasmFuncTypePtr wasm_get_func_type_by_var(const WasmModule* module,
@@ -486,8 +483,7 @@ WasmResult wasm_extend_type_bindings(struct WasmAllocator*,
                                      WasmTypeBindings* src) WASM_WARN_UNUSED;
 WasmResult wasm_make_type_binding_reverse_mapping(
     struct WasmAllocator*,
-    const WasmTypeBindings* bindings,
-    uint32_t index_offset,
+    const WasmTypeBindings*,
     WasmStringSliceVector* out_reverse_mapping);
 WASM_EXTERN_C_END
 
diff --git a/src/wasm-binary-reader-ast.c b/src/wasm-binary-reader-ast.c
index 685c2006..feeab288 100644
--- a/src/wasm-binary-reader-ast.c
+++ b/src/wasm-binary-reader-ast.c
@@ -56,7 +56,7 @@
   do {                                                                      \
     assert((ctx)->current_func->flags& WASM_FUNC_FLAG_HAS_FUNC_TYPE);       \
     uint32_t max_local_index =                                              \
-        get_num_func_params_and_locals((ctx)->module, (ctx)->current_func); \
+        wasm_get_num_params_and_locals((ctx)->current_func);                \
     if ((local_index) >= max_local_index) {                                 \
       print_error((ctx), "invalid local_index: %d (max %d)", (local_index), \
                   max_local_index);                                         \
@@ -107,22 +107,6 @@ static void print_error(WasmContext* ctx, const char* format, ...) {
   on_error(UNKNOWN_OFFSET, buffer, ctx);
 }
 
-static WasmFuncSignature* get_func_sig(WasmModule* module, WasmFunc* func) {
-  return &module->func_types.data[func->type_var.index]->sig;
-}
-
-static uint32_t get_num_func_params(WasmModule* module, WasmFunc* func) {
-  WasmFuncSignature* sig = get_func_sig(module, func);
-  return sig->param_types.size;
-}
-
-static uint32_t get_num_func_params_and_locals(WasmModule* module,
-                                               WasmFunc* func) {
-  uint32_t num_params = get_num_func_params(module, func);
-  uint32_t num_locals = func->locals.types.size;
-  return num_params + num_locals;
-}
-
 /* TODO(binji): remove all this if-block stuff when we switch to postorder */
 static WasmResult push_depth(WasmContext* ctx) {
   uint32_t* depth = wasm_append_uint32(ctx->allocator, &ctx->depth_stack);
@@ -319,11 +303,20 @@ static WasmResult on_function_signature(uint32_t index,
 
   WasmFunc* func = &field->func;
   WASM_ZERO_MEMORY(*func);
-  func->flags = WASM_FUNC_FLAG_HAS_FUNC_TYPE;
+  func->flags = WASM_FUNC_FLAG_HAS_FUNC_TYPE | WASM_FUNC_FLAG_HAS_SIGNATURE;
   func->type_var.type = WASM_VAR_TYPE_INDEX;
   assert(sig_index < ctx->module->func_types.size);
   func->type_var.index = sig_index;
 
+  /* copy the signature from the function type */
+  WasmFuncSignature* sig = &ctx->module->func_types.data[sig_index]->sig;
+  size_t i;
+  for (i = 0; i < sig->param_types.size; ++i) {
+    CHECK_ALLOC(ctx, wasm_append_type_value(ctx->allocator, &func->params.types,
+                                            &sig->param_types.data[i]));
+  }
+  func->result_type = sig->result_type;
+
   assert(index < ctx->module->funcs.capacity);
   WasmFuncPtr* func_ptr =
       wasm_append_func_ptr(ctx->allocator, &ctx->module->funcs);
@@ -1034,21 +1027,12 @@ static WasmResult on_local_names_count(uint32_t index,
   WasmModule* module = ctx->module;
   assert(index < module->funcs.size);
   WasmFunc* func = module->funcs.data[index];
-  uint32_t num_params_and_locals = get_num_func_params_and_locals(module, func);
+  uint32_t num_params_and_locals = wasm_get_num_params_and_locals(func);
   if (count > num_params_and_locals) {
     print_error(ctx, "expected local name count (%d) <= local count (%d)",
                 count, num_params_and_locals);
     return WASM_ERROR;
   }
-  /* copy the signature from the function type */
-  WasmFuncSignature* sig = get_func_sig(module, func);
-  size_t i;
-  for (i = 0; i < sig->param_types.size; ++i) {
-    CHECK_ALLOC(ctx, wasm_append_type_value(ctx->allocator, &func->params.types,
-                                            &sig->param_types.data[i]));
-  }
-  func->result_type = sig->result_type;
-  func->flags |= WASM_FUNC_FLAG_HAS_SIGNATURE;
   return WASM_OK;
 }
 
@@ -1059,22 +1043,25 @@ static WasmResult on_local_name(uint32_t func_index,
   WasmContext* ctx = user_data;
   WasmModule* module = ctx->module;
   WasmFunc* func = module->funcs.data[func_index];
-  uint32_t num_params = get_num_func_params(module, func);
+  uint32_t num_params = func->params.types.size;
   WasmStringSlice dup_name;
   CHECK_ALLOC_NULL_STR(ctx,
                        dup_name = wasm_dup_string_slice(ctx->allocator, name));
+  WasmBindingHash* bindings;
   WasmBinding* binding;
+  uint32_t index;
   if (local_index < num_params) {
     /* param name */
-    binding =
-        wasm_insert_binding(ctx->allocator, &func->params.bindings, &dup_name);
+    bindings = &func->params.bindings;
+    index = local_index;
   } else {
     /* local name */
-    binding =
-        wasm_insert_binding(ctx->allocator, &func->locals.bindings, &dup_name);
+    bindings = &func->locals.bindings;
+    index = local_index - num_params;
   }
+  binding = wasm_insert_binding(ctx->allocator, bindings, &dup_name);
   CHECK_ALLOC_NULL(ctx, binding);
-  binding->index = local_index;
+  binding->index = index;
   /* TODO(binji): We could update the get_local/set_local expressions to use
    * the name instead of the index */
   return WASM_OK;
diff --git a/src/wasm-binary-writer-spec.c b/src/wasm-binary-writer-spec.c
index b68e953b..003decc7 100644
--- a/src/wasm-binary-writer-spec.c
+++ b/src/wasm-binary-writer-spec.c
@@ -470,9 +470,6 @@ static void write_commands(WasmWriteSpecContext* ctx, WasmScript* script) {
           CHECK_ALLOC_NULL(caller);
           CHECK_ALLOC(wasm_append_type_value(
               script->allocator, &caller->locals.types, &result_type));
-          CHECK_ALLOC(wasm_append_type_value(script->allocator,
-                                             &caller->params_and_locals.types,
-                                             &result_type));
           expr_ptr = wasm_append_expr_ptr(script->allocator, &caller->exprs);
           CHECK_ALLOC_NULL(expr_ptr);
           *expr_ptr = create_set_local_expr(
diff --git a/src/wasm-binary-writer.c b/src/wasm-binary-writer.c
index 2958af7c..022b08de 100644
--- a/src/wasm-binary-writer.c
+++ b/src/wasm-binary-writer.c
@@ -84,10 +84,8 @@ typedef struct WasmContext {
 
   int* import_sig_indexes;
   int* func_sig_indexes;
-  uint32_t* remapped_locals; /* from unpacked -> packed index, has params */
-  uint32_t*
-      reverse_remapped_locals; /* from packed -> unpacked index, no params */
-  const WasmStringSlice** local_type_names; /* from packed -> local name */
+  uint32_t* remapped_locals;         /* from unpacked -> packed index */
+  uint32_t* reverse_remapped_locals; /* from packed -> unpacked index */
 } WasmContext;
 
 WASM_DEFINE_VECTOR(func_signature, WasmFuncSignature);
@@ -488,9 +486,9 @@ static void remap_locals(WasmContext* ctx, const WasmFunc* func) {
   if (num_params_and_locals)
     CHECK_ALLOC_NULL(ctx->remapped_locals);
 
-  ctx->reverse_remapped_locals =
-      wasm_realloc(ctx->allocator, ctx->reverse_remapped_locals,
-                   num_locals * sizeof(uint32_t), WASM_DEFAULT_ALIGN);
+  ctx->reverse_remapped_locals = wasm_realloc(
+      ctx->allocator, ctx->reverse_remapped_locals,
+      num_params_and_locals * sizeof(uint32_t), WASM_DEFAULT_ALIGN);
   if (num_locals)
     CHECK_ALLOC_NULL(ctx->reverse_remapped_locals);
 
@@ -498,7 +496,7 @@ static void remap_locals(WasmContext* ctx, const WasmFunc* func) {
     /* just pass the index straight through */
     for (i = 0; i < num_params_and_locals; ++i)
       ctx->remapped_locals[i] = i;
-    for (i = 0; i < num_locals; ++i)
+    for (i = 0; i < num_params_and_locals; ++i)
       ctx->reverse_remapped_locals[i] = i;
     return;
   }
@@ -511,8 +509,10 @@ static void remap_locals(WasmContext* ctx, const WasmFunc* func) {
   }
 
   /* params don't need remapping */
-  for (i = 0; i < num_params; ++i)
+  for (i = 0; i < num_params; ++i) {
     ctx->remapped_locals[i] = i;
+    ctx->reverse_remapped_locals[i] = i;
+  }
 
   uint32_t start[WASM_NUM_TYPES];
   start[WASM_TYPE_I32] = num_params;
@@ -527,7 +527,7 @@ static void remap_locals(WasmContext* ctx, const WasmFunc* func) {
     uint32_t unpacked_index = num_params + i;
     uint32_t packed_index = start[type] + seen[type]++;
     ctx->remapped_locals[unpacked_index] = packed_index;
-    ctx->reverse_remapped_locals[packed_index - num_params] = i;
+    ctx->reverse_remapped_locals[packed_index] = unpacked_index;
   }
 }
 
@@ -642,7 +642,6 @@ static void write_expr(WasmContext* ctx,
       break;
     case WASM_EXPR_TYPE_GET_LOCAL: {
       int index = wasm_get_local_index_by_var(func, &expr->get_local.var);
-      assert(index >= 0 && (size_t)index < func->params_and_locals.types.size);
       out_opcode(ctx, WASM_OPCODE_GET_LOCAL);
       out_u32_leb128(ctx, ctx->remapped_locals[index], "remapped local index");
       break;
@@ -796,10 +795,12 @@ static void write_func_locals(WasmContext* ctx,
     return;
   }
 
-/* remapped_locals includes the parameter list, so skip those */
-#define FIRST_LOCAL_INDEX (0)
-#define LAST_LOCAL_INDEX (local_types->size)
-#define GET_LOCAL_TYPE(x) (local_types->data[ctx->reverse_remapped_locals[x]])
+  uint32_t num_params = func->params.types.size;
+
+#define FIRST_LOCAL_INDEX (num_params)
+#define LAST_LOCAL_INDEX (num_params + local_types->size)
+#define GET_LOCAL_TYPE(x) \
+  (local_types->data[ctx->reverse_remapped_locals[x] - num_params])
 
   /* loop through once to count the number of local declaration runs */
   WasmType current_type = GET_LOCAL_TYPE(FIRST_LOCAL_INDEX);
@@ -972,54 +973,49 @@ static void write_module(WasmContext* ctx, const WasmModule* module) {
   }
 
   if (ctx->options->write_debug_names) {
+    WasmStringSliceVector index_to_name;
+    WASM_ZERO_MEMORY(index_to_name);
+
     char desc[100];
     begin_section(ctx, WASM_SECTION_NAME_NAMES, leb_size_guess);
     out_u32_leb128(ctx, module->funcs.size, "num functions");
     for (i = 0; i < module->funcs.size; ++i) {
       const WasmFunc* func = module->funcs.data[i];
+      uint32_t num_params = func->params.types.size;
+      uint32_t num_locals = func->locals.types.size;
+      uint32_t num_params_and_locals = wasm_get_num_params_and_locals(func);
+
       wasm_snprintf(desc, sizeof(desc), "func name %" PRIzd, i);
       out_str(ctx, func->name.start, func->name.length, WASM_PRINT_CHARS, desc);
-      out_u32_leb128(ctx, func->params_and_locals.types.size, "num locals");
+      out_u32_leb128(ctx, num_params_and_locals, "num locals");
 
-      if (func->params_and_locals.types.size) {
+      if (num_params_and_locals) {
         remap_locals(ctx, func);
 
-        /* create mapping from packed index to its name (if any) */
-        size_t local_type_names_size =
-            func->params_and_locals.types.size * sizeof(WasmStringSlice*);
-        ctx->local_type_names =
-            wasm_realloc(ctx->allocator, ctx->local_type_names,
-                         local_type_names_size, WASM_DEFAULT_ALIGN);
-        CHECK_ALLOC_NULL(ctx->local_type_names);
-        memset(ctx->local_type_names, 0, local_type_names_size);
-
+        CHECK_ALLOC(wasm_make_type_binding_reverse_mapping(
+            ctx->allocator, &func->params, &index_to_name));
         size_t j;
-        for (j = 0; j < func->params_and_locals.bindings.entries.capacity;
-             ++j) {
-          const WasmBindingHashEntry* entry =
-              &func->params_and_locals.bindings.entries.data[j];
-          if (wasm_hash_entry_is_free(entry))
-            continue;
-
-          ctx->local_type_names[ctx->remapped_locals[entry->binding.index]] =
-              &entry->binding.name;
+        for (j = 0; j < num_params; ++j) {
+          WasmStringSlice name = index_to_name.data[j];
+          wasm_snprintf(desc, sizeof(desc), "remapped local name %" PRIzd, j);
+          out_str(ctx, name.start, name.length, WASM_PRINT_CHARS, desc);
         }
 
-        for (j = 0; j < func->params_and_locals.types.size; ++j) {
-          WasmStringSlice name;
-          if (ctx->local_type_names[j]) {
-            name = *ctx->local_type_names[j];
-          } else {
-            name.start = "";
-            name.length = 0;
-          }
-
-          wasm_snprintf(desc, sizeof(desc), "remapped local name %" PRIzd, j);
+        CHECK_ALLOC(wasm_make_type_binding_reverse_mapping(
+            ctx->allocator, &func->locals, &index_to_name));
+        for (j = 0; j < num_locals; ++j) {
+          WasmStringSlice name =
+              index_to_name.data[ctx->reverse_remapped_locals[num_params + j] -
+                                 num_params];
+          wasm_snprintf(desc, sizeof(desc), "remapped local name %" PRIzd,
+                        num_params + j);
           out_str(ctx, name.start, name.length, WASM_PRINT_CHARS, desc);
         }
       }
     }
     end_section(ctx);
+
+    wasm_destroy_string_slice_vector(ctx->allocator, &index_to_name);
   }
   destroy_func_signature_vector_and_elements(ctx->allocator, &sigs);
 }
@@ -1049,7 +1045,6 @@ static void cleanup_context(WasmContext* ctx) {
   wasm_free(ctx->allocator, ctx->func_sig_indexes);
   wasm_free(ctx->allocator, ctx->remapped_locals);
   wasm_free(ctx->allocator, ctx->reverse_remapped_locals);
-  wasm_free(ctx->allocator, ctx->local_type_names);
 }
 
 WasmResult wasm_write_binary_module(WasmAllocator* allocator,
diff --git a/src/wasm-bison-parser.y b/src/wasm-bison-parser.y
index 5f5806b3..c8d7ba46 100644
--- a/src/wasm-bison-parser.y
+++ b/src/wasm-bison-parser.y
@@ -957,13 +957,6 @@ module :
                                           &func_type->sig.param_types));
           }
         }
-
-        /* now that func->params is set, we can easily create params_and_locals
-         * as well */
-        CHECK_ALLOC(wasm_extend_type_bindings(
-            parser->allocator, &func->params_and_locals, &func->params));
-        CHECK_ALLOC(wasm_extend_type_bindings(
-            parser->allocator, &func->params_and_locals, &func->locals));
       }
     }
 ;