Details
- Reviewers
ldionne cjdb - Group Reviewers
Restricted Project - Commits
- rGe0adf7e06a9e: [libc++][NFC] Move incrementable_traits and indirectly_readable_traits into…
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
LGTM with fixes applied and CI passing. Thanks!
libcxx/include/__iterator/incrementable_traits.h | ||
---|---|---|
1 ↗ | (On Diff #338214) | This needs a license. What I generally do is copy-paste an existing header and replace the contents. That makes sure I get the right boilerplate (not that we shouldn't work on reducing that boilerplate if possible, especially the pragma stuff). |
libcxx/include/__iterator/readable_traits.h | ||
16 ↗ | (On Diff #338214) | It would make sense to hoist that out of this file and into <iterator>. Same for incrementable traits. |
libcxx/include/__iterator/readable_traits.h | ||
---|---|---|
16 ↗ | (On Diff #338214) | Eh, I don't know. I'd be OK doing that and adding an #error here if it's not defined. I'd kind of like all our headers to always be includable, though. |
libcxx/include/__iterator/readable_traits.h | ||
---|---|---|
16 ↗ | (On Diff #338214) | Ok, this is non-blocking anyway, feel free to leave as-is. Actually when we only support compilers that support concepts, this will become #if C++ >= 20, which we should leave in this header. So it makes sense to keep. |
libcxx/include/__iterator/readable_traits.h | ||
---|---|---|
16 ↗ | (On Diff #338214) | SGTM. |
I accidently updated this patch instead of D100686. So, I'm just putting back the correct patch here.
libcxx/include/__iterator/concepts.h | ||
---|---|---|
1 ↗ | (On Diff #338245) | I made a mistake: putting them in this header will create circular dependencies, since iterator_traits needs these exposition-only concepts and __iterator_traits/concepts.h will depends on iterator_traits. They, along with the cpp17-*-iterator concepts should either be in __iterator/iterator_traits.h. |