Page MenuHomePhabricator

[libcxx] adds `std::incrementable_traits` to <iterator>
ClosedPublic

Authored by cjdb on Mar 22 2021, 9:01 PM.

Details

Summary

Implements parts of:

  • P0896R4 The One Ranges Proposal

Depends on D99041

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Quuxplusone added inline comments.Mar 23 2021, 8:25 AM
libcxx/include/iterator
461

This is the only reason <iterator> needs to include <concepts> in C++20 mode, right?
@ldionne, how about we just use is_integral_v here instead?

template<class _Tp>
concept __has_integral_minus =
  !__has_difference_type_member<_Tp> &&
  requires(const _Tp& __x, const _Tp& __y) {
    requires is_integral_v<decltype( __x - __y )>;
  };

(And/or, wait for <concepts> to stop depending on <iterator>; D99041, D99124.)

I note that the name __has_integral_minus is slightly misleading; it currently means __has_integral_minus_without_difference_type. The paper standard doesn't factor out these concepts, so we have no exposition-only names to imitate here. Maybe __incrementable_traits_use_difference_type<_Tp> and __incrementable_traits_use_minus<_Tp> would be better? But I don't really object to the current name even if I do find it "slightly misleading."

471

Just declval<_Tp>(), not std::declval<_Tp>(). (If it had any arguments, then definitely _VSTD::declval to prevent ADL — but it has no arguments. Actually we still have a ton of places that do _VSTD::declval, so I'm okay with that spelling too. Just not std::.)

libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.pass.cpp
22 ↗(On Diff #332518)

Almost all places where you write incrementable_traits, you should be writing difference_typehas_difference_type, difference_type_matches, check_difference_type.

158 ↗(On Diff #332518)

"with_cv"? I see no cv-qualifiers here.
Also, my usual opinion on noexcept here: noexcept is absolutely irrelevant to this test, and so it would be nice not to see it at all. At the very least, please make at least one of these tests check what happens when the subtraction operation is non-noexcept, because that's the realistic case in the wild.

cjdb updated this revision to Diff 333040.Mar 24 2021, 10:34 AM

applies @Quuxplusone's feedback

libcxx/include/iterator
461

This is the only reason <iterator> needs to include <concepts> in C++20 mode, right?

For this patch? Yes.

It's a part of the synopsis, so in general, no.

(And/or, wait for <concepts> to stop depending on <iterator>; D99041, D99124.)

That was my intention, hence "Depends on D99041".

The paper standard doesn't factor out these concepts, so we have no exposition-only names to imitate here. Maybe __incrementable_traits_use_difference_type<_Tp> and __incrementable_traits_use_minus<_Tp> would be better?

Right. I think I got the idea of naming these from readable.traits, so __has_member_difference_type (sic) is probably best for consistency. As for __has_integral_minus, I'm ambivalent, so long as it's consistent.

471

Eh, ADL isn't a concern here. Removed.

libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.pass.cpp
43 ↗(On Diff #332518)

See below for int(*)() and friends.

Can you also make sure the patch passes CI?

libcxx/include/iterator
16

Can you add the required includes?
http://eel.is/c++draft/iterator.synopsis

#include <compare>              // see [compare.syn]
#include <concepts>             // see [concepts.syn]
431

Maybe also add compare while you're at it.

461

The synopsis requires concepts http://eel.is/c++draft/iterator.synopsis

libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.pass.cpp
1 ↗(On Diff #332518)

http://eel.is/c++draft/incrementable.traits#3 Users may specialize incrementable_­traits on program-defined types.
can you add a test for this?

cjdb updated this revision to Diff 333580.Mar 26 2021, 10:02 AM
cjdb marked 7 inline comments as done.

partially applies @Mordante's feedback

cjdb added a comment.Mar 26 2021, 10:03 AM

Can you also make sure the patch passes CI?

This is failing CI because of a bug in GCC 10 that was fixed in trunk (example). @ldionne should I add an include guard around the offending line or should we upgrade our CI's GCC?

libcxx/include/iterator
16

I've added it for <concepts>, but will leave <compare> for the person who adds that functionality (probably me in a future patch).

431

See above.

Quuxplusone added inline comments.Mar 28 2021, 8:53 AM
libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.pass.cpp
31–36 ↗(On Diff #333580)

Take the suggested edit.

222 ↗(On Diff #333580)

My usual complaints about hundreds of lines of tests, which test very little.
Please remove a large percentage of lines 94–222, and replace them with:

  • One test that fails when line 458's const _Tp& __x is replaced with const _Tp x. (You currently lack any.)
  • One test that fails when line 458's const _Tp& __x is replaced with _Tp& x. (You currently lack any.)
  • One test that fails when line 458's const _Tp& __x is replaced with _Tp x. (You currently lack any.)
43 ↗(On Diff #332518)

Please add int[10].

cjdb updated this revision to Diff 333769.Mar 28 2021, 11:17 PM
cjdb marked an inline comment as done.

adds test coverage

cjdb marked an inline comment as done.Mar 28 2021, 11:18 PM
cjdb added inline comments.
libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.pass.cpp
31–36 ↗(On Diff #333580)

The suggested edit doesn't add anything constructive and appears to be a stylistic preference, so there's nothing to change.

222 ↗(On Diff #333580)

My usual complaints about hundreds of lines of tests, which test very little.
Please remove a large percentage of lines 94–222

As mentioned in previous patches: I have been asked to add tests, and my comprehensive coverage has been encouraged by several others, including two maintainers.

There are many lines, yes, but each of these tests ensures that cv-qualifiers and reference qualifiers are correctly handled.

and replace them with:

  • One test that fails when line 458's const _Tp& __x is replaced with const _Tp x. (You currently lack any.)
  • One test that fails when line 458's const _Tp& __x is replaced with _Tp& x. (You currently lack any.)
  • One test that fails when line 458's const _Tp& __x is replaced with _Tp x. (You currently lack any.)

I have added these three and more.

zoecarver accepted this revision as: zoecarver.Mar 29 2021, 11:08 PM

Thanks!

libcxx/include/iterator
474

Hmm. I really feel like we should have a better way to track what is implemented, what is blocked on other patches, etc. TODOs might be OK for now, but I'd like to sort this out.

libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.pass.cpp
222 ↗(On Diff #333580)

I don't feel opposition to lots of tests, even if they might seem excessive. Though, it might be good to have a few simple tests at the top or bottom that are easily verifiable and human-readable that just show "this works right in the common case". That being said, this patch LGTM (but let's make sure Arthur is also OK with it).

Quuxplusone added inline comments.Mar 30 2021, 7:40 AM
libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.pass.cpp
222 ↗(On Diff #333580)

Well, I wouldn't say I'm "OK" with it; but I agree I haven't got the clout to block this patch series until it looks the way I think it should.

The number of tests in a patch should generally be proportional to the number of lines-of-code changed: here Chris is adding 24 LOC, so the tests should be on that order of magnitude.
In any given patch, each test should usually guard against some specific failure mode; failure modes are usually of the form "what happens if line X of the patch bit-rots under maintenance?" So for example, "what happens if line 458's const _Tp& is replaced with _Tp?" Tests that can't be traced back to a specific failure mode are automatically suspect, because in my experience (both here and elsewhere in life) "hundreds of tests" is often a substitute for "any tests that actually test something."

Another way to put it is, test-code is just like ordinary code. Writing hundreds of lines of code can be useful, but it can also indicate that you need to take a step back to look at the problem you're trying to solve. Once you understand the problem being solved, you might find that an elegant fix is available.

The hundred-line rough draft can still be useful! Looking at Chris's "rough draft" with its hundreds of lines of const_lvalue_ref_volatile_rvalue_ref_subtraction gave me the clue "okay, cvref-qualifiers are important to this code somehow." Then I looked at the problem being solved — "okay, we need to test that something might bit-rot in this code, in the area of cvref-qualifiers specifically." That's how I found that none of these tests were testing what they intended. The next step is to write a surgical patch that solves the problem; I think I can write the new test for each of the three bullet points above in about 5 lines each. With the smaller patch in hand, the big patch is no longer technically necessary — and therefore I personally wouldn't check it in. (Although, again, I understand that I'm not the gatekeeper here, and the big patch is quite likely to get checked in regardless.)

Can you fix the Apple build failures? (Guess it's caused by the parent commit.)

libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.pass.cpp
222 ↗(On Diff #333580)

I'm personally also not too fond of the amount of tests and I agree with @Quuxplusone. However some maintainers have asked for verbose tests for some of the concepts in earlier reviews. Maybe we should discuss on discord what we expect of unit tests and how verbose. I don't like to block this review on that discussion; the tests aren't wrong and we can always reduce the number of tests later.

cjdb updated this revision to Diff 335057.Apr 2 2021, 9:21 PM
cjdb marked an inline comment as done.

fixes include guard to guard on concepts

cjdb updated this revision to Diff 335200.Apr 4 2021, 10:47 PM

adds internal ranges feature-test macro

Quuxplusone added inline comments.Apr 5 2021, 9:09 AM
libcxx/include/__config
851

Please use <= 17, for consistency with similar lines in this file. (We tend to guard C++2a stuff on > 17. I don't think there's anything special about the Ranges stuff in this respect, right?)

libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.pass.cpp
34–35 ↗(On Diff #335200)

Note that T const is the same type as T whenever T is a reference type. Therefore, any place in this file that instantiates check_incrementable_traits<Foo&> is suspicious — it's simply static-asserting that the difference type of T is the difference type of T.
I suggest that you add static_assert(!std::is_reference_v<T>) inside this template, and then re-run the tests, and investigate/fix any failures that arise from that.
I also suggest that if you actually expect result to be true in all cases, you should add

assert(result);

inside this function. (Or is it okay if result is false, sometimes? I didn't closely examine all the call sites.)

ldionne requested changes to this revision.EditedApr 5 2021, 3:06 PM

I'd like to put an end to comments about "too many tests" once and for all. I think we all agree that test coverage is a good property to have. I think we also agree that readability is a good property to have, and that code duplication is bad. We want both, as much as possible.

I think what we're seeing here is an example where we have a lot of code coverage, which is great, however the cost (right now) is a lot of repetition, and hence decreased readability. For me, the solution is not to remove the test coverage, but instead to implement it without reducing readability so much. Here's an example of how this can be done (there are other ways, for example we could use templates instead but I just hacked this together). This replaces lines 174 to 702. Yes it's still not *small*, but it's much better and IMO easier to read, while not dropping any of the coverage. And it would be easy to make it even better by using two nested "loops" over all qualifiers, but it's unclear to me that would actually increase readability.

#define TEST_NOT_DIFFERENCE_TYPE(qual1, qual2)                                    \
  struct TEST_CONCAT(test_subtraction_,__LINE__) {                                     \
    friend int operator-(TEST_CONCAT(test_subtraction_,__LINE__) qual1,                \
                         TEST_CONCAT(test_subtraction_,__LINE__) qual2);               \
  };                                                                              \
  static_assert(!check_has_difference_type<TEST_CONCAT(test_subtraction_,__LINE__)>)   \
/**/

TEST_NOT_DIFFERENCE_TYPE(&&, &&);
TEST_NOT_DIFFERENCE_TYPE(&, &);
TEST_NOT_DIFFERENCE_TYPE(&, const&);
TEST_NOT_DIFFERENCE_TYPE(&, volatile&);
TEST_NOT_DIFFERENCE_TYPE(&, const volatile&);
TEST_NOT_DIFFERENCE_TYPE(&, &&);
TEST_NOT_DIFFERENCE_TYPE(&, const&&);
TEST_NOT_DIFFERENCE_TYPE(&, volatile&&);
TEST_NOT_DIFFERENCE_TYPE(&, const volatile&&);
TEST_NOT_DIFFERENCE_TYPE(const&, &);
TEST_NOT_DIFFERENCE_TYPE(const&, volatile&);
TEST_NOT_DIFFERENCE_TYPE(const&, &&);
TEST_NOT_DIFFERENCE_TYPE(const&, const&&);
TEST_NOT_DIFFERENCE_TYPE(const&, volatile&&);
TEST_NOT_DIFFERENCE_TYPE(const&, const volatile&&);
TEST_NOT_DIFFERENCE_TYPE(volatile&, &);
TEST_NOT_DIFFERENCE_TYPE(volatile&, const&);
TEST_NOT_DIFFERENCE_TYPE(volatile&, volatile&);
TEST_NOT_DIFFERENCE_TYPE(volatile&, const volatile&);
TEST_NOT_DIFFERENCE_TYPE(volatile&, &&);
TEST_NOT_DIFFERENCE_TYPE(volatile&, const&&);
TEST_NOT_DIFFERENCE_TYPE(volatile&, volatile&&);
TEST_NOT_DIFFERENCE_TYPE(volatile&, const volatile&&);
TEST_NOT_DIFFERENCE_TYPE(const volatile&, &);
TEST_NOT_DIFFERENCE_TYPE(const volatile&, volatile&);
TEST_NOT_DIFFERENCE_TYPE(const volatile&, &&);
TEST_NOT_DIFFERENCE_TYPE(const volatile&, const&&);
TEST_NOT_DIFFERENCE_TYPE(const volatile&, volatile&&);
TEST_NOT_DIFFERENCE_TYPE(const volatile&, const volatile&&);
TEST_NOT_DIFFERENCE_TYPE(&&, &);
TEST_NOT_DIFFERENCE_TYPE(&&, const&);
TEST_NOT_DIFFERENCE_TYPE(&&, volatile&);
TEST_NOT_DIFFERENCE_TYPE(&&, const volatile&);
TEST_NOT_DIFFERENCE_TYPE(&&, &&);
TEST_NOT_DIFFERENCE_TYPE(&&, const&&);
TEST_NOT_DIFFERENCE_TYPE(&&, volatile&&);
TEST_NOT_DIFFERENCE_TYPE(&&, const volatile&&);
TEST_NOT_DIFFERENCE_TYPE(const&&, &);
TEST_NOT_DIFFERENCE_TYPE(const&&, const&);
TEST_NOT_DIFFERENCE_TYPE(const&&, volatile&);
TEST_NOT_DIFFERENCE_TYPE(const&&, const volatile&);
TEST_NOT_DIFFERENCE_TYPE(const&&, &&);
TEST_NOT_DIFFERENCE_TYPE(const&&, const&&);
TEST_NOT_DIFFERENCE_TYPE(const&&, volatile&&);
TEST_NOT_DIFFERENCE_TYPE(const&&, const volatile&&);
TEST_NOT_DIFFERENCE_TYPE(volatile&&, &);
TEST_NOT_DIFFERENCE_TYPE(volatile&&, const&);
TEST_NOT_DIFFERENCE_TYPE(volatile&&, volatile&);
TEST_NOT_DIFFERENCE_TYPE(volatile&&, const volatile&);
TEST_NOT_DIFFERENCE_TYPE(volatile&&, &&);
TEST_NOT_DIFFERENCE_TYPE(volatile&&, const&&);
TEST_NOT_DIFFERENCE_TYPE(volatile&&, volatile&&);
TEST_NOT_DIFFERENCE_TYPE(volatile&&, const volatile&&);
TEST_NOT_DIFFERENCE_TYPE(const volatile&&, &);
TEST_NOT_DIFFERENCE_TYPE(const volatile&&, const&);
TEST_NOT_DIFFERENCE_TYPE(const volatile&&, volatile&);
TEST_NOT_DIFFERENCE_TYPE(const volatile&&, const volatile&);
TEST_NOT_DIFFERENCE_TYPE(const volatile&&, &&);
TEST_NOT_DIFFERENCE_TYPE(const volatile&&, const&&);
TEST_NOT_DIFFERENCE_TYPE(const volatile&&, volatile&&);
TEST_NOT_DIFFERENCE_TYPE(const volatile&&, const volatile&&);
TEST_NOT_DIFFERENCE_TYPE(&, /*by value*/);
TEST_NOT_DIFFERENCE_TYPE(const&, /*by value*/);
TEST_NOT_DIFFERENCE_TYPE(volatile&, /*by value*/);
TEST_NOT_DIFFERENCE_TYPE(&&, /*by value*/);
TEST_NOT_DIFFERENCE_TYPE(const&&, /*by value*/);
TEST_NOT_DIFFERENCE_TYPE(volatile&&, /*by value*/);
TEST_NOT_DIFFERENCE_TYPE(const volatile&&, /*by value*/);
TEST_NOT_DIFFERENCE_TYPE(/*by value*/, &);
TEST_NOT_DIFFERENCE_TYPE(/*by value*/, const&);
TEST_NOT_DIFFERENCE_TYPE(/*by value*/, volatile&);
TEST_NOT_DIFFERENCE_TYPE(/*by value*/, &&);
TEST_NOT_DIFFERENCE_TYPE(/*by value*/, const&&);
TEST_NOT_DIFFERENCE_TYPE(/*by value*/, volatile&&);
TEST_NOT_DIFFERENCE_TYPE(/*by value*/, const volatile&&);

WDYT? If we have to do this sort of thing frequently, we could also bundle a few utilities under support/ to reduce duplication further.

This revision now requires changes to proceed.Apr 5 2021, 3:06 PM

I'd like to put an end to comments about "too many tests" once and for all. I think we all agree that test coverage is a good property to have. I think we also agree that readability is a good property to have, and that code duplication is bad. We want both, as much as possible.

[...]

I really like your suggestion. The only two things I'd say are

  1. We could take that even further and have a test macro that generates all the const/ref/volatile combinations we want (this is sort of like what you were saying with the two loops). Then we could just pass it a macro that generated the tests for any given concept.
  2. These tests are good because they cover as much code as possible, but they make it harder to verify what's actually being tested (especially if it's two macros deep). I'd suggest that we have the macro tests and some simple, easily verifyable tests that verify the basic/common case.

Using macros like this will also make it easy to write lots of tests quickly, so it shouldn't be too hard to back port to the other concepts that have already landed.

cjdb updated this revision to Diff 335404.Apr 5 2021, 10:36 PM

applies @ldionne's proposed test framework

cjdb added a comment.Apr 5 2021, 10:37 PM

I'd like to put an end to comments about "too many tests" once and for all. I think we all agree that test coverage is a good property to have. I think we also agree that readability is a good property to have, and that code duplication is bad. We want both, as much as possible.

I think what we're seeing here is an example where we have a lot of code coverage, which is great, however the cost (right now) is a lot of repetition, and hence decreased readability. For me, the solution is not to remove the test coverage, but instead to implement it without reducing readability so much. Here's an example of how this can be done (there are other ways, for example we could use templates instead but I just hacked this together). This replaces lines 174 to 702. Yes it's still not *small*, but it's much better and IMO easier to read, while not dropping any of the coverage. And it would be easy to make it even better by using two nested "loops" over all qualifiers, but it's unclear to me that would actually increase readability.

...

WDYT? If we have to do this sort of thing frequently, we could also bundle a few utilities under support/ to reduce duplication further.

Now that I know we're not afraid of macros, I'll employ this where large tests are necessary.

libcxx/include/__config
851

Please use <= 17, for consistency with similar lines in this file.

Done.

(We tend to guard C++2a stuff on > 17. I don't _think_ there's anything special about the Ranges stuff in this respect, right?)

Not sure I'm following this line of thought, so I'm leaving the comment as unresolved for now.

libcxx/include/iterator
474

In general: yes please.

libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.pass.cpp
34–35 ↗(On Diff #335200)

Note that T const is the same type as T whenever T is a reference type. Therefore, any place in this file that instantiates check_incrementable_traits<Foo&> is suspicious — it's simply static-asserting that the difference type of T is the difference type of T.
I suggest that you add static_assert(!std::is_reference_v<T>) inside this template, and then re-run the tests, and investigate/fix any failures that arise from that.

Lines 63-70 should already account for this, unless you're asking we check T& against T const& and friends.

I also suggest that if you actually expect result to be true in all cases, you should add

assert(result);

inside this function. (Or is it okay if result is false, sometimes? I didn't closely examine all the call sites.)

Lines 89 and 90-1 expect false.

I really like @ldionne suggestion. I am a bit unhappy about it being a macro, but meh.

FYI, this is the MSVC test set which is kind of the same but less extensive:

https://github.com/microsoft/STL/blob/fb2f89f9366b1c170587ff37e39b962dd8ead3d6/tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp#L667-L721

This still LGTM. I think we should land this to get stuff unblocked! (After another reviewer approves.)

libcxx/include/__config
851

Not sure I'm following this line of thought, so I'm leaving the comment as unresolved for now.

I don't think this is needed anymore, because C++20 has been released. We did this pre-c++20 (when it was still C++2a) because we weren't sure it would be released in C++20 (i.e., if there was a global pandemic...). But, it was still released on time, and it is C++20, so I think we can guard on that now.

libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.compile.pass.cpp
150

What's the comment for?

246

What's going on here? Why is this commented?

cjdb added inline comments.Apr 8 2021, 10:55 AM
libcxx/include/__config
851

I still don't understand, sorry 😭

libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.compile.pass.cpp
150

Paging @ldionne. He's not the only Boost dev I've seen do this.

246

I think it would be best to keep the commented out lines there to show these permutations weren't forgotten, but they can't be enabled because they actually pass.

Mordante added inline comments.Apr 8 2021, 11:05 AM
libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.compile.pass.cpp
150

Isn't it because line 148 ends with a \? Just to avoid when writing code on line 149 to be part of the macro.

cjdb added inline comments.Apr 8 2021, 11:06 AM
libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.compile.pass.cpp
150

Yeah, but why does line 148 end with \?

ldionne accepted this revision.Apr 8 2021, 12:58 PM

Ship it with comments applied and CI passing!

libcxx/include/__config
852

FWIW, I'm fine with defining this for now, however as soon as we only support compilers that implement concepts, we will get rid of this and instead use just #if _LIBCPP_STD_VER >= 20 to guard concepts code. Agreed?

libcxx/include/iterator
463

IMO this name is misleading because you're really checking whether T has an integral minus AND no member difference type. I think it would be cleaner to take the !__has_member_difference_type out of this helper concept and use it directly below in incrementable_traits.

libcxx/test/std/iterators/iterator.requirements/iterator.assoc.types/incrementable.traits/incrementable_traits.compile.pass.cpp
150

Good question, I do it as a reflex now. I'm not sure where it came from originally. I know at some point I had a tool to automatically add \ at the end of macro lines and it would be keyed on /**/ to stop IIRC, but I'm not sure why we do this historically.

154

You could use this common idiom to show that you explicitly wanted the macro to expand to nothing, and you didn't forget to escape the line break with \:

#define NO_QUALIFIER /*nothing*/
This revision is now accepted and ready to land.Apr 8 2021, 12:58 PM
cjdb updated this revision to Diff 336692.Apr 11 2021, 2:34 PM

rebases to activate CI

This revision was automatically updated to reflect the committed changes.
cjdb marked an inline comment as done.
thakis added inline comments.
libcxx/include/iterator
446

This breaks using this header with -std=c++14 apparently: http://45.33.8.238/linux/43983/step_4.txt

Looks like _LIBCPP_HAS_NO_RANGES is supposed to be defined in that case, but maybe it doesn't work right?

thakis added inline comments.Apr 13 2021, 6:00 AM
libcxx/include/iterator
446

Oh wait, this might be a bot config problem. Please ignore for now.

thakis added inline comments.Apr 13 2021, 6:13 AM
libcxx/include/iterator
446

Yes, was a problem on my end. All good now, sorry for the noise.