[libcxx-commits] [PATCH] D136268: [libc++][ranges] implement `std::views::elements_view`

Hui via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Fri Nov 18 12:47:11 PST 2022


huixie90 added inline comments.


================
Comment at: libcxx/include/__iterator/concepts/can_reference.h:2
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
----------------
philnik wrote:
> huixie90 wrote:
> > philnik wrote:
> > > I don't think we should granularize `__iterator/concepts.h`. Compiling the header takes just ~35ms, 20 of which are including `<type_traits>`. So we'd save maybe 10ms by granularizing it, which I don't think is worth it. `__iterator/concepts.h` also isn't very large at ~300 LoC, so it also doesn't increase readability much.
> > I wasn't too concerned about compilation time. Without doing this, we will add public dependency from `<tuple>` to `<concepts>` (see description). I'd like to avoid adding dependencies like this so lifted several headers required by `subrange`'s forward declaration
> I'm not sure what exactly you mean. `__iterator/concepts.h` doesn't include `<concepts>`. Did this change between the creation of this patch and current trunk?
Yes. things changed a lot since the creation of this patch. I think the main change is this patch
https://github.com/llvm/llvm-project/commit/89b356f05ab7aa3d96fc7b68aece6e7a5bdb0db5

After this patch, I guess the dependency is removed. I will try rebase and see what happens in the CI


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136268



More information about the libcxx-commits mailing list