refactor: Instead of optional for table elements, check instance is set

lib/fizzy/execute.cpp

@@ -621,18 +621,18 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args,
                 goto trap;
 
             const auto called_func = (*instance.table)[elem_idx];
-            if (!called_func.has_value())
+            if (!called_func.instance)  // Table element not initialized.
                 goto trap;
 
             // check actual type against expected type
             const auto& actual_type =
-                called_func->instance->module->get_function_type(called_func->func_idx);
+                called_func.instance->module->get_function_type(called_func.func_idx);
             const auto& expected_type = instance.module->typesec[expected_type_idx];
             if (expected_type != actual_type)
                 goto trap;
 
             if (!invoke_function(
-                    actual_type, called_func->func_idx, *called_func->instance, stack, depth))
+                    actual_type, called_func.func_idx, *called_func.instance, stack, depth))
                 goto trap;
             break;
         }

lib/fizzy/instantiate.cpp

@@ -357,7 +357,7 @@ std::unique_ptr<Instance> instantiate(std::unique_ptr<const Module> module,
                     for ([[maybe_unused]] auto _ : shared_instance->module->elementsec[i].init)
                     {
                         // Capture shared instance in table element.
-                        (*it_table++)->shared_instance = shared_instance;
+                        (*it_table++).shared_instance = shared_instance;
                     }
                 }
             }

lib/fizzy/instantiate.hpp

@@ -31,15 +31,17 @@ struct ExternalFunction
 // Table element, which references a function in any instance.
 struct TableElement
 {
-    Instance* instance;
-    FuncIdx func_idx;
+    // Pointer to function's instance or nullptr when table element is not initialized.
+    Instance* instance = nullptr;
+    // Index of the function in instance.
+    FuncIdx func_idx = 0;
     // This pointer is empty most of the time and is used only to keep instance alive in one edge
     // case, when start function traps, but instantiate has already modified some elements of a
     // shared (imported) table.
     std::shared_ptr<Instance> shared_instance;
 };
 
-using table_elements = std::vector<std::optional<TableElement>>;
+using table_elements = std::vector<TableElement>;
 using table_ptr = std::unique_ptr<table_elements, void (*)(table_elements*)>;
 
 struct ExternalTable

test/unittests/api_test.cpp

@@ -412,12 +412,10 @@ TEST(api, find_exported_table)
     ASSERT_TRUE(opt_table);
     EXPECT_EQ(opt_table->table, instance->table.get());
     EXPECT_EQ(opt_table->table->size(), 2);
-    ASSERT_TRUE((*opt_table->table)[0].has_value());
-    EXPECT_EQ((*opt_table->table)[0]->instance, instance.get());
-    EXPECT_EQ((*opt_table->table)[0]->func_idx, 1);
-    ASSERT_TRUE((*opt_table->table)[1].has_value());
-    EXPECT_EQ((*opt_table->table)[1]->instance, instance.get());
-    EXPECT_EQ((*opt_table->table)[1]->func_idx, 0);
+    EXPECT_EQ((*opt_table->table)[0].instance, instance.get());
+    EXPECT_EQ((*opt_table->table)[0].func_idx, 1);
+    EXPECT_EQ((*opt_table->table)[1].instance, instance.get());
+    EXPECT_EQ((*opt_table->table)[1].func_idx, 0);
     EXPECT_EQ(opt_table->limits.min, 2);
     ASSERT_TRUE(opt_table->limits.max.has_value());
     EXPECT_EQ(opt_table->limits.max, 20);

test/unittests/instantiate_test.cpp

@@ -18,7 +18,7 @@ namespace
 uint32_t call_table_func(Instance& instance, size_t idx)
 {
     const auto& elem = (*instance.table)[idx];
-    const auto res = execute(*elem->instance, elem->func_idx, {}, 0);
+    const auto res = execute(*elem.instance, elem.func_idx, {}, 0);
     EXPECT_TRUE(res.has_value);
     return as_uint32(res.value);
 }
@@ -551,7 +551,7 @@ TEST(instantiate, element_section)
     auto instance = instantiate(parse(bin));
 
     ASSERT_EQ(instance->table->size(), 4);
-    EXPECT_FALSE((*instance->table)[0].has_value());
+    EXPECT_EQ((*instance->table)[0].instance, nullptr);
     EXPECT_EQ(call_table_func(*instance, 1), 1);
     EXPECT_EQ(call_table_func(*instance, 2), 3);
     EXPECT_EQ(call_table_func(*instance, 3), 3);
@@ -576,10 +576,10 @@ TEST(instantiate, element_section_offset_from_global)
     auto instance = instantiate(parse(bin));
 
     ASSERT_EQ(instance->table->size(), 4);
-    EXPECT_FALSE((*instance->table)[0].has_value());
+    EXPECT_EQ((*instance->table)[0].instance, nullptr);
     EXPECT_EQ(call_table_func(*instance, 1), 1);
     EXPECT_EQ(call_table_func(*instance, 2), 2);
-    EXPECT_FALSE((*instance->table)[3].has_value());
+    EXPECT_EQ((*instance->table)[3].instance, nullptr);
 }
 
 TEST(instantiate, element_section_offset_from_imported_global)
@@ -601,10 +601,10 @@ TEST(instantiate, element_section_offset_from_imported_global)
     auto instance = instantiate(parse(bin), {}, {}, {}, {g});
 
     ASSERT_EQ(instance->table->size(), 4);
-    EXPECT_FALSE((*instance->table)[0].has_value());
+    EXPECT_EQ((*instance->table)[0].instance, nullptr);
     EXPECT_EQ(call_table_func(*instance, 1), 1);
     EXPECT_EQ(call_table_func(*instance, 2), 2);
-    EXPECT_FALSE((*instance->table)[3].has_value());
+    EXPECT_EQ((*instance->table)[3].instance, nullptr);
 }
 
 TEST(instantiate, element_section_offset_too_large)
@@ -695,10 +695,10 @@ TEST(instantiate, element_section_out_of_bounds_doesnt_change_imported_table)
 
     const auto& table_elements = *table->table;
     ASSERT_EQ(table_elements.size(), 3);
-    EXPECT_EQ(table_elements[0]->instance, instance_exported_table.get());
-    EXPECT_EQ(table_elements[0]->func_idx, 0);
-    EXPECT_FALSE(table_elements[1].has_value());
-    EXPECT_FALSE(table_elements[2].has_value());
+    EXPECT_EQ(table_elements[0].instance, instance_exported_table.get());
+    EXPECT_EQ(table_elements[0].func_idx, 0);
+    EXPECT_EQ(table_elements[1].instance, nullptr);
+    EXPECT_EQ(table_elements[2].instance, nullptr);
 }
 
 TEST(instantiate, data_section)
@@ -815,8 +815,8 @@ TEST(instantiate, data_elem_section_errors_dont_change_imports)
         instantiate(parse(bin_data_error), {}, {{&table, {3, std::nullopt}}}, {{&memory, {1, 1}}}),
         instantiate_error, "data segment is out of memory bounds");
 
-    EXPECT_FALSE(table[0].has_value());
-    EXPECT_FALSE(table[1].has_value());
+    EXPECT_EQ(table[0].instance, nullptr);
+    EXPECT_EQ(table[1].instance, nullptr);
     EXPECT_EQ(memory[0], 0);
 
     /* wat2wasm
@@ -837,9 +837,9 @@ TEST(instantiate, data_elem_section_errors_dont_change_imports)
         instantiate(parse(bin_elem_error), {}, {{&table, {3, std::nullopt}}}, {{&memory, {1, 1}}}),
         instantiate_error, "element segment is out of table bounds");
 
-    EXPECT_FALSE(table[0].has_value());
-    EXPECT_FALSE(table[1].has_value());
-    EXPECT_FALSE(table[2].has_value());
+    EXPECT_EQ(table[0].instance, nullptr);
+    EXPECT_EQ(table[1].instance, nullptr);
+    EXPECT_EQ(table[2].instance, nullptr);
     EXPECT_EQ(memory[0], 0);
 }