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.

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

Changes for BSIP40 custom authorities #79

Merged
merged 7 commits into from
Oct 25, 2018
Merged

Changes for BSIP40 custom authorities #79

merged 7 commits into from
Oct 25, 2018

Conversation

abitmore
Copy link
Member

@abitmore abitmore commented Oct 6, 2018

This PR contains changes need in FC for implementing custom active authorities (BSIP40 / bitshares/bitshares-core#1285):

  • variant: a function to convert bool to variant;
  • static_variant: ability to get data type with a given index (which) without constructing an object;
  • reflector: ability to visit local member of reflected data structure by index (was only able to visit by name).

Ready for merge.

@@ -326,6 +326,16 @@ class static_variant {
return impl::storage_ops<0, Types...>::apply(_tag, storage, v);
}

template<tag_type w, typename visitor>
static typename visitor::result_type visit(visitor& v) {
Copy link

Choose a reason for hiding this comment

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

The PR comment indicates that this intends to provide the

ability to get data type with a given index (which) without constructing an object

If it's only about this then a construct like position<> might be more efficient. But if you need the visitor it's fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

The visitor is used here. (I assume) it just works. Perhaps there are better implementation though.

Copy link
Member Author

@abitmore abitmore left a comment

Choose a reason for hiding this comment

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

Thanks.

static typename visitor::result_type visit( tag_type tag, visitor& v, void* data )
{
static std::vector<std::function<typename visitor::result_type(visitor&,void*)>> wrappers = init_wrappers<visitor,void*,Types...>();
FC_ASSERT( tag < count(), "Unsupported type ${tag}!", ("tag",tag) );
Copy link
Member Author

Choose a reason for hiding this comment

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

Assert tag >= 0 as well?

BTW please wrap long lines (perhaps can use auto here)? (BTW just found that Github in Firefox in Ubuntu desktop 18.04 only shows 123 characters a line, while in Chrome in Windows 7 it shows 130 characters)

@jmjatlanta
Copy link

Compiled and ran tests on Ubuntu 18.04 / Boost 1.67 / OpenSSL 1.10. All without errors

( Unrelated note: The app test/bip_lock is not a true Boost test. )

Not approving PR to give @pmconrad a chance to wrap lines. But all looks good to me.

template<typename Visitor,typename Data, typename T, typename ... Types>
std::vector<std::function<typename Visitor::result_type(Visitor&,Data)>> init_const_wrappers()
{
std::vector<std::function<typename Visitor::result_type(Visitor&,Data)>> result = init_const_wrappers<Visitor,Data,Types...>();
Copy link
Member Author

Choose a reason for hiding this comment

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

This line is still too long for me..

template<typename Visitor,typename Data, typename T, typename ... Types>
std::vector<std::function<typename Visitor::result_type(Visitor&,Data)>> init_wrappers()
{
std::vector<std::function<typename Visitor::result_type(Visitor&,Data)>> result = init_wrappers<Visitor,Data,Types...>();
Copy link
Member Author

Choose a reason for hiding this comment

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

This line is OK in Chrome but too long in Firefox

@pmconrad
Copy link

@jmjatlanta since both abit and me have contributed code, could you please re-review and approve? Thanks!

Copy link

@jmjatlanta jmjatlanta left a comment

Choose a reason for hiding this comment

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

All looks good. Thank you both!

@jmjatlanta
Copy link

0# fc::print_stacktrace(std::ostream&) in tests/all_tests
1# fc_stacktrace::_svdt_visitor::operator()abi:cxx11 const in tests/all_tests
2# 0x000056380B4E9E19 in tests/all_tests
3# 0x000056380B4E9E6F in tests/all_tests
4# fc_stacktrace::_svdt_visitor::result_type fc::static_variant<unsigned char, unsigned short, unsigned int, unsigned long, signed char, short, int, long>::visit<fc_stacktrace::_svdt_visitor>(long, fc_stacktrace::_svdt_visitor const&, void*) in tests/all_tests
5# fc_stacktrace::_svdt_visitor::result_type fc::static_variant<unsigned char, unsigned short, unsigned int, unsigned long, signed char, short, int, long>::visit<fc_stacktrace::_svdt_visitor>(fc_stacktrace::_svdt_visitor const&) in tests/all_tests
6# fc_stacktrace::static_variant_depth_test::test_method() in tests/all_tests
7# 0x000056380B4E88FB in tests/all_tests
8# boost::detail::function::void_function_invoker0<void ()(), void>::invoke(boost::detail::function::function_buffer&) in tests/all_tests
9# boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) in tests/all_tests
10# boost::execution_monitor::catch_signals(boost::function<int ()> const&) in tests/all_tests
11# boost::execution_monitor::execute(boost::function<int ()> const&) in tests/all_tests
12# boost::execution_monitor::vexecute(boost::function<void ()> const&) in tests/all_tests
13# boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned int) in tests/all_tests
14# boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned int, boost::unit_test::framework::state::random_generator_helper const
) in tests/all_tests
15# boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned int, boost::unit_test::framework::state::random_generator_helper const*) in tests/all_tests
16# boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned int, boost::unit_test::framework::state::random_generator_helper const*) in tests/all_tests
17# boost::unit_test::framework::run(unsigned long, bool) in tests/all_tests
18# boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) in tests/all_tests
19# __libc_start_main in /lib/x86_64-linux-gnu/libc.so.6
20# _start in tests/all_tests

Compiled on Ubutntu 18.04 / Boost 1.65.1

@abitmore abitmore deleted the for-custom-auth branch May 21, 2019 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants