c-writer.cc: cleanup handling of functions & tags (NFC)

- eliminate redundant import_syms_ SymbolSet
- eliminate ResultType struct
- replace ExternalPtr (only used for tags) with TagPtr
- refer to functions with bare name (no need for * or &)
- don't use "return" for functions returning void (fixes warning on MSVC)
- add missing "static" on ExportName + some missing "const"s
- fix spacing around init_instance_import

Author: keithw

src/c-writer.cc

@@ -94,8 +94,9 @@ struct GlobalName {
   const std::string& name;
 };
 
-struct ExternalPtr : GlobalName {
-  using GlobalName::GlobalName;
+struct TagPtr : GlobalName {
+  explicit TagPtr(const std::string& name)
+      : GlobalName(ModuleFieldType::Tag, name) {}
 };
 
 struct ExternalRef : GlobalName {
@@ -142,11 +143,6 @@ struct SignedType {
   Type type;
 };
 
-struct ResultType {
-  explicit ResultType(const TypeVector& types) : types(types) {}
-  const TypeVector& types;
-};
-
 struct TryCatchLabel {
   TryCatchLabel(const std::string& name, size_t try_catch_stack_size)
       : name(name), try_catch_stack_size(try_catch_stack_size), used(false) {}
@@ -267,17 +263,18 @@ class CWriter {
   static std::string Mangle(std::string_view name, bool double_underscores);
   static std::string MangleName(std::string_view);
   static std::string MangleModuleName(std::string_view);
-  std::string ExportName(std::string_view module_name,
-                         std::string_view export_name);
-  std::string ExportName(std::string_view export_name);
+  static std::string ExportName(std::string_view module_name,
+                                std::string_view export_name);
+  std::string ExportName(std::string_view export_name) const;
   std::string ModuleInstanceTypeName() const;
   static std::string ModuleInstanceTypeName(std::string_view module_name);
   void ClaimName(SymbolSet& set,
                  SymbolMap& map,
                  char type_suffix,
                  std::string_view wasm_name,
                  const std::string& c_name);
-  std::string FindUniqueName(SymbolSet& set, std::string_view proposed_name);
+  std::string FindUniqueName(SymbolSet& set,
+                             std::string_view proposed_name) const;
   std::string ClaimUniqueName(SymbolSet& set,
                               SymbolMap& map,
                               char type_suffix,
@@ -334,7 +331,7 @@ class CWriter {
   void Write(const ParamName&);
   void Write(const LabelName&);
   void Write(const GlobalName&);
-  void Write(const ExternalPtr&);
+  void Write(const TagPtr&);
   void Write(const ExternalRef&);
   void Write(const ExternalInstancePtr&);
   void Write(const ExternalInstanceRef&);
@@ -345,7 +342,7 @@ class CWriter {
   void Write(const LabelDecl&);
   void Write(const GlobalInstanceVar&);
   void Write(const StackVar&);
-  void Write(const ResultType&);
+  void Write(const TypeVector&);
   void Write(const Const&);
   void WriteInitExpr(const ExprList&);
   void WriteInitExprTerminal(const Expr*);
@@ -453,6 +450,8 @@ class CWriter {
 
   void PushFuncSection(std::string_view include_condition = "");
 
+  bool IsImport(const std::string& name) const;
+
   const WriteCOptions& options_;
   const Module* module_ = nullptr;
   const Func* func_ = nullptr;
@@ -473,7 +472,6 @@ class CWriter {
   StackVarSymbolMap stack_var_sym_map_;
   SymbolSet global_syms_;
   SymbolSet local_syms_;
-  SymbolSet import_syms_;
   TypeVector type_stack_;
   std::vector<Label> label_stack_;
   std::vector<TryCatchLabel> try_catch_stack_;
@@ -647,7 +645,7 @@ std::string CWriter::MangleTagTypes(const TypeVector& types) {
 }
 
 /* The C symbol for an export from this module. */
-std::string CWriter::ExportName(std::string_view export_name) {
+std::string CWriter::ExportName(std::string_view export_name) const {
   return kGlobalSymbolPrefix + module_prefix_ + '_' + MangleName(export_name);
 }
 
@@ -816,7 +814,7 @@ void CWriter::ClaimName(SymbolSet& set,
  * an integer to the symbol if necessary.
  */
 std::string CWriter::FindUniqueName(SymbolSet& set,
-                                    std::string_view proposed_name) {
+                                    std::string_view proposed_name) const {
   std::string unique{proposed_name};
   if (set.find(unique) != set.end()) {
     std::string base = unique + "_";
@@ -878,7 +876,6 @@ void CWriter::DefineImportName(const Import* import,
       break;
   }
 
-  import_syms_.insert(name);
   import_module_sym_map_.emplace(name, import->module_name);
 
   const std::string mangled = ExportName(module, field_name);
@@ -1052,34 +1049,30 @@ void CWriter::Write(const GlobalName& name) {
   Write(GetGlobalName(name.type, name.name));
 }
 
-void CWriter::Write(const ExternalPtr& name) {
-  bool is_import = import_syms_.count(name.name) != 0;
-  if (!is_import) {
+void CWriter::Write(const TagPtr& name) {
+  if (!IsImport(name.name)) {
     Write("&");
   }
   Write(GlobalName(name));
 }
 
 void CWriter::Write(const ExternalInstancePtr& name) {
-  bool is_import = import_syms_.count(name.name) != 0;
-  if (!is_import) {
+  if (!IsImport(name.name)) {
     Write("&");
   }
   Write("instance->", GlobalName(name));
 }
 
 void CWriter::Write(const ExternalRef& name) {
-  bool is_import = import_syms_.count(name.name) != 0;
-  if (is_import) {
-    Write("(*", GlobalName(name), ")");
-  } else {
+  if (name.type == ModuleFieldType::Func || !IsImport(name.name)) {
     Write(GlobalName(name));
+  } else {
+    Write("(*", GlobalName(name), ")");
   }
 }
 
 void CWriter::Write(const ExternalInstanceRef& name) {
-  bool is_import = import_syms_.count(name.name) != 0;
-  if (is_import) {
+  if (IsImport(name.name)) {
     Write("(*instance->", GlobalName(name), ")");
   } else {
     Write("instance->", GlobalName(name));
@@ -1197,13 +1190,13 @@ void CWriter::Write(SignedType type) {
   // clang-format on
 }
 
-void CWriter::Write(const ResultType& rt) {
-  if (rt.types.empty()) {
+void CWriter::Write(const TypeVector& types) {
+  if (types.empty()) {
     Write("void");
-  } else if (rt.types.size() == 1) {
-    Write(rt.types[0]);
+  } else if (types.size() == 1) {
+    Write(types[0]);
   } else {
-    Write("struct ", MangleMultivalueTypes(rt.types));
+    Write("struct ", MangleMultivalueTypes(types));
   }
 }
 
@@ -1384,17 +1377,15 @@ void CWriter::WriteInitExprTerminal(const Expr* expr) {
 
       Write("(wasm_rt_funcref_t){", FuncTypeExpr(func_type), ", ",
             "(wasm_rt_function_ptr_t)",
-            ExternalPtr(ModuleFieldType::Func, func->name), ", ");
+            ExternalRef(ModuleFieldType::Func, func->name), ", ");
 
-      bool is_import = import_module_sym_map_.count(func->name) != 0;
-      if (is_import) {
+      if (IsImport(func->name)) {
         Write("instance->", GlobalName(ModuleFieldType::Import,
                                        import_module_sym_map_[func->name]));
       } else {
         Write("instance");
       }
-
-      Write("};", Newline());
+      Write("}");
     } break;
 
     case ExprType::RefNull:
@@ -1790,7 +1781,7 @@ void CWriter::WriteFuncDeclarations() {
 
 void CWriter::WriteFuncDeclaration(const FuncDeclaration& decl,
                                    const std::string& name) {
-  Write(ResultType(decl.sig.result_types), " ", name, "(");
+  Write(decl.sig.result_types, " ", name, "(");
   Write(ModuleInstanceTypeName(), "*");
   WriteParamTypes(decl);
   Write(")");
@@ -1799,15 +1790,15 @@ void CWriter::WriteFuncDeclaration(const FuncDeclaration& decl,
 void CWriter::WriteImportFuncDeclaration(const FuncDeclaration& decl,
                                          const std::string& module_name,
                                          const std::string& name) {
-  Write(ResultType(decl.sig.result_types), " ", name, "(");
+  Write(decl.sig.result_types, " ", name, "(");
   Write("struct ", ModuleInstanceTypeName(module_name), "*");
   WriteParamTypes(decl);
   Write(")");
 }
 
 void CWriter::WriteCallIndirectFuncDeclaration(const FuncDeclaration& decl,
                                                const std::string& name) {
-  Write(ResultType(decl.sig.result_types), " ", name, "(void*");
+  Write(decl.sig.result_types, " ", name, "(void*");
   WriteParamTypes(decl);
   Write(")");
 }
@@ -2158,9 +2149,8 @@ void CWriter::WriteElemInitializers() {
           const Func* func = module_->GetFunc(cast<RefFuncExpr>(&expr)->var);
           const FuncType* func_type = module_->GetFuncType(func->decl.type_var);
           Write("{", FuncTypeExpr(func_type), ", (wasm_rt_function_ptr_t)",
-                ExternalPtr(ModuleFieldType::Func, func->name), ", ");
-          const bool is_import = import_module_sym_map_.count(func->name) != 0;
-          if (is_import) {
+                ExternalRef(ModuleFieldType::Func, func->name), ", ");
+          if (IsImport(func->name)) {
             Write("offsetof(", ModuleInstanceTypeName(), ", ",
                   GlobalName(ModuleFieldType::Import,
                              import_module_sym_map_[func->name]),
@@ -2290,8 +2280,7 @@ void CWriter::WriteExports(CWriterPhase kind) {
           local_syms_ = global_syms_;
           local_sym_map_.clear();
           stack_var_sym_map_.clear();
-          Write(ResultType(func_->decl.sig.result_types), " ", mangled_name,
-                "(");
+          Write(func_->decl.sig.result_types, " ", mangled_name, "(");
           MakeTypeBindingReverseMapping(func_->GetNumParamsAndLocals(),
                                         func_->bindings, &index_to_name);
           WriteParams(index_to_name);
@@ -2343,11 +2332,12 @@ void CWriter::WriteExports(CWriterPhase kind) {
     switch (export_->kind) {
       case ExternalKind::Func: {
         Write(OpenBrace());
-        Write("return ", ExternalRef(ModuleFieldType::Func, internal_name),
-              "(");
+        if (func_->GetNumResults() > 0) {
+          Write("return ");
+        }
+        Write(ExternalRef(ModuleFieldType::Func, internal_name), "(");
 
-        bool is_import = import_module_sym_map_.count(internal_name) != 0;
-        if (is_import) {
+        if (IsImport(internal_name)) {
           Write("instance->",
                 GlobalName(ModuleFieldType::Import,
                            import_module_sym_map_[internal_name]));
@@ -2388,8 +2378,7 @@ void CWriter::WriteExports(CWriterPhase kind) {
         break;
 
       case ExternalKind::Tag:
-        Write("= ", ExternalPtr(ModuleFieldType::Tag, internal_name), ";",
-              Newline());
+        Write("= ", TagPtr(internal_name), ";", Newline());
         break;
 
       default:
@@ -2435,9 +2424,7 @@ void CWriter::WriteInit() {
 
   for (Var* var : module_->starts) {
     Write(ExternalRef(ModuleFieldType::Func, module_->GetFunc(*var)->name));
-    bool is_import =
-        import_module_sym_map_.count(module_->GetFunc(*var)->name) != 0;
-    if (is_import) {
+    if (IsImport(module_->GetFunc(*var)->name)) {
       Write("(instance->",
             GlobalName(ModuleFieldType::Import,
                        import_module_sym_map_[module_->GetFunc(*var)->name]),
@@ -2494,7 +2481,7 @@ void CWriter::WriteInitInstanceImport() {
     Write(", struct ", ModuleInstanceTypeName(import_module_name), "* ",
           GlobalName(ModuleFieldType::Import, import_module_name));
   }
-  Write(")", OpenBrace());
+  Write(") ", OpenBrace());
 
   for (const auto& import_module : import_func_module_set_) {
     Write("instance->", GlobalName(ModuleFieldType::Import, import_module),
@@ -2623,6 +2610,10 @@ void CWriter::PushFuncSection(std::string_view include_condition) {
   stream_ = &func_sections_.back().second;
 }
 
+bool CWriter::IsImport(const std::string& name) const {
+  return import_module_sym_map_.count(name);
+}
+
 void CWriter::Write(const Func& func) {
   func_ = &func;
   local_syms_.clear();
@@ -2643,7 +2634,7 @@ void CWriter::Write(const Func& func) {
   Write(Newline());
 
   PushFuncSection();
-  Write(ResultType(func.decl.sig.result_types), " ",
+  Write(func.decl.sig.result_types, " ",
         GlobalName(ModuleFieldType::Func, func.name), "(");
   WriteParamsAndLocals();
   Write("FUNC_PROLOGUE;", Newline());
@@ -2666,7 +2657,7 @@ void CWriter::Write(const Func& func) {
     Write("return ", StackVar(0), ";", Newline());
   } else if (num_results >= 2) {
     Write(OpenBrace());
-    Write(ResultType(func.decl.sig.result_types), " tmp;", Newline());
+    Write(func.decl.sig.result_types, " tmp;", Newline());
     for (Index i = 0; i < num_results; ++i) {
       Type type = func.GetResultType(i);
       Writef("tmp.%c%d = ", MangleType(type), i);
@@ -2909,9 +2900,8 @@ void CWriter::Write(const Catch& c) {
     return;
   }
 
-  Write("if (wasm_rt_exception_tag() == ",
-        ExternalPtr(ModuleFieldType::Tag, module_->GetTag(c.var)->name), ") ",
-        OpenBrace());
+  Write("if (wasm_rt_exception_tag() == ", TagPtr(module_->GetTag(c.var)->name),
+        ") ", OpenBrace());
 
   const Tag* tag = module_->GetTag(c.var);
   const FuncDeclaration& tag_type = tag->decl;
@@ -3060,8 +3050,7 @@ void CWriter::Write(const ExprList& exprs) {
 
         assert(var.is_name());
         Write(ExternalRef(ModuleFieldType::Func, var.name()), "(");
-        bool is_import = import_module_sym_map_.count(func.name) != 0;
-        if (is_import) {
+        if (IsImport(func.name)) {
           Write("instance->", GlobalName(ModuleFieldType::Import,
                                          import_module_sym_map_[func.name]));
         } else {
@@ -3389,10 +3378,9 @@ void CWriter::Write(const ExprList& exprs) {
 
         Write(StackVar(0), " = (wasm_rt_funcref_t){", FuncTypeExpr(func_type),
               ", (wasm_rt_function_ptr_t)",
-              ExternalPtr(ModuleFieldType::Func, func->name), ", ");
+              ExternalRef(ModuleFieldType::Func, func->name), ", ");
 
-        bool is_import = import_module_sym_map_.count(func->name) != 0;
-        if (is_import) {
+        if (IsImport(func->name)) {
           Write("instance->", GlobalName(ModuleFieldType::Import,
                                          import_module_sym_map_[func->name]));
         } else {
@@ -3518,12 +3506,10 @@ void CWriter::Write(const ExprList& exprs) {
 
         Index num_params = tag->decl.GetNumParams();
         if (num_params == 0) {
-          Write("wasm_rt_load_exception(",
-                ExternalPtr(ModuleFieldType::Tag, tag->name), ", 0, NULL);",
+          Write("wasm_rt_load_exception(", TagPtr(tag->name), ", 0, NULL);",
                 Newline());
         } else if (num_params == 1) {
-          Write("wasm_rt_load_exception(",
-                ExternalPtr(ModuleFieldType::Tag, tag->name), ", sizeof(",
+          Write("wasm_rt_load_exception(", TagPtr(tag->name), ", sizeof(",
                 tag->decl.GetParamType(0), "), &", StackVar(0), ");",
                 Newline());
         } else {
@@ -3534,8 +3520,7 @@ void CWriter::Write(const ExprList& exprs) {
             Write(StackVar(i), ", ");
           }
           Write("};", Newline());
-          Write("wasm_rt_load_exception(",
-                ExternalPtr(ModuleFieldType::Tag, tag->name),
+          Write("wasm_rt_load_exception(", TagPtr(tag->name),
                 ", sizeof(tmp), &tmp);", Newline());
           Write(CloseBrace(), Newline());
         }

test/wasm2c/check-imports.txt

@@ -694,7 +694,7 @@ static void init_memories(w2c_test* instance) {
 }
 
 static const wasm_elem_segment_expr_t elem_segment_exprs_w2c_test_e0[] = {
-  {w2c_test_t1, (wasm_rt_function_ptr_t)&w2c_test_f1, 0},
+  {w2c_test_t1, (wasm_rt_function_ptr_t)w2c_test_f1, 0},
 };
 
 static void init_tables(w2c_test* instance) {
@@ -704,7 +704,7 @@ static void init_tables(w2c_test* instance) {
 static void init_elem_instances(w2c_test *instance) {
 }
 
-static void init_instance_import(w2c_test* instance, struct w2c_env* w2c_env_instance){
+static void init_instance_import(w2c_test* instance, struct w2c_env* w2c_env_instance) {
   instance->w2c_env_0x5F_indirect_function_table = w2c_env_0x5F_indirect_function_table(w2c_env_instance);
   instance->w2c_env_0x5F_linear_memory = w2c_env_0x5F_linear_memory(w2c_env_instance);
 }