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

eastl::ranges #465

Open
bsviglo opened this issue Mar 28, 2022 · 11 comments
Open

eastl::ranges #465

bsviglo opened this issue Mar 28, 2022 · 11 comments

Comments

@bsviglo
Copy link

bsviglo commented Mar 28, 2022

Is any plan to add ranges API to C++ ?

@MasterDrake
Copy link

seems an enormous task to do it alone

@MasterDrake
Copy link

seems an enormous task to do it alone

I'm working on it, but there are still a lot of problems to be solved and I'm frankly don't know what I'm doing or what half of the stuff breaks.
A help would be appreciated :)

@MasterDrake
Copy link

Just needs some problems to fix and checks whether it compiles on clang and GCC.

@jhopkins-ea
Copy link
Contributor

there is no plans to add ranges to EASTL. EASTL has never been intended as a drop in replacement for the C++ std library. it is only intended to implement certain features - primarily the containers and algorithms.
EASTL compatability and interoperability with the std library is also not a primary goal. I may investigate EASTL compatability with std::ranges, but there's no timeline on this.

@MasterDrake
Copy link

MasterDrake commented Jan 8, 2024

there is no plans to add ranges to EASTL. EASTL has never been intended as a drop in replacement for the C++ std library. it is only intended to implement certain features - primarily the containers and algorithms. EASTL compatability and interoperability with the std library is also not a primary goal. I may investigate EASTL compatability with std::ranges, but there's no timeline on this.

I'm porting range-v3 to EASTL but I'm still facing some issues that I can't seem to fix.
When I think it's ready or worth it, I'll publish it, even though I won't see you using it or whatever,

However during my process, I found some minor issues and problems with EASTL, but I'm not confident enough to make a PR.

Mainly the lack of constexpr in tuple, the lack of a constexpr addressof, a lacking "random" header, and what I think is a typo bug in heap.

@jhopkins-ea
Copy link
Contributor

even if you aren't confident enough to submit a PR, feel free to raise issues for those things. though, the lack of random number generators is outside of the scope for EASTL.

@MasterDrake
Copy link

MasterDrake commented Jan 19, 2024

@jhopkins-ea I really hate to ping you again and bother you,
but I was wondering if there was an up-to-date version of tuple.h, maybe an internal version
I have a test that's failing and it's because of a missing default constructor. I tried to copy&paste msvc or gcc implementation but tuple is such a complex class I don't understand what I'm reading.
The line in the original implementation is this.
This is MoveOnlyString, if needed.
(downloading range-v3 and stepping into that line will tell you what constructor is being called and what's need to be added to EASTL/tuple.h), but I understand if it's too much work.

I'm asking because if I can fix this, it could lead me to fixing views::zip and getting closer to finishing EARanges.
Thanks and sorry again.

That is the error log:

/home/drake/GameDev/Projects/range-v3.5/test/iterator/basic_iterator.cpp:438:37: error: use of deleted function ‘constexpr eastl::tuple<T, Ts ...>::tuple() [with T = test_move_only::MoveOnly; Ts = {}]’
[build]   438 |         *i = eastl::tuple<MoveOnly>{};
[build]       |                                     ^
[build] In file included from /home/drake/GameDev/Projects/range-v3.5/include/EARanges/concepts/swap.hpp:16,
[build]                  from /home/drake/GameDev/Projects/range-v3.5/include/EARanges/concepts/concepts.hpp:28,
[build]                  from /home/drake/GameDev/Projects/range-v3.5/include/EARanges/iterator/basic_iterator.hpp:22,
[build]                  from /home/drake/GameDev/Projects/range-v3.5/test/iterator/basic_iterator.cpp:12:
[build] /home/drake/GameDev/Projects/range-v3.5/build/vcpkg_installed/x64-linux/include/EASTL/tuple.h:711:22: note: ‘constexpr eastl::tuple<T, Ts ...>::tuple() [with T = test_move_only::MoveOnly; Ts = {}]’ is implicitly deleted because the default definition would be ill-formed:
[build]   711 |         EA_CONSTEXPR tuple() = default;
[build]       |                      ^~~~~
[build] /home/drake/GameDev/Projects/range-v3.5/build/vcpkg_installed/x64-linux/include/EASTL/tuple.h:711:22: error: use of deleted function ‘constexpr eastl::Internal::TupleImpl<eastl::integer_sequence<long unsigned int, Is ...>, Ts ...>::TupleImpl() [with long unsigned int ...Indices = {0}; Ts = {test_move_only::MoveOnly}]’
[build] /home/drake/GameDev/Projects/range-v3.5/build/vcpkg_installed/x64-linux/include/EASTL/tuple.h:339:30: note: ‘constexpr eastl::Internal::TupleImpl<eastl::integer_sequence<long unsigned int, Is ...>, Ts ...>::TupleImpl() [with long unsigned int ...Indices = {0}; Ts = {test_move_only::MoveOnly}]’ is implicitly deleted because the default definition would be ill-formed:
[build]   339 |                 EA_CONSTEXPR TupleImpl() = default;
[build]       |                              ^~~~~~~~~
[build] /home/drake/GameDev/Projects/range-v3.5/build/vcpkg_installed/x64-linux/include/EASTL/tuple.h:339:30: error: no matching function for call to ‘eastl::Internal::TupleLeaf<0, test_move_only::MoveOnly, true>::TupleLeaf()’
[build] /home/drake/GameDev/Projects/range-v3.5/build/vcpkg_installed/x64-linux/include/EASTL/tuple.h:258:26: note: candidate: ‘template<class T> eastl::Internal::TupleLeaf<I, ValueType, true>::TupleLeaf(const eastl::Internal::TupleLeaf<I, T>&) [with T = T; long unsigned int I = 0; ValueType = test_move_only::MoveOnly]’
[build]   258 |                 explicit TupleLeaf(const TupleLeaf<I, T>& t)
[build]       |                          ^~~~~~~~~
[build] /home/drake/GameDev/Projects/range-v3.5/build/vcpkg_installed/x64-linux/include/EASTL/tuple.h:258:26: note:   template argument deduction/substitution failed:
[build] /home/drake/GameDev/Projects/range-v3.5/build/vcpkg_installed/x64-linux/include/EASTL/tuple.h:339:30: note:   candidate expects 1 argument, 0 provided
[build]   339 |                 EA_CONSTEXPR TupleImpl() = default;
[build]       |                              ^~~~~~~~~
[build] /home/drake/GameDev/Projects/range-v3.5/build/vcpkg_installed/x64-linux/include/EASTL/tuple.h:252:26: note: candidate: ‘template<class T, class> eastl::Internal::TupleLeaf<I, ValueType, true>::TupleLeaf(T&&) [with T = T; <template-parameter-2-2> = <template-parameter-1-2>; long unsigned int I = 0; ValueType = test_move_only::MoveOnly]’
[build]   252 |                 explicit TupleLeaf(T&& t)
[build]       |                          ^~~~~~~~~
[build] /home/drake/GameDev/Projects/range-v3.5/build/vcpkg_installed/x64-linux/include/EASTL/tuple.h:252:26: note:   template argument deduction/substitution failed:
[build] /home/drake/GameDev/Projects/range-v3.5/build/vcpkg_installed/x64-linux/include/EASTL/tuple.h:339:30: note:   candidate expects 1 argument, 0 provided
[build]   339 |                 EA_CONSTEXPR TupleImpl() = default;
[build]       |                              ^~~~~~~~~

@jhopkins-ea
Copy link
Contributor

Are you using the most recent version of EASTL? It looks like you are running into an issue that was fixed in 3.21.12:

Make tuple default constructible when an element type has no member types (which tuple has an empty base optimization for).

@MasterDrake
Copy link

Are you using the most recent version of EASTL? It looks like you are running into an issue that was fixed in 3.21.12:

Make tuple default constructible when an element type has no member types (which tuple has an empty base optimization for).

I hate my life.
This is the second time in this project where if I just read or checked beforehand I've saved sooo much time...
I'm using vcpkg to pull EASTL and I thought it was gonna use the latest commit since it's been a while.
Well, when I checked apparently the vcpkg version is the same that you linked but just to be sure I decided to try with the latest commit and now a lot of the unfixable stuff works right off the bat. What a relief.
Now it's missing some static checks due iterators quirks and most importantly eastl::swap vs ADL(which seems above my paygrade due all the complexity of range-v3)...
Once I fix that, I'll re-check how it runs on Clang and GCC just to be sure and check all the C++ versions.
Done that I think I'm ready to release it, even though there's a lot to be fixed or trimmed or adjusted, parity with eastl::algorithms first.
(funny thing, it still doesn't use eastl:: namespace, so there's also that)

I should've asked for help sooner, thank you so much!

@MasterDrake
Copy link

@jhopkins-ea
I'm continuing my work and now I'm trying to match eastl algorithms to ranges since you said that was your focus but I'm kinda stuck on the best course of action:

  • should I strive for algorithm equality(meaning they do the same thing) or should I try to use the eastl versions?
    The biggest problem in doing that is iterator and sentinel are/may be different template parameters and eastl algorithms also lack projection. That's why I'm just matching the same implementation and then calling the eastl algorithms when I can.
  • Another problem I face is with the return values, as I said not only iterator and sentinel are different types but ranges return a couple of them usually, as per tradition of not losing computed information. Not sure if I'm supposed just to return the same thing an eastl algorithm returns or return a pair of iterator/sentinel.
  • Is adding an XSentinel template parameter for everything in eastl that defaulted to the XIterator template feasible or acceptable?
    (would that break the API or increase the template instantiations count?)

@jhopkins-ea
Copy link
Contributor

I can't give advice on what to do with your library. You should bear in mind that eastl and specifically its algorithms are not equivalent or a conforming implementation of the standard library. I can only assume ranges-v3 relies on a conforming implementation.
For the specific issue of the differences between iterators and sentinels, you might find ranges-v3 has some equivalent to std::ranges::common_view that may help.

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

No branches or pull requests

3 participants