[libcxx-commits] [PATCH] D101098: [libcxx][docs] Add section for header layout and guidlines.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/docs/Contributing.rst:70-71
+  ``__memory/smart_pointers/shared_ptr.h``.
+* All sub-directories should be mangled with a double underscore. Conversely, headers
+  that live in these directories should not be mangled.
+* Headers should contain specific features. A header such as ``__detail/utils.h`` might
----------------
Stronger: The current naming convention is exactly `__<toplevelheadername>/<featurename>.h`. `toplevelheadername` must be the exact name of whatever header the Standard requires to contain this feature, e.g. `string` or `ranges` or `memory`. `featurename` should be the exact public identifier of the feature defined in this header, e.g. `iterator_traits` or `construct_at`. Exceptions exist (e.g. //maybe// `__iterator/iterator_tags.h`; `__functional/ops.h`) but I expect those exceptions are very rare.

If we enforce the general rule "Name the header after what it defines," then we successfully prevent most attempts to introduce "__detail/utils.h" (because `detail` is not the name of a top-level header and `utils` is not a public identifier).


================
Comment at: libcxx/docs/Contributing.rst:75-78
+* All sub-headers should be included by their parent header. To use the example above,
+  it is imperative that ``<ranges>`` includes the sub-header ``__ranges/size.h``. Or, to use
+  another example, ``<iterator>`` must include ``__iterator/iterator_traits.h``, even if it
+  doesn't use ``iterator_traits`` itself.
----------------
Someone likes the word "imperative" lately. :)
I suggest: All libc++ headers, both public and helper headers, should "Include What You Use." You should never rely on some other header to transitively include something that you need. If your header needs a definition of `iterator_traits`, then you should either `#include <iterator>` (exactly as user code does), or, to improve compile speed, `#include <__iterator/iterator_traits.h>`. You should never expect `iterator_traits` to be "pulled in" by any other header.
User code, on the other hand, should never `#include` any of these helper headers directly; they are intended as unstable implementation details of libc++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101098