[libcxx-commits] [PATCH] D123600: [libc++][ranges] Implement `views::take`.

Nikolas Klauser via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Apr 19 10:46:25 PDT 2022


philnik added inline comments.


================
Comment at: libcxx/include/__fwd/string_view.h:10
+
+#ifndef _LIBCPP_FWD_STRING_VIEW_H
+#define _LIBCPP_FWD_STRING_VIEW_H
----------------
Just so you don't miss them when moving the headers.


================
Comment at: libcxx/include/__fwd/string_view.h:1
+// -*- C++ -*-
+//===---------------------------------------------------------------------===//
----------------
var-const wrote:
> huixie90 wrote:
> > philnik wrote:
> > > huixie90 wrote:
> > > > ldionne wrote:
> > > > > philnik wrote:
> > > > > > `__string_view/fwd.h` would make more sense I think. Same for `span`.
> > > > > FWIW I don't really have a strong opinion. The only downside to this is that we might end up with a lot of one-header subdirectories if we start adding forward declarations to more classes, and putting everything under `__fwd/` would avoid that.
> > > > I am in favour of fwd folder. I don’t like to have too many files with the exact same name fwd.hpp 
> > > I don't like having a `__fwd` subdirectory because that would mean we have a single header spread across multiple subdirecories. That makes it a lot harder to see what is part of a header.
> > I think having a single fwd folder whose subdirectories hierarchies follow the non-fwd hierarchies is pretty neat. You can easily find the forward declarations by adding a fwd at the beginning. One example code base is Boost.Hana 
> I'm slightly in favor of having all the forward declarations under a single folder because it would make it easier to see what can and cannot be forward-declared. While having contents of a single header spread across several directories in general would be bad, I think splitting out forward declarations following a consistent pattern shouldn't cause any issues. @philnik How strongly do you feel about this?
The reason you want to forward declare the things here is to avoid including all of the header and having longer compile times, right? If there is another technical reason for that, please tell me! So there shouldn't be any reason for new headers to have them in 2-3 years (hopefully).  I don't think there will be a lot of new forward declared headers because of this, so I think having forward declarations in another directory is quite surprising.
So yeah, I do feel quite strongly about this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123600



More information about the libcxx-commits mailing list