[libcxx-commits] [PATCH] D151900: [libc++][NFC] Create a new folder for config-related headers.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

In D151900#4402597 <https://reviews.llvm.org/D151900#4402597>, @var-const wrote:

> In D151900#4387750 <https://reviews.llvm.org/D151900#4387750>, @Mordante wrote:
>
>> Could you add some information in the commit message why this is a good idea?
>
> Added some comments to the patch description, please let me know what you think. This came out of my first attempts to add macros for configuring hardening modes. I find `__config` hard to navigate due to its size, and it seems to have a few "themes" that seem like a good fit to be split into separate, more focused headers. I thought to refactor `__config` first before adding even more stuff to it. I also hope that having more focused headers (e.g. `__config_dir/language_support.h`, `__config_dir/abi.h`) would make finding macros easier.

Thanks! I agree the file is somewhat large and hard to navigate.

I think granularizing this header has some pitfalls as mention in my comment, but I think we can find ways to guard against them.



================
Comment at: libcxx/include/__config_dir/base.h:12
+#define _LIBCPP___CONFIG_DIR_BASE_H
 
 #include <__config_site>
----------------
I actually would rather like to see how you envision the final split, this is the normal way how we granularize headers.

I also think we should never include one of the granularized headers from headers outside of `__config/`, except from `config`. Here I feel that including the granularized headers could easily result in bugs. A define not set might be on purpose or a bug due to forgetting an include.
So I think we need generated tests verify tests that give an error when I write code like

```
#include <__config/constexpr.h>
```
>From a header like `<format>`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151900