[libcxx-commits] [PATCH] D101316: [libcxx][ranges] Add `random_access_{iterator, range}`.

Mark de Wever via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Apr 29 09:44:58 PDT 2021


Mordante added inline comments.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.random.access/random_access_iterator.compile.pass.cpp:75
+
+struct no_plus_equals {
+    typedef std::random_access_iterator_tag iterator_category;
----------------
zoecarver wrote:
> Mordante wrote:
> > Quuxplusone wrote:
> > > Mordante wrote:
> > > > These classes share a lot of the same boiler-plate. That makes it hard to spot the part being tested. How do you feel about moving the common part to a macro? 
> > > I agree, but... Each "part being tested" touches a different line of the boilerplate. Do you have an idea of what kind of macro could help here?
> > > My best offering is, what if you cut-and-pasted the exact same text N times and then //simply added the characters `//`// in order to comment out the one line you wanted to affect in each case. That would still have a lot of the same downsides, but at least it would be a tiny bit clearer what line(s) were intentionally missing, and it would make it easier for a human maintainer to verify that the test "correctly fails" when you comment-in any single one of those lines.
> > > 
> > > A very minor improvement would be to use the short names `int*`, `int&`, `int` instead of `pointer`, `reference`, `difference_type`, throughout. That reduces the amount of "stuff" the human eye has to wade through.
> > If we remove the constructor the classes look the same until `friend bool operator>=(const self, const self y);`. (Look the same since it takes too much effort to validate.) So that could be put in a macro. Then at the interesting part of the class we could use your suggested comments. I think that would make the test easier to maintain.
> > 
> What about just putting it into a base class, `bidirectional_iterator`?
Sounds good to me, actually better than a macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101316



More information about the libcxx-commits mailing list