Repository navigation
Get rid of nested std::bind, and some boost removal #152
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 from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -48,7 +48,9 @@ inline void sort(RandomAccessRange& range) { | |||||
| */ | ||||||
| template <class RandomAccessIterator, class Compare> | ||||||
| inline void sort(RandomAccessIterator first, RandomAccessIterator last, Compare comp) { | ||||||
| return std::sort(first, last, std::bind(comp, std::placeholders::_1, std::placeholders::_2)); | ||||||
| return std::sort(first, last, [comp](const auto& a, const auto& b) { | ||||||
| return std::invoke(comp, a, b); | ||||||
| }); | ||||||
| } | ||||||
|
|
||||||
| /*! | ||||||
|
|
@@ -63,7 +65,9 @@ template <typename I, // I models RandomAccessIterator | |||||
| inline void sort(I f, I l, C c, P p) { | ||||||
| return std::sort( | ||||||
| f, l, | ||||||
| std::bind(c, std::bind(p, std::placeholders::_1), std::bind(p, std::placeholders::_2))); | ||||||
| [&](const auto& a, const auto& b) { | ||||||
| return std::invoke(c, std::invoke(p, a), std::invoke(p, b)); | ||||||
| }); | ||||||
| } | ||||||
|
|
||||||
| /*! | ||||||
|
|
@@ -76,9 +80,7 @@ template <typename R, // I models RandomAccessRange | |||||
| typename P> | ||||||
| // P models UnaryFunction(value_type(I)) -> T | ||||||
| inline void sort(R& r, C c, P p) { | ||||||
| return adobe::sort( | ||||||
| boost::begin(r), boost::end(r), | ||||||
| std::bind(c, std::bind(p, std::placeholders::_1), std::bind(p, std::placeholders::_2))); | ||||||
| return adobe::sort(boost::begin(r), boost::end(r), c, p); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we avoid boost here?
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we could, but that will come at a price this will not work, std::begin()/end doesn't work with pair, and passing pair of iterators is extremely effective. None of it will matter with c++20 and ranges, but before that it's useful
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for that clarification. The changes LGTM - lambdas are much easier to read than |
||||||
| } | ||||||
|
|
||||||
| /*! | ||||||
|
|
@@ -109,7 +111,9 @@ inline void stable_sort(RandomAccessRange& range) { | |||||
| template <class RandomAccessIterator, class Compare> | ||||||
| inline void stable_sort(RandomAccessIterator first, RandomAccessIterator last, Compare comp) { | ||||||
| return std::stable_sort(first, last, | ||||||
| std::bind(comp, std::placeholders::_1, std::placeholders::_2)); | ||||||
| [&](const auto& a, const auto& b) { | ||||||
| return std::invoke(comp, a, b); | ||||||
| }); | ||||||
| } | ||||||
|
|
||||||
| /*! | ||||||
|
|
@@ -154,7 +158,9 @@ inline void partial_sort_copy(InputIterator first, InputIterator last, | |||||
| RandomAccessIterator result_first, RandomAccessIterator result_last, | ||||||
| Compare comp) { | ||||||
| return std::partial_sort_copy(first, last, result_first, result_last, | ||||||
| std::bind(comp, std::placeholders::_1, std::placeholders::_2)); | ||||||
| [&](const auto& a, const auto& b) { | ||||||
| return std::invoke(comp, a, b); | ||||||
| }); | ||||||
| } | ||||||
|
|
||||||
| /*! | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -739,7 +739,7 @@ void sheet_t::implementation_t::add_output(name_t name, const line_position_t& p | |
| // REVISIT (sparent) : Non-transactional on failure. | ||
| cell_set_m.push_back(cell_t( | ||
| access_output, name, | ||
| std::bind(&implementation_t::calculate_expression, std::ref(*this), position, expression), | ||
| [&, this]() { return calculate_expression(position, expression); }, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The expression is escaping. In the original bind position and expression are captured by value - capturing them by reference is dangerous and may be escaping a temporary. I believe this should be |
||
| cell_set_m.size(), nullptr)); | ||
|
|
||
| output_index_m.insert(cell_set_m.back()); | ||
|
|
@@ -765,8 +765,9 @@ void sheet_t::implementation_t::add_interface(name_t name, bool linked, | |
|
|
||
| if (initializer_expression.size()) { | ||
| cell_set_m.push_back(cell_t(name, linked, | ||
| std::bind(&implementation_t::calculate_expression, | ||
| std::ref(*this), position1, initializer_expression), | ||
| [&, this]() { | ||
| return calculate_expression(position1, initializer_expression); | ||
| }, | ||
| cell_set_m.size())); | ||
| } else { | ||
| cell_set_m.push_back(cell_t(name, linked, cell_t::calculator_t(), cell_set_m.size())); | ||
|
|
@@ -781,12 +782,13 @@ void sheet_t::implementation_t::add_interface(name_t name, bool linked, | |
| if (expression.size()) { | ||
| // REVISIT (sparent) : Non-transactional on failure. | ||
| cell_set_m.push_back(cell_t(access_interface_output, name, | ||
| std::bind(&implementation_t::calculate_expression, | ||
| std::ref(*this), position2, expression), | ||
| [&, this]() { | ||
| return calculate_expression(position2, expression); | ||
| }, | ||
| cell_set_m.size(), &cell_set_m.back())); | ||
| } else { | ||
| cell_set_m.push_back(cell_t(access_interface_output, name, | ||
| std::bind(&implementation_t::get, std::ref(*this), name), | ||
| [&, this]() { return get(name); }, | ||
| cell_set_m.size(), &cell_set_m.back())); | ||
| } | ||
| output_index_m.insert(cell_set_m.back()); | ||
|
|
@@ -810,7 +812,7 @@ void sheet_t::implementation_t::add_interface(name_t name, any_regular_t initial | |
| cell.priority_m = ++priority_high_m; | ||
|
|
||
| cell_set_m.push_back(cell_t(access_interface_output, name, | ||
| std::bind(&implementation_t::get, std::ref(*this), name), | ||
| [&, this]() { return get(name); }, | ||
| cell_set_m.size(), &cell)); | ||
|
|
||
| output_index_m.insert(cell_set_m.back()); | ||
|
|
@@ -852,7 +854,7 @@ void sheet_t::implementation_t::add_logic(name_t logic, const line_position_t& p | |
| const array_t& expression) { | ||
| cell_set_m.push_back(cell_t( | ||
| access_logic, logic, | ||
| std::bind(&implementation_t::calculate_expression, std::ref(*this), position, expression), | ||
| [&, this]() { return calculate_expression(position, expression); }, | ||
| cell_set_m.size(), nullptr)); | ||
|
|
||
| if (!name_index_m.insert(cell_set_m.back()).second) { | ||
|
|
@@ -868,7 +870,7 @@ void sheet_t::implementation_t::add_invariant(name_t name, const line_position_t | |
| // REVISIT (sparent) : Non-transactional on failure. | ||
| cell_set_m.push_back(cell_t( | ||
| access_invariant, name, | ||
| std::bind(&implementation_t::calculate_expression, std::ref(*this), position, expression), | ||
| [&, this]() { return calculate_expression(position, expression); }, | ||
| cell_set_m.size(), nullptr)); | ||
|
|
||
| output_index_m.insert(cell_set_m.back()); | ||
|
|
@@ -953,8 +955,11 @@ sheet_t::connection_t sheet_t::implementation_t::monitor_enabled(name_t n, const | |
| monitor(active_m.test(iter->cell_set_pos_m) || (value_accessed_m.test(iter->cell_set_pos_m) && | ||
| (touch_set & priority_accessed_m).any())); | ||
|
|
||
| return monitor_enabled_m.connect(std::bind(&sheet_t::implementation_t::enabled_filter, this, | ||
| touch_set, iter->cell_set_pos_m, monitor, _1, _2)); | ||
| std::size_t iter_pos = iter->cell_set_pos_m; | ||
| return monitor_enabled_m.connect( | ||
| [&, this](const cell_bits_t& a, const cell_bits_t& b) { | ||
| enabled_filter(touch_set, iter_pos, monitor, a, b); | ||
| }); | ||
| } | ||
|
|
||
| /**************************************************************************************************/ | ||
|
|
@@ -1006,8 +1011,7 @@ sheet_t::implementation_t::monitor_contributing(name_t n, const dictionary_t& ma | |
| monitor(contributing_set(mark, iter->contributing_m)); | ||
|
|
||
| return iter->monitor_contributing_m.connect( | ||
| std::bind(monitor, std::bind(&sheet_t::implementation_t::contributing_set, std::ref(*this), | ||
| mark, _1))); | ||
| [this, mark, monitor](const cell_bits_t& bits) { monitor(contributing_set(mark, bits)); }); | ||
| } | ||
|
|
||
| /**************************************************************************************************/ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -200,17 +200,37 @@ eve_callback_suite_t bind_layout(const bind_layout_proc_t& proc, sheet_t& sheet, | |
| eve_callback_suite_t suite; | ||
|
|
||
| suite.add_view_proc_m = | ||
| std::bind(proc, _1, _3, std::bind(&evaluate_named_arguments, std::ref(evaluator), _4)); | ||
| suite.add_cell_proc_m = std::bind(&add_cell, std::ref(sheet), _1, _2, _3, _4); | ||
| suite.add_relation_proc_m = std::bind(&add_relation, std::ref(sheet), _1, _2, _3, _4); | ||
| [&evaluator, &proc](const eve_callback_suite_t::position_t& parent, const line_position_t& /* parse_location */, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The prior call captured |
||
| name_t name, const array_t& parameters, const std::string& /* brief */, | ||
| const std::string& /* detailed */) -> eve_callback_suite_t::position_t { | ||
| return proc(parent, name, evaluate_named_arguments(evaluator, parameters)); | ||
| }; | ||
| suite.add_cell_proc_m = | ||
| [&sheet](adobe::eve_callback_suite_t::cell_type_t type, | ||
| adobe::name_t name, const adobe::line_position_t& position, | ||
| const adobe::array_t& init_or_expr, | ||
| const std::string& /* brief */, const std::string& /* detailed */) -> void { | ||
| add_cell(sheet, type, name, position, init_or_expr); | ||
| }; | ||
| suite.add_relation_proc_m = | ||
| [&sheet](const adobe::line_position_t& position, | ||
| const adobe::array_t& conditional, | ||
| const adobe::eve_callback_suite_t::relation_t* first, | ||
| const adobe::eve_callback_suite_t::relation_t* last, | ||
| const std::string& /* brief */, const std::string& /* detailed */) -> void { | ||
| add_relation(sheet, position, conditional, first, last); | ||
| }; | ||
| suite.add_interface_proc_m = | ||
| [&sheet](name_t name, bool linked, const line_position_t& position1, | ||
| const array_t& initializer, const line_position_t& position2, | ||
| const array_t& expression, const std::string& /* brief */, | ||
| const std::string& /* detailed */) -> void { | ||
| sheet.add_interface(name, linked, position1, initializer, position2, expression); | ||
| }; | ||
| suite.finalize_sheet_proc_m = std::bind(&sheet_t::update, std::ref(sheet)); | ||
| suite.finalize_sheet_proc_m = | ||
| [&sheet]() -> void { | ||
| sheet.update(); | ||
| }; | ||
|
|
||
| return suite; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pred()needs to be called withstd::invoketo match the prior implementation and support.vshould be passed asconst&