Skip to content

refactor: introduce a python-like enumerate() method, and use it in block_reward_reallocation_tests.cpp#5059

Merged
UdjinM6 merged 1 commit into
dashpay:developfrom
PastaPastaPasta:util/enumerate
Oct 23, 2022
Merged

refactor: introduce a python-like enumerate() method, and use it in block_reward_reallocation_tests.cpp#5059
UdjinM6 merged 1 commit into
dashpay:developfrom
PastaPastaPasta:util/enumerate

Conversation

@PastaPastaPasta
Copy link
Copy Markdown
Member

@PastaPastaPasta PastaPastaPasta commented Oct 22, 2022

Issue being fixed or feature implemented

I like being explicit where possible, and the lack of an enumerate function in c++ has annoyed me for a while. This PR implements a enumerate() function that takes in an iterable object, and returns an tuple containing an iterator and a index. This can be decomposed using structured bindings. This makes code clearer and more readable, and should easily compile down to the same machine code based on my limited playing with it in godbolt

What was done?

How Has This Been Tested?

make check

Breaking Changes

n/a

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@PastaPastaPasta PastaPastaPasta added this to the 18.2 milestone Oct 22, 2022
UdjinM6
UdjinM6 requested changes Oct 22, 2022
View reviewed changes
Copy link
Copy Markdown

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

pls see 35db267504

Also, is src/test/block_reward_reallocation_tests.cpp the only place where this could be useful?

@PastaPastaPasta
Copy link
Copy Markdown
Member Author

PastaPastaPasta commented Oct 22, 2022

No, I'm sure it could be useful in a plethora of places, but here was the location that made me think of this. I'll apply your commit soon, I had actually done those changes but forgot to push 🙈

@PastaPastaPasta
refactor: introduce a new enumerate.h file that operates like python'…
5268c46 
…s enumerate() method, and use it in block_reward_reallocation_tests.cpp
UdjinM6
UdjinM6 approved these changes Oct 23, 2022
View reviewed changes
Copy link
Copy Markdown

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK

@UdjinM6 UdjinM6 changed the title refactor: introduce a new enumerate.h file that operates like python's enumerate() method, and use it in block_reward_reallocation_tests.cpp refactor: introduce a python-like enumerate() method, and use it in block_reward_reallocation_tests.cpp Oct 23, 2022
@UdjinM6 UdjinM6 merged commit 88adf24 into dashpay:develop Oct 23, 2022
@PastaPastaPasta PastaPastaPasta deleted the util/enumerate branch October 23, 2022 21:00
@kwvg kwvg mentioned this pull request Jan 10, 2026
Merged
5 tasks
PastaPastaPasta added a commit that referenced this pull request Mar 12, 2026
@PastaPastaPasta
Merge #7098: refactor: consolidate Dash-specific utils between C++23 …
ccdb0dc 
…polyfills (`util/std23.h`) and helpers (`util/helpers.h`)

1c8260c refactor: add `util::to_string(bool)` for convenience (Kittywhiskers Van Gogh)
dec2766 refactor: merge `util/ranges.h` and `util/pointer.h` to `util/helpers.h` (Kittywhiskers Van Gogh)
f57edb5 refactor: use `std23::ranges::fold_left` in Dash-specific code (Kittywhiskers Van Gogh)
4c30be5 util: add polyfill for `std::ranges::fold_left` (Kittywhiskers Van Gogh)
5dc17fc refactor: use `std23::ranges::contains` in Dash-specific code (Kittywhiskers Van Gogh)
43e64ed util: add polyfill for `std::ranges::contains` (Kittywhiskers Van Gogh)
79f0711 refactor: replace `irange::range` with thin `std::views::iota` wrapper (Kittywhiskers Van Gogh)
58a9a3d refactor: switch from `irange(min, max)` to `std::views::iota(min, max)` (Kittywhiskers Van Gogh)
6bee408 refactor: distinguish between custom helpers and polyfills (Kittywhiskers Van Gogh)
f1d5d30 refactor: move `enumerate` to C++23 polyfill, mirror namespace structure (Kittywhiskers Van Gogh)
aa12106 refactor: rename `ToUnderlying()` to match C++23 convention (Kittywhiskers Van Gogh)
2ca0fd7 fix: enum bound check in GetSimulatedErrorRate (Kittywhiskers Van Gogh)

Pull request description:

  ## Motivation

  The upgrade to C++20 ([dash#6380](#6380)) allowed us to drop most of our polyfills meant as bridge from C++17, though we still have utils beyond the ones inherited from upstream that have accumulated over time, most of them, seeking to bridge C++23 features while others are entirely custom or serve as convenience functions or thin wrappers.

  Some of them include:
  * [dash#4622](#4622) for `util/ranges.h` (mostly now an alias to [`std::ranges`](https://en.cppreference.com/w/cpp/ranges.html) after C++20 migration except for `find_if_opt`, which is custom)
  * [dash#4788](#4788) for `util/irange.h` (equivalent to [`std::views::iota`](https://en.cppreference.com/w/cpp/ranges/iota_view.html), introduced in C++20)
  * [dash#5059](#5059) for `util/enumerate.h`  (equivalent to [`std::views::enumerate`](http://en.cppreference.com/w/cpp/ranges/enumerate_view.html), introduced in C++23)
  * [dash#5210](#5210) for `util/underlying.h` (equivalent to [`std::to_underlying`](https://en.cppreference.com/w/cpp/utility/to_underlying.html), introduced in C++23)
  * 0a4e726 for `util/pointer.h` (custom `std::shared_ptr` helper)

  This pull requests compacts all of those helpers into two files:
  * `util/helpers.h` (custom helpers, replacing `util/irange.h`, `util/pointer.h`, `util/ranges.h`)
  * `util/std23.h` (polyfill for C++23, replacing `util/enumerate.h` and `util/underlying.h`)

  And additionally, introduces additional capabilities utilised in Dash-specific code:

  * `std23::ranges::contains` (equivalent to [`std::ranges::contains`](https://en.cppreference.com/w/cpp/algorithm/ranges/contains.html))
  * `std23::ranges::fold_left` (equivalent to [`std::ranges::fold_left`](https://en.cppreference.com/w/cpp/algorithm/ranges/fold_left.html))
  * `util::to_string(bool)` (equivalent to `value ? "true" : "false"`)

  ## Additional Information

  * `ToUnderlying()` needed to be renamed to `to_underlying()` to match with the standard library naming so we can discontinue the polyfill easily when we migrate to C++23 by just going `s/std23/std/g`.

  * While `iranges::range` is equivalent to `std::views::iota`, the latter requires both min and max to be of the same type but literal `0` has a type incompatible with `size_t`, requiring `size_t{0}` to compile. As propagation of these changes for each instance is bothersome, `util::irange()` exists as a thin wrapper that wraps the literal around the type for `max`

  ## Breaking Changes

  None expected.

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    utACK 1c8260c; seems reasonable
  UdjinM6:
    utACK 1c8260c

Tree-SHA512: 82f4b74f304d8680ddb03a923843905a470b374c034284dbe6a013d19e67a769743965f710c5267751742488961907d346ce721d01029cffa543178fd560dcc1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@UdjinM6 UdjinM6 UdjinM6 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

18.2

Development

Successfully merging this pull request may close these issues.

2 participants

@PastaPastaPasta @UdjinM6