Introduce new class IteratorFacade and add proxy iterator support
The new CRTP-mixin class IteratorFacade
is parameterized
by implementation, iterator category, value_type
, reference
,
pointer
, and difference_type
. Discussion of the changes:
- Making the iterator category a template parameter, allows to work with the facades class in a more uniform way. E.g. comparisons only need to be defined once for all categories.
- Being able to customize
pointer
allows to use the facade classes for proxy-iterators. This was not possible before, because it is not sufficient to simply exchange thereference
type to support proxies. - The design is in line with the
std::iterator_interface
proposal, in the sense that the template parameters have the same order to avoid surprises. Notice however, that this is not a drop-in replacement for two reasons:-
std::iterator_interface
is parameterized by the iterator concept instead of the category tag. -
srd::iterator_interface
avoids classic CRTP by deducingthis
.
-
The old facade classes are aliases toIteratorFacade
with the appropriate category tag. They also now have an additional template parameter forpointer
that defaults tovalue_type*
. Notice that the position of this template parameter is (confusingly, but for good reason) different toIteratorFacade
: For the old facade classes it is appended to keep backward compatibility, forIteratorFacade
it follows the order ofstd::iterator_interface
.- The old facade classes remain unchanged for backward compatibility reasons.
A major consequence of this change is, that it allows to
use the facades for proxy-iterators, like e.g. TransformedRangeIterator
.
In this case you have to specify a suitable proxy-type for
the pointer. In this case the new helper Dune::ProxyArrowResult
can
be used. This was adopted from the TransformedRangeIterator
, but
the name was adjusted to the std::iterator_interface
proposal.
This MR also includes some cleanups to TransformedRangeIterator
made possible by the new facades and preparing it for supporting
free operators.
Merge request reports
Activity
Unfortunately, adding support for specifying the pointer type is not possible for the old facades without breaking compatibility. Using an additional defaulted template parameter still changes the template parameter list which is part of the
GenericIterator
interface. While gcc is happy with this (due to the default value) clang complains about a mismatch. Hence I'll drop this feature again. If you want to create proxy-iterators you thus have to use the newIteratorFacade
.mentioned in merge request !1386
assigned to @simon.praetorius
It seems that there's no sane way to let the old facade classes use the new one. There's a few options, but all fail in some way:
- (A) We cannot simply define the old ones as template alias. This breaks friend declarations for the facades.
- (B) Alternatively we could let the old facades derive from the new one. But then access to the implementation methods in the derives classes does not work if they only declare the old facade as friend.
- (C) To fix the issue with (B) one may pass the old facade as implementation class to the new one and forward
dereference()
,increment()
, ... to the derived user implementation. But then it's getting rely tricky to get the comparisons correct.
In the end it seems that the simplest and cleanest approach is to keep the new and old facade in parallel.
- Resolved by Carsten Gräser
- Resolved by Carsten Gräser
- Resolved by Carsten Gräser
- Resolved by Carsten Gräser
- Resolved by Carsten Gräser
- Resolved by Carsten Gräser
- Resolved by Carsten Gräser
- Resolved by Carsten Gräser
- Resolved by Carsten Gräser
- Resolved by Carsten Gräser
- Resolved by Carsten Gräser