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

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added a comment.

Once you've updated the line length for clang-format, please run the tests through clang-format a second time to remove the excess line-breaking and get them readable again.



================
Comment at: libcxx/include/iterator:532
+concept __can_reference = requires {
+  typename __with_reference<_Tp>;
+};
----------------
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.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99854