[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