Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move execute(initializer_list) to fizzy::test #657

Merged
merged 1 commit into from
Nov 20, 2020
Merged

Move execute(initializer_list) to fizzy::test #657

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions lib/fizzy/execute.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,4 @@ constexpr ExecutionResult Trap{false};

// Execute a function on an instance.
ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, int depth = 0);

inline ExecutionResult execute(
Instance& instance, FuncIdx func_idx, std::initializer_list<Value> args)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why move this though?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The std::initializer_list<Value> cannot be used in public API like that. This is not what it is designed for. We can use std::span instead.
  2. I prefer to change it to std::initializer_list<TypedValue> later instead of having a ton of overloads spread accross many places.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The std::initializer_list cannot be used in public API like that. This is not what it is designed for. We can use std::span instead.

Not arguing against, but can you explain the reason for this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This usage is probably fine, but there are some doubts about the lifetime of the {} object. The std::initializer_list is to provide fancy syntax to containers. Somehow in C++20 had been decided that std::span cannot reference std::initializer_list.

In practice this is mostly useful in cases when you want to provide some fixed number of arguments, i.e. mostly tests and examples. None of the other tools need it.

{
assert(args.size() == instance.module->get_function_type(func_idx).inputs.size());
return execute(instance, func_idx, args.begin());
}
} // namespace fizzy
2 changes: 1 addition & 1 deletion test/unittests/api_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
#include <gtest/gtest.h>
#include <lib/fizzy/limits.hpp>
#include <test/utils/asserts.hpp>
#include <test/utils/execute_helpers.hpp>
#include <test/utils/hex.hpp>
#include <test/utils/instantiate_helpers.hpp>

using namespace fizzy;
using namespace fizzy::test;
Expand Down
2 changes: 1 addition & 1 deletion test/unittests/end_to_end_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
#include "parser.hpp"
#include <gtest/gtest.h>
#include <test/utils/asserts.hpp>
#include <test/utils/execute_helpers.hpp>
#include <test/utils/hex.hpp>
#include <test/utils/instantiate_helpers.hpp>

using namespace fizzy;
using namespace fizzy::test;
Expand Down
1 change: 1 addition & 0 deletions test/unittests/execute_floating_point_conversion_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "parser.hpp"
#include "trunc_boundaries.hpp"
#include <test/utils/asserts.hpp>
#include <test/utils/execute_helpers.hpp>
#include <test/utils/hex.hpp>
#include <cmath>

Expand Down
2 changes: 1 addition & 1 deletion test/unittests/execute_floating_point_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include "parser.hpp"
#include "trunc_boundaries.hpp"
#include <test/utils/asserts.hpp>
#include <test/utils/instantiate_helpers.hpp>
#include <test/utils/execute_helpers.hpp>
#include <cmath>

using namespace fizzy;
Expand Down
9 changes: 8 additions & 1 deletion test/utils/execute_helpers.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,17 @@

namespace fizzy::test
{
inline ExecutionResult execute(
Instance& instance, FuncIdx func_idx, std::initializer_list<Value> args)
{
assert(args.size() == instance.module->get_function_type(func_idx).inputs.size());
return fizzy::execute(instance, func_idx, args.begin());
}

inline ExecutionResult execute(const std::unique_ptr<const Module>& module, FuncIdx func_idx,
std::initializer_list<Value> args)
{
auto instance = instantiate(*module);
return execute(*instance, func_idx, args);
return test::execute(*instance, func_idx, args);
}
} // namespace fizzy::test