Skip to content
Snippets Groups Projects

Introduce new class IteratorFacade and add proxy iterator support

Merged Carsten Gräser requested to merge feature/iterator-facades-proxy-support into master

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 the reference 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 deducing this.
  • The old facade classes are aliases to IteratorFacade with the appropriate category tag. They also now have an additional template parameter for pointer that defaults to value_type*. Notice that the position of this template parameter is (confusingly, but for good reason) different to IteratorFacade: For the old facade classes it is appended to keep backward compatibility, for IteratorFacade it follows the order of std::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.

Edited by Carsten Gräser

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Carsten Gräser added 3 commits

    added 3 commits

    • 267229b1 - Introduce new class IteratorFacade
    • 9f5e91b4 - Implement TransformedRangeIterator using IteratorFacade
    • 4e923517 - Add changelog entry for IteratorFacade

    Compare with previous version

  • 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 new IteratorFacade.

  • Carsten Gräser changed the description

    changed the description

  • Carsten Gräser changed title from Introduce new class IteratorFacade and add prox support to Introduce new class IteratorFacade and add proxy iterator support

    changed title from Introduce new class IteratorFacade and add prox support to Introduce new class IteratorFacade and add proxy iterator support

  • added 1 commit

    • bc818f0b - Make TransformedRangeIterator more flexible

    Compare with previous version

  • Carsten Gräser changed the description

    changed the description

  • Carsten Gräser mentioned in merge request !1386

    mentioned in merge request !1386

  • 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.

  • Simon Praetorius
  • Simon Praetorius
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading