Skip to content

Commit

Permalink
Avoid using std::optional for maybe uninitialized variables
Browse files Browse the repository at this point in the history
  • Loading branch information
akokoshn committed May 9, 2024
1 parent d0f3289 commit 4e3014d
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 22 deletions.
28 changes: 14 additions & 14 deletions include/nil/blueprint/assigner.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -731,22 +731,20 @@ namespace nil {
size_t num_cells = layout_resolver->get_cells_num<BlueprintFieldType>(dest->getType());
if (num_cells == 1) {
auto &cell = memory[ptr];
if(!cell.v.has_value()) {
if(!detail::is_initialized(cell.v)) {
log.debug(boost::format("Load uninitialized var %1% = 0") % ptr);
cell.v = zero_var;
}
var v = cell.v.value();
frame.scalars[dest] = v;
frame.scalars[dest] = cell.v;
} else {
std::vector<var> res;
for (size_t i = 0; i < num_cells; ++i) {
auto &cell = memory[ptr + i];
if(!cell.v.has_value()) {
if(!detail::is_initialized(cell.v)) {
log.debug(boost::format("Load uninitialized var %1% = 0") % (ptr + i));
cell.v = zero_var;
}
var v = cell.v.value();
res.push_back(v);
res.push_back(cell.v);
}
frame.vectors[dest] = res;
}
Expand Down Expand Up @@ -981,30 +979,30 @@ namespace nil {
if (i < false_memory_region_end && i < true_memory_region_end) {
auto v_true = is_stack ? state.stack[i - memory_region_begin].v : state.heap[i - memory_region_begin].v;
auto v_false = memory[i].v;
if (v_true.has_value() && v_false.has_value()) {
if (!detail::is_internal<var>(v_true.value()) && !detail::is_internal<var>(v_false.value())) {
if (detail::is_initialized(v_true) && detail::is_initialized(v_false)) {
if (!detail::is_internal<var>(v_true) && !detail::is_internal<var>(v_false)) {
// cell exist and contains real var in both state and current memory, so merged result = select(cond, state var, current memory var)
memory.store(i, create_select_component<BlueprintFieldType, var>(
cond, v_true.value(), v_false.value(), circuits[currProverIdx], assignments[currProverIdx], internal_storage, statistics, param, one_var));
cond, v_true, v_false, circuits[currProverIdx], assignments[currProverIdx], internal_storage, statistics, param, one_var));
} else {
typename BlueprintFieldType::value_type res_value = 0;
if (gen_mode.has_assignments()) {
res_value = (get_var_value(cond) != res_value) ? get_var_value(v_true.value()) : get_var_value(v_false.value());
res_value = (get_var_value(cond) != res_value) ? get_var_value(v_true) : get_var_value(v_false);
}
// cell exist in both state and current memory, but contains internal var, so merged result = internal var
var internal_select_res = put_value_into_internal_storage(res_value);
memory.store(i, internal_select_res);
}
} else if (v_true.has_value()) {
} else if (detail::is_initialized(v_true)) {
// cell exist in both state and current memory, but only state_var has value, so merged result = state_var
memory.store(i, v_true.value());
memory.store(i, v_true);
}
// otherwise merge result = current_memory var
} else if (i < true_memory_region_end) {
auto v_true = is_stack ? state.stack[i - memory_region_begin].v : state.heap[i - memory_region_begin].v;
if (v_true.has_value()) {
if (detail::is_initialized(v_true)) {
// cell exist only in state, so merged result = state_var
memory.store(i, v_true.value());
memory.store(i, v_true);
}
}
// otherwise merge result = current_memory var
Expand Down Expand Up @@ -1564,12 +1562,14 @@ namespace nil {
extract_inst->getAggregateOperand()->getType(), extract_inst->getIndices())
.second;
var v = memory.load(ptr);
ASSERT(detail::is_initialized(v));
frame.scalars[inst] = v;
return inst->getNextNonDebugInstruction();
}
case llvm::Instruction::IndirectBr: {
ptr_type ptr = resolve_number<ptr_type>(frame, inst->getOperand(0));
var bb_var = memory.load(ptr);
ASSERT(detail::is_initialized(bb_var));
llvm::BasicBlock *bb = (llvm::BasicBlock *)(resolve_number<uintptr_t>(bb_var));
ASSERT(labels.find(bb) != labels.end());
return &bb->front();
Expand Down
1 change: 1 addition & 0 deletions include/nil/blueprint/extract_constructor_parameters.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ namespace nil {
for (std::size_t i = 0; i < input_length; i++) {
ASSERT(memory[input_ptr].size == (BlueprintFieldType::number_bits + 7) / 8);
auto v = memory.load(input_ptr++);
ASSERT(detail::is_initialized<var>(v));
res.push_back(v);
}
return res;
Expand Down
2 changes: 1 addition & 1 deletion include/nil/blueprint/handle_component.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ namespace nil {
BOOST_LOG_TRIVIAL(trace) << "input var: " << v.get() << " " << var_value(assignment, v.get()).data;

// component input can't be from internal_storage'
ASSERT(!detail::is_internal<var>(v.get()));
ASSERT(!detail::is_internal<var>(v.get()) && detail::is_initialized<var>(v.get()));
if ((used_rows.find(v.get().rotation) == used_rows.end()) &&
(v.get().type == var::column_type::witness || v.get().type == var::column_type::constant)) {
var new_v;
Expand Down
12 changes: 5 additions & 7 deletions include/nil/blueprint/memory.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ namespace nil {

template<typename VarType>
struct cell {
std::optional<VarType> v;
VarType v;
size_t offset;
int8_t size;
int8_t following;
Expand All @@ -59,9 +59,9 @@ namespace nil {
struct program_memory : public std::vector<cell<VarType>> {
public:
program_memory(size_t stack_size) : stack_size(stack_size), heap_top(stack_size) {
this->push_back({std::nullopt, 0, 0});
this->push_back({VarType(), 0, 0});
this->resize(heap_top);
this->push_back({std::nullopt, stack_size + 1, 0});
this->push_back({VarType(), stack_size + 1, 0});
heap_top++;
push_frame();
}
Expand Down Expand Up @@ -93,7 +93,7 @@ namespace nil {
auto offset = this->back().offset + this->back().size;
ptr_type res = this->size();
for (size_t i = 0; i < num_bytes; ++i) {
this->push_back(cell<VarType>{std::nullopt, offset++, 1});
this->push_back(cell<VarType>{VarType(), offset++, 1});
heap_top++;
}
return res;
Expand All @@ -104,9 +104,7 @@ namespace nil {
}

VarType load(ptr_type ptr) {
auto maybe_v = (*this)[ptr].v;
ASSERT(maybe_v.has_value());
return maybe_v.value();
return (*this)[ptr].v;
}

size_t ptrtoint(ptr_type ptr) {
Expand Down
5 changes: 5 additions & 0 deletions include/nil/blueprint/utilities.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@ namespace nil {
}
return false;
}

template<typename var>
bool is_initialized(const var &v) {
return (v.type != var::column_type::uninitialized);
}
} // namespace detail
} // namespace blueprint
} // namespace nil
Expand Down

0 comments on commit 4e3014d

Please sign in to comment.