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

remove std::distance usage #2030

Merged
merged 1 commit into from
Aug 5, 2021
Merged

remove std::distance usage #2030

merged 1 commit into from
Aug 5, 2021

Conversation

neheb
Copy link
Contributor

@neheb neheb commented Aug 5, 2021

std::distance internally runs a loop, which may or may not be optimized
away. Just use simple arithmetic.

Signed-off-by: Rosen Penev rosenp@gmail.com

std::distance internally runs a loop, which may or may not be optimized
away. Just use simple arithmetic.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
@gabime gabime merged commit ba29e1d into gabime:v1.x Aug 5, 2021
@gabime
Copy link
Owner

gabime commented Aug 5, 2021

Thanks @neheb

@kelbon
Copy link

kelbon commented Oct 12, 2022

lol what

@kin4stat
Copy link
Contributor

kin4stat commented Oct 13, 2022

template<class InputIterator>
  constexpr typename iterator_traits<InputIterator>::difference_type
    distance(InputIterator first, InputIterator last);

Distance guarantees a substraction operator, which is the same as your "fix" in your PR, if InputIterator type meets RandomAccessIterator requirements.
iterator.operations#5

std::begin and std::end with C-array in arguments returns pointer to element, [iterator.range-4], [iterator.range-55]

RandomAccessIterator requirement can be seen here [random.access.iterators#1].

We can add and substract two pointers and pointer + intergral and get implementation defined integral result: [expr.add-2.2] [expr.add-2.3]
We can compare two pointers, and get bool result: [expr.rel-4]
We can subscript pointer with resulting type X&: [expr.sub-2]
We can add/substract and assign pointers in single expression: [expr.ass-6]

Thus, the pointer meets the RandomAccessIterator requirements, and then std::distance from it and std::begin(level_string_views) is the same as it - std::begin(level_string_views)

@kin4stat
Copy link
Contributor

kin4stat commented Oct 13, 2022

@gabime please roll back changes

@tt4g
Copy link
Contributor

tt4g commented Oct 13, 2022

@kin4stat It would be better to open PR to revert changes.

gabime added a commit that referenced this pull request Oct 14, 2022
Replace iterator difference with std::distance(revert #2030)
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.

5 participants