Task #2859

Change ArrayRef iterator type from pointer to std::iterator

Added by Paul Bauer 15 days ago. Updated 13 days ago.

Fix uploaded
core library
Target version:


Currently a number of coding bugs are possible when comparing iterators from ArrayRef to those from other std::containers, as ArrayRef::iterator is actual a pointer to the data in the container.
Changing the iterator type would avoid bugs where those iterators are compared to pointers or nullptr, but would add complexity to the code.
Another option would be using std::span for ArrayRef itself but this would either need to be natively implemented or imported from an existing implementation.

Associated revisions

Revision 8b901fad (diff)
Added by Roland Schulz 10 days ago

Remove EmptyArrayRef

Make it more aligned with std::span.
Almost everywhere it wasn't needed anyhow.
Only exception: ternary conditional operator. But there the type
was already explict in all but one case and that one case
(src/gromacs/domdec/distribute.cpp) was confusing because of
multiple implicit conversions.

Related #2859

Change-Id: I0b61abdc2e60285a7ca17e3bdc7e53cb72c5cf6a


#1 Updated by Paul Bauer 14 days ago

  • Tracker changed from Feature to Task

I think this is better described as a task

#2 Updated by Mark Abraham 14 days ago

My suggestion is that we fix the ArrayRef iterator (I think that is easy). Then we should get a c++14 std::span like thing into compat, or similar. I did have some WIP on gerrit for a c++11 span some time ago, but by now we'd do better to import a new one.

#3 Updated by Gerrit Code Review Bot 14 days ago

Gerrit received a related patchset '1' for Issue #2859.
Uploader: Roland Schulz ()
Change-Id: gromacs~master~I80e2b6a75bca0973f550e0ab8fa62c622c330f90
Gerrit URL:

#4 Updated by Roland Schulz 14 days ago

  • Status changed from New to Fix uploaded

Instead of importing a new std::span we probably should explore making span a simple 1D template alias of mdspan. If we don't discover any significant disadvantage this would avoid duplication.

#5 Updated by Roland Schulz 14 days ago

We need to fix this for begin/end for mdspan too (added here). Independent of whether we use mdspan for span or not.

#6 Updated by Gerrit Code Review Bot 13 days ago

Gerrit received a related patchset '1' for Issue #2859.
Uploader: Roland Schulz ()
Change-Id: gromacs~master~I0b61abdc2e60285a7ca17e3bdc7e53cb72c5cf6a
Gerrit URL:

Also available in: Atom PDF