[libcxx-commits] [PATCH] D115578: [libc++][NFC] Granularize <filesystem>

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Dec 13 09:08:04 PST 2021


Quuxplusone added a comment.

In D115578#3188839 <https://reviews.llvm.org/D115578#3188839>, @ldionne wrote:

> Given that most filesystem operations simply make a call to the dylib and they all share almost the same dependencies, it seems overkill to me to split them in separate files. Instead, I'd rather have one header that contains all the operations, something like `__filesystem/operations.h`. WDYT?

FWIW, I wouldn't object to that. But (1) it's less clearly defined, compared to the "one file per overload set" rule; and (2) some of these overload sets do have 2, 3, or 4 members, so at least it's not like the granular files are //laughably// trivial. :)
Yet another option would be to pack all the dylib functions' prototypes together in e.g. `<__filesystem/fwd.h>` (I can't think of a good name), and then including `fwd.h` from all the granular headers. However, sprinkling them one-by-one throughout the files is essentially the same thing we do for the underscore versions of algorithms in `<algorithm>` — e.g. we put `__push_heap` in the same header with `push_heap`, albeit for good perf reasons in that case — so there's a //sort of// precedent for declaring e.g. `__weakly_canonical` in the same header with `weakly_canonical`.
In any case, my understanding is that the failure mode we're trying to protect against is that we accidentally forget one of the dylib functions, or we slip in an extra prototype that doesn't exist in the dylib, or something like that. (In fact, that seems to have already happened, with `__system_complete`.)

In short: *shrug*.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115578



More information about the libcxx-commits mailing list