beman-project / iterator_interface

Apache License 2.0
1 stars 3 forks source link

Address deducing *this dependency #10

Open camio opened 1 day ago

camio commented 1 day ago

This code, unlike the implementation used in beman.optional26, depends on deducing *this, a C++23 feature. This dependency makes it impossible to evaluate the library in projects that are using C++20 and earlier toolchains.

We could conditionally compile an implementation that doesn't depend on deducing *this, but this changes the interface. With deducing this, note that the CRTP isn't used,

template <class IteratorConcept,
          class ValueType,
          class Reference      = ValueType&,
          class Pointer        = ValueType*,
          class DifferenceType = ptrdiff_t>
class iterator_interface;

With the Boost, version, the CRTP is used. Note the Derived template argument.

template <typename Derived,
          typename IteratorConcept,
          typename ValueType,
          typename Reference      = ValueType&,
          typename Pointer        = ValueType*,
          typename DifferenceType = std::ptrdiff_t
          >
struct iterator_interface;

Here are some possible ways to address this:

  1. Do nothing. Here we turn away many potential users and opportunities for feedback.
  2. Make a second structure available. Here we'd provide a iterator_interface_crtp which doesn't use deducing *this.
  3. Provide different interfaces based on a build setting. In this case we'd have a configuration variable that would select whether the CRTP or deducing this interface is available.
steve-downey commented 1 day ago

The CRTP implementation isn't that crazy to do, but there might be edge/corner cases that behave slightly differently?

Feature test macros and std version checks are a good way of getting ODR violations if someone mixes them. It's the main reason to avoid setting it within a Cmake project.

camio commented 21 hours ago

Feature test macros and std version checks are a good way of getting ODR violations if someone mixes them.

I agree. We had this problem with https://github.com/stlab/libraries. We successfully mitigated it by generating a config.hpp at CMake time for the project.