[libcxx-commits] [libcxx] [libc++] Remove `optional::iterator` from experimental mode (PR #203782)

[Nikolas Klauser via libcxx-commits]

philnik777 wrote:

> @philnik777 One thing we can do here is:
> 
> ```c++
> struct __optional_iterator : __capacity_aware_iterator<...> {
>   // whatever
> };
> 
> // then, inside optional
> using iterator = __optional_iterator<...>;
> ```
> 
> Then, whether we end up implementing `optional::iterator` using `__capacity_aware_iterator` directly or not becomes an implementation detail. If we want to detect that an iterator is capacity-aware, we'd implement a trait like `__is_capacity_aware` and query iterators for that instead of checking if they're exactly a specialization of `__capacity_aware_iterator`. I'd be okay with that approach too, in fact it encapsulates `optional::iterator` more than using a transparent typedef. I'm not sure if we've discussed this approach already in the `__capacity_aware_iterator` PR and decided against it, but right not that seems like an acceptable idea to me.
> 
> This would remove concerns around baking that type into our ABI right now and allow us to un-experimentalize `optional::iterator`.

I do think that design would probably be an improvement, but part of my contention with the design is that we have new types at all. Having to instantiate all the algorithms again is a substantial compile-time cost for benefits that I think are rather weak. OTOH if we want to pay this cost, we might as well benefit from it fully by e.g. significantly improving safety, say through an iterator that sets a flag once it's incremented (I guess essentially boiling down to an efficiently stored count). If we get that right we might even be able to communicate to the compiler that the range is either zero or one elements in size without having to do special magic in the library. AFAIK that would be ABI breaking.

I guess I feel like we're currently in a state that is neither as compile-time efficient as I'd like it to be nor does it make full use of the capabilities of a special iterator, which I find rather bad to set in stone.

https://github.com/llvm/llvm-project/pull/203782