[libcxx-commits] [PATCH] D99854: [libcxx] adds `cpp17-.*iterator` concepts (as `__legacy_.*iterator`)

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/iterator:532
+concept __can_reference = requires {
+  typename __with_reference<_Tp>;
+};
----------------
ldionne wrote:
> cjdb wrote:
> > zoecarver wrote:
> > > Quuxplusone wrote:
> > > > CaseyCarter wrote:
> > > > > ldionne wrote:
> > > > > > I was surprised to see that we can't simply write `typename _Tp&`. I'm not sure what part of the concepts spec disallows it, do you know?
> > > > > > 
> > > > > > Would it make sense to rename this to `__referenceable`? Then below we would have
> > > > > > 
> > > > > > ```
> > > > > > concept __dereferenceable = requires(_Tp& __t) {
> > > > > >   { *__t } -> __referenceable;
> > > > > > };
> > > > > > ```
> > > > > > 
> > > > > > which I think is nicely consistent.
> > > > > > I was surprised to see that we can't simply write `typename _Tp&`. I'm not sure what part of the concepts spec disallows it, do you know?
> > > > > 
> > > > > So was I! The _type-requirement_ grammar is "_type-requirement_: `typename` _nested-name-specifier<sub>opt</sub>_ _type-name_ `;`", which doesn't allow any kind of adornments. From a very high level I suppose this makes sense - `_Tp&` is a type, but not the _name_ of a type - but it does mean that some things we expect to work don't.
> > > > > 
> > > > > There's probably material here for a 1-2 page paper to propose a change.
> > > > Re `__can_reference` versus `__referenceable`: This is matching the (current) draft wording in http://eel.is/c++draft/iterator.synopsis#concept:can-reference so I think it's defensible.
> > > > 
> > > I agree, `__referenceable` does sound better. But I think it's not outweighed by the benefit of having this code more or less match the standard wording verbatim. 
> > That name can be editorially changed (probably with LWG's permission), so I'm okay with choosing the name that makes more sense.
> FWIW I would have been fine with keeping `__can_reference` after being told that it's what's used in the Standard. I'm fine either way, basically, feel free to do as you think is best.
Oh, and thanks for the explanation here @CaseyCarter !


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99854/new/

https://reviews.llvm.org/D99854