[libc] Make two include/llvm-libc-types/ headers more self-contained.

[vonosmas]

• imaxdiv_t.h needs intmax_t. Get it from <stdint.h> directly instead of relying on prior inclusion inside generated inttypes.h.
• include <termios-macros.h> for struct termios to get the #defined constant. We do allow inclusion of macro headers from types headers in other places.

[llvmbot]

@llvm/pr-subscribers-libc

Author: Alexey Samsonov (vonosmas)

Changes

• imaxdiv_t.h needs intmax_t. Get it from <stdint.h> directly instead of relying on prior inclusion inside generated inttypes.h.
• include <termios-macros.h> for struct termios to get the #defined constant. We do allow inclusion of macro headers from types headers in other places.

---

Full diff: https://github.com/llvm/llvm-project/pull/128094.diff

4 Files Affected:

• (modified) libc/include/inttypes.h.def (-1)
• (modified) libc/include/llvm-libc-types/CMakeLists.txt (+8-1)
• (modified) libc/include/llvm-libc-types/imaxdiv_t.h (+2)
• (modified) libc/include/llvm-libc-types/struct_termios.h (+2)

diff --git a/libc/include/inttypes.h.def b/libc/include/inttypes.h.def
index 5879d2d8e0410..61a6c0cf7e473 100644
--- a/libc/include/inttypes.h.def
+++ b/libc/include/inttypes.h.def
@@ -11,7 +11,6 @@
 
 #include "__llvm-libc-common.h"
 #include "llvm-libc-macros/inttypes-macros.h"
-#include <stdint.h>
 
 %%public_api()
 
diff --git a/libc/include/llvm-libc-types/CMakeLists.txt b/libc/include/llvm-libc-types/CMakeLists.txt
index 7ed69ab1af6d9..b4264db76745f 100644
--- a/libc/include/llvm-libc-types/CMakeLists.txt
+++ b/libc/include/llvm-libc-types/CMakeLists.txt
@@ -99,7 +99,14 @@ add_header(tss_dtor_t HDR tss_dtor_t.h)
 add_header(__atexithandler_t HDR __atexithandler_t.h)
 add_header(speed_t HDR speed_t.h)
 add_header(tcflag_t HDR tcflag_t.h)
-add_header(struct_termios HDR struct_termios.h DEPENDS .cc_t .speed_t .tcflag_t)
+add_header(
+    struct_termios
+  HDR
+    struct_termios.h
+  DEPENDS
+    .cc_t .speed_t .tcflag_t
+    libc.include.llvm-libc-macros.termios_macros
+)
 add_header(__getoptargv_t HDR __getoptargv_t.h)
 add_header(wchar_t HDR wchar_t.h)
 add_header(char8_t HDR char8_t.h)
diff --git a/libc/include/llvm-libc-types/imaxdiv_t.h b/libc/include/llvm-libc-types/imaxdiv_t.h
index 5062b643065a7..bbfdfd3375095 100644
--- a/libc/include/llvm-libc-types/imaxdiv_t.h
+++ b/libc/include/llvm-libc-types/imaxdiv_t.h
@@ -9,6 +9,8 @@
 #ifndef __LLVM_LIBC_TYPES_IMAXDIV_T_H__
 #define __LLVM_LIBC_TYPES_IMAXDIV_T_H__
 
+#include <stdint.h>
+
 typedef struct {
   intmax_t quot;
   intmax_t rem;
diff --git a/libc/include/llvm-libc-types/struct_termios.h b/libc/include/llvm-libc-types/struct_termios.h
index e3c5f2809e439..aad53eb9cb75e 100644
--- a/libc/include/llvm-libc-types/struct_termios.h
+++ b/libc/include/llvm-libc-types/struct_termios.h
@@ -13,6 +13,8 @@
 #include "speed_t.h"
 #include "tcflag_t.h"
 
+#include "../llvm-libc-macros/termios-macros.h" // NCCS
+
 struct termios {
   tcflag_t c_iflag; // Input mode flags
   tcflag_t c_oflag; // Output mode flags

[michaelrj-google]

overall LGTM with one change

[michaelrj-google]

the C standard says "The header <inttypes.h> includes the header <stdint.h> and extends it with additional facilities provided by hosted implementations."

[frobtech]

I think this is a good opportunity to add another little hdrgen feature and then eliminate inttypes.h.def.
That is, we can have an includes list in YAML, where this is the list of public <foo.h> headers that the actual API spec is for this other public header to include for re-export. I think there may be other cases than just <inttypes.h> -> <stdint.h> in standard C though I can't think of one off hand; there are certainly more in POSIX.
These should be in the source of truth about the API contract, and thus the actual #include can be generated. (Later we can exploit this in other ways like with generated tests.)

[vonosmas]

In this specific case though, helpful hdrgen already inserts #include <stdint.h> statement to the generated header: because intmax_t is among the types returned by the functions in inttypes.h.def. See:

llvm-project/libc/utils/hdrgen/header.py

Line 31 in fd5d1cb

COMPILER_HEADER_TYPES.update({f"int{size}_t": "<stdint.h>" for size in STDINT_SIZES})

Given that, is it OK to still remove this explicit include?

[frobtech]

• imaxdiv_t.h needs intmax_t. Get it from <stdint.h> directly instead of relying on prior inclusion inside generated inttypes.h.

This is fine. In general we should not have transitive includes from llvm-libc-types headers. But the compiler-provided headers are a special exception to that (<stddef.h>, <stdint.h>, <stdarg.h>).

• include <termios-macros.h> for struct termios to get the #defined constant. We do allow inclusion of macro headers from types headers in other places.

I don't think we should do this, for two reasons.

1. Name space: the point of per-type headers is that define only what must be defined to make that type complete. There may well be places where the type must be available, but the public header isn't supposed to define all the macros that are associated with that type's full API.
2. Build management: it's currently manually plumbed in CMake what headers rely on which others; but it's not that way in the Fuchsia build, and eventually we should find a way to move the CMake build in the same direction. That is, pretty much all the dependencies between headers go from a root public <foo.h> header directly to a set of implementation headers in llivm-libc-types and llvm-libc-macros. The YAML files are the source of truth that produces all the #include directives in a fully-generated header (no .h.def file, which is where we want to get nearly all of the headers to eventually). This lets us use automation to manage the build and installation rules for all the headers, rather than manual maintenance of lists of the implementation headers used by the public headers.

So the contrary change I would recommend here is to remove termios.h.def and its header_template line in termios.yaml.

[vonosmas]

I don't think we should do this, for two reasons.

1. Name space: the point of per-type headers is that define only what must be defined to make that type complete. There may well be places where the type must be available, but the public header isn't supposed to define all the macros that are associated with that type's full API.

So, basically, you're saying the current logic where the termios.h header only works because llvm-libc-types/struct_termios.h is included after llvm-libc-macros/termios-macros.h (which defines the necessary macros) is "working as intended"? (note that we do have a somewhat reverse case in stdbit.h header, which only works if llvm-libc-macros/stdbit-macros.h is included at the very bottom, as the macro there relies on function declarations - see PR 128101).

If that's the case then should we essentially treat everything under both llvm-libc-types/ and llvm-libc-macros/ as "textual headers" which are not always expected to work / be analyzed individually? I'm asking this in context of Bazel rules for headers, which we expect to start working on soon.

2. Build management: it's currently manually plumbed in CMake what headers rely on which others; but it's not that way in the Fuchsia build, and eventually we should find a way to move the CMake build in the same direction. That is, pretty much all the dependencies between headers go from a root public <foo.h> header directly to a set of implementation headers in llivm-libc-types and llvm-libc-macros. The YAML files are the source of truth that produces all the #include directives in a fully-generated header (no .h.def file, which is where we want to get nearly all of the headers to eventually). This lets us use automation to manage the build and installation rules for all the headers, rather than manual maintenance of lists of the implementation headers used by the public headers.

So the contrary change I would recommend here is to remove termios.h.def and its header_template line in termios.yaml.

Acknowledged, so you it looks like you're moving in the direction of libc/include/fenv.yaml, which has no .h.def file and declares all the macro it needs in YAML itself? That's fine with me, but I'd prefer to make this type of migration in a different PR, this one was aimed at making llvm-libc-types/ headers "work", which may not be a worthy goal at all (see comment above).

[vonosmas]

@frobtech - could you please confirm that we shouldn't assume that headers under both include/llvm-libc-types and include/llvm-libc-macros are not expected to work on their own?