[RFC][libc] Codify header inclusion policy by nickdesaulniers · Pull Request #87017 · llvm/llvm-project (original) (raw)

@llvm/pr-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

Changes

This RFC is meant to spur discussion; it's not yet clear if this policy is best (or even makes sense).

When supporting "overlay" vs "fullbuild" modes, "what ABI are you using?" becomes a fundamental question to have concrete answers for. Overlay mode MUST match the ABI of the system being overlayed onto; fullbuild more flexible (the only system ABI relevant is the OS kernel).

When implementing llvm-libc we generally prefer the include-what-you use style of avoiding transitive dependencies (since that makes refactoring headers more painful, and slows down build times). So what header do you include for any given type or function declaration? For any given userspace program, the answer is straightforward. But for llvm-libc which is trying to support multiple ABIs (at least one per configuration), the answer is perhaps less clear.

This proposal seeks to add one layer of indirection relative to what's being done today.

It then converts users of sigset_t and struct epoll_event and the epoll implemenations over to this convention as an example.


Patch is 23.10 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/87017.diff

31 Files Affected:

-#include <signal.h> +#include "src/errno/libc_errno.h" +#include "src/signal/linux/signal_utils.h" +#include "src/signal/sigset_t.h" namespace LIBC_NAMESPACE { diff --git a/libc/src/signal/linux/sigaddset.cpp b/libc/src/signal/linux/sigaddset.cpp index 536391734e0587..f1264d10971d4e 100644 --- a/libc/src/signal/linux/sigaddset.cpp +++ b/libc/src/signal/linux/sigaddset.cpp @@ -7,11 +7,11 @@ //===----------------------------------------------------------------------===// #include "src/signal/sigaddset.h" + #include "src/__support/common.h" #include "src/errno/libc_errno.h" #include "src/signal/linux/signal_utils.h"

-#include <signal.h> +#include "src/signal/sigset_t.h" namespace LIBC_NAMESPACE { diff --git a/libc/src/signal/linux/sigdelset.cpp b/libc/src/signal/linux/sigdelset.cpp index 5cb645e461cf8e..1eed6d1501b58b 100644 --- a/libc/src/signal/linux/sigdelset.cpp +++ b/libc/src/signal/linux/sigdelset.cpp @@ -7,11 +7,11 @@ //===----------------------------------------------------------------------===// #include "src/signal/sigdelset.h" + #include "src/__support/common.h" #include "src/errno/libc_errno.h" #include "src/signal/linux/signal_utils.h"

-#include <signal.h> +#include "src/signal/sigset_t.h" namespace LIBC_NAMESPACE { diff --git a/libc/src/signal/linux/sigfillset.cpp b/libc/src/signal/linux/sigfillset.cpp index e17c85a897ce74..ccf9ced10024c8 100644 --- a/libc/src/signal/linux/sigfillset.cpp +++ b/libc/src/signal/linux/sigfillset.cpp @@ -7,11 +7,11 @@ //===----------------------------------------------------------------------===// #include "src/signal/sigfillset.h" + #include "src/__support/common.h" #include "src/errno/libc_errno.h" #include "src/signal/linux/signal_utils.h"

-#include <signal.h> +#include "src/signal/sigset_t.h" namespace LIBC_NAMESPACE { diff --git a/libc/src/signal/linux/signal_utils.h b/libc/src/signal/linux/signal_utils.h index 5e9dd9a5c53ab0..30ae0b95f678f8 100644 --- a/libc/src/signal/linux/signal_utils.h +++ b/libc/src/signal/linux/signal_utils.h @@ -11,8 +11,9 @@ #include "src/__support/OSUtil/syscall.h" // For internal syscall function. #include "src/__support/common.h" +#include "src/signal/sigset_t.h" -#include <signal.h> +#include <signal.h> // sigaction #include <stddef.h> #include <sys/syscall.h> // For syscall numbers. diff --git a/libc/src/signal/linux/sigprocmask.cpp b/libc/src/signal/linux/sigprocmask.cpp index 79a35dd59d75c8..f347f28a8f5d06 100644 --- a/libc/src/signal/linux/sigprocmask.cpp +++ b/libc/src/signal/linux/sigprocmask.cpp @@ -7,13 +7,13 @@ //===----------------------------------------------------------------------===// #include "src/signal/sigprocmask.h" + #include "src/__support/OSUtil/syscall.h" // For internal syscall function. +#include "src/__support/common.h" #include "src/errno/libc_errno.h" #include "src/signal/linux/signal_utils.h" +#include "src/signal/sigset_t.h" -#include "src/__support/common.h"

-#include <signal.h> #include <sys/syscall.h> // For syscall numbers. namespace LIBC_NAMESPACE { diff --git a/libc/src/signal/sigaddset.h b/libc/src/signal/sigaddset.h index 626eb20a295c83..1e57609b501d5f 100644 --- a/libc/src/signal/sigaddset.h +++ b/libc/src/signal/sigaddset.h @@ -9,7 +9,7 @@ #ifndef LLVM_LIBC_SRC_SIGNAL_SIGADDSET_H #define LLVM_LIBC_SRC_SIGNAL_SIGADDSET_H -#include <signal.h> +#include "src/signal/sigset_t.h" namespace LIBC_NAMESPACE { diff --git a/libc/src/signal/sigdelset.h b/libc/src/signal/sigdelset.h index c4fdb9975fa3d0..649546baa98845 100644 --- a/libc/src/signal/sigdelset.h +++ b/libc/src/signal/sigdelset.h @@ -9,7 +9,7 @@ #ifndef LLVM_LIBC_SRC_SIGNAL_SIGDELSET_H #define LLVM_LIBC_SRC_SIGNAL_SIGDELSET_H -#include <signal.h> +#include "src/signal/sigset_t.h" namespace LIBC_NAMESPACE { diff --git a/libc/src/signal/sigemptyset.h b/libc/src/signal/sigemptyset.h index f3763d1f4f3d44..0f7f87cef28cd4 100644 --- a/libc/src/signal/sigemptyset.h +++ b/libc/src/signal/sigemptyset.h @@ -9,7 +9,7 @@ #ifndef LLVM_LIBC_SRC_SIGNAL_SIGEMPTYSET_H #define LLVM_LIBC_SRC_SIGNAL_SIGEMPTYSET_H -#include <signal.h> +#include "src/signal/sigset_t.h" namespace LIBC_NAMESPACE { diff --git a/libc/src/signal/sigfillset.h b/libc/src/signal/sigfillset.h index d8e3168871ea8a..35500f1f792bde 100644 --- a/libc/src/signal/sigfillset.h +++ b/libc/src/signal/sigfillset.h @@ -9,7 +9,7 @@ #ifndef LLVM_LIBC_SRC_SIGNAL_SIGFILLSET_H #define LLVM_LIBC_SRC_SIGNAL_SIGFILLSET_H -#include <signal.h> +#include "src/signal/sigset_t.h" namespace LIBC_NAMESPACE { diff --git a/libc/src/signal/sigprocmask.h b/libc/src/signal/sigprocmask.h index e0658860579e4e..fea8e8ca82a6b1 100644 --- a/libc/src/signal/sigprocmask.h +++ b/libc/src/signal/sigprocmask.h @@ -9,7 +9,7 @@ #ifndef LLVM_LIBC_SRC_SIGNAL_SIGPROCMASK_H #define LLVM_LIBC_SRC_SIGNAL_SIGPROCMASK_H -#include <signal.h> +#include "src/signal/sigset_t.h" namespace LIBC_NAMESPACE { diff --git a/libc/src/signal/sigset_t.h b/libc/src/signal/sigset_t.h new file mode 100644 index 00000000000000..60f9eeeb8aeb8a --- /dev/null +++ b/libc/src/signal/sigset_t.h @@ -0,0 +1,15 @@ +// TODO: license block +#ifndef LLVM_LIBC_SRC_SIGNAL_SIGSET_T_H +#define LLVM_LIBC_SRC_SIGNAL_SIGSET_T_H + +#ifdef LIBC_FULL_BUILD + +#include "include/llvm-libc-types/sigset_t.h" + +#else + +#include <signal.h> + +#endif // LIBC_FULL_BUILD + +#endif // LLVM_LIBC_SRC_SIGNAL_SIGSET_T_H diff --git a/libc/src/sys/epoll/CMakeLists.txt b/libc/src/sys/epoll/CMakeLists.txt index d4991a238e2a77..d0efeb9a588c73 100644 --- a/libc/src/sys/epoll/CMakeLists.txt +++ b/libc/src/sys/epoll/CMakeLists.txt @@ -2,6 +2,14 @@ if(EXISTS CMAKECURRENTSOURCEDIR/{CMAKE_CURRENT_SOURCE_DIR}/CMAKECURRENTSOURCEDIR/{LIBC_TARGET_OS}) add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS}) endif() +add_header_library( + struct_epoll_event + HDRS + struct_epoll_event.h + DEPENDS + libc.include.llvm-libc-types.struct_epoll_event +) + add_entrypoint_object( epoll_wait ALIAS diff --git a/libc/src/sys/epoll/epoll_pwait.h b/libc/src/sys/epoll/epoll_pwait.h index 9dcb55533009f9..1b832718de8a28 100644 --- a/libc/src/sys/epoll/epoll_pwait.h +++ b/libc/src/sys/epoll/epoll_pwait.h @@ -9,11 +9,8 @@ #ifndef LLVM_LIBC_SRC_SYS_EPOLL_EPOLL_PWAIT_H #define LLVM_LIBC_SRC_SYS_EPOLL_EPOLL_PWAIT_H -// TODO: Use this include once the include headers are also using quotes. -// #include "include/llvm-libc-types/sigset_t.h" -// #include "include/llvm-libc-types/struct_epoll_event.h"

-#include <sys/epoll.h> +#include "src/signal/sigset_t.h" +#include "src/sys/epoll/struct_epoll_event.h" namespace LIBC_NAMESPACE { diff --git a/libc/src/sys/epoll/epoll_pwait2.h b/libc/src/sys/epoll/epoll_pwait2.h index 622ede6a0f9f9a..3fd598f73f993f 100644 --- a/libc/src/sys/epoll/epoll_pwait2.h +++ b/libc/src/sys/epoll/epoll_pwait2.h @@ -9,18 +9,15 @@ #ifndef LLVM_LIBC_SRC_SYS_EPOLL_EPOLL_PWAIT2_H #define LLVM_LIBC_SRC_SYS_EPOLL_EPOLL_PWAIT2_H -// TODO: Use this include once the include headers are also using quotes. -// #include "include/llvm-libc-types/sigset_t.h" -// #include "include/llvm-libc-types/struct_epoll_event.h" -// #include "include/llvm-libc-types/struct_timespec.h"

-#include <sys/epoll.h> +#include "src/signal/sigset_t.h" +#include "src/sys/epoll/struct_epoll_event.h" +#include "src/time/struct_timespec.h" namespace LIBC_NAMESPACE { // TODO: sigmask and timeout should be nullable -int epoll_pwait2(int epfd, epoll_event *events, int maxevents, - const timespec *timeout, const sigset_t *sigmask); +int epoll_pwait2(int epfd, struct epoll_event *events, int maxevents, + const struct timespec *timeout, const sigset_t *sigmask); } // namespace LIBC_NAMESPACE diff --git a/libc/src/sys/epoll/epoll_wait.h b/libc/src/sys/epoll/epoll_wait.h index d51c9100846ce0..0cd2f7e9b57ad0 100644 --- a/libc/src/sys/epoll/epoll_wait.h +++ b/libc/src/sys/epoll/epoll_wait.h @@ -9,14 +9,12 @@ #ifndef LLVM_LIBC_SRC_SYS_EPOLL_EPOLL_WAIT_H #define LLVM_LIBC_SRC_SYS_EPOLL_EPOLL_WAIT_H -// TODO: Use this include once the include headers are also using quotes. -// #include "include/llvm-libc-types/struct_epoll_event.h"

-#include <sys/epoll.h> +#include "src/sys/epoll/struct_epoll_event.h" namespace LIBC_NAMESPACE { -int epoll_wait(int epfd, epoll_event *events, int maxevents, int timeout); +int epoll_wait(int epfd, struct epoll_event *events, int maxevents, + int timeout); } // namespace LIBC_NAMESPACE diff --git a/libc/src/sys/epoll/linux/CMakeLists.txt b/libc/src/sys/epoll/linux/CMakeLists.txt index a27905d962dc57..108c6b9bb05301 100644 --- a/libc/src/sys/epoll/linux/CMakeLists.txt +++ b/libc/src/sys/epoll/linux/CMakeLists.txt @@ -3,12 +3,19 @@ add_entrypoint_object( SRCS epoll_wait.cpp HDRS + # TODO: these 2 should not be necessary + ../../../signal/sigset_t.h + ../../../time/struct_timespec.h + ../epoll_wait.h DEPENDS - libc.include.sys_epoll libc.include.sys_syscall libc.src.__support.OSUtil.osutil libc.src.errno.errno + libc.src.sys.epoll.struct_epoll_event + # TODO: why don't these work for overlay mode? + # libc.src.signal.sigset_t + # libc.src.time.struct_timespec ) add_entrypoint_object( @@ -16,13 +23,20 @@ add_entrypoint_object( SRCS epoll_pwait.cpp HDRS + # TODO: these 2 should not be necessary + ../../../signal/sigset_t.h + ../../../time/struct_timespec.h + ../epoll_pwait.h DEPENDS - libc.include.sys_epoll libc.include.signal libc.include.sys_syscall libc.src.__support.OSUtil.osutil libc.src.errno.errno + libc.src.sys.epoll.struct_epoll_event + # TODO: why don't these work for overlay mode? + # libc.src.signal.sigset_t + # libc.src.time.struct_timespec ) add_entrypoint_object( @@ -30,12 +44,19 @@ add_entrypoint_object( SRCS epoll_pwait2.cpp HDRS + # TODO: these 2 should not be necessary + ../../../signal/sigset_t.h + ../../../time/struct_timespec.h + ../epoll_pwait2.h DEPENDS - libc.include.sys_epoll libc.include.signal - libc.include.time libc.include.sys_syscall + libc.include.time libc.src.__support.OSUtil.osutil libc.src.errno.errno + libc.src.sys.epoll.struct_epoll_event + # TODO: why don't these work for overlay mode? + # libc.src.signal.sigset_t + # libc.src.time.struct_timespec ) diff --git a/libc/src/sys/epoll/linux/epoll_pwait.cpp b/libc/src/sys/epoll/linux/epoll_pwait.cpp index ee1b4e66e98444..7e0d6988b02ff3 100644 --- a/libc/src/sys/epoll/linux/epoll_pwait.cpp +++ b/libc/src/sys/epoll/linux/epoll_pwait.cpp @@ -10,15 +10,11 @@ #include "src/__support/OSUtil/syscall.h" // For internal syscall function. #include "src/__support/common.h"

#include "src/errno/libc_errno.h" -#include <sys/syscall.h> // For syscall numbers. +#include "src/signal/sigset_t.h" +#include "src/sys/epoll/struct_epoll_event.h" -// TODO: Use this include once the include headers are also using quotes. -// #include "include/llvm-libc-types/sigset_t.h" -// #include "include/llvm-libc-types/struct_epoll_event.h"

-#include <sys/epoll.h> +#include <sys/syscall.h> // For syscall numbers. namespace LIBC_NAMESPACE { diff --git a/libc/src/sys/epoll/linux/epoll_pwait2.cpp b/libc/src/sys/epoll/linux/epoll_pwait2.cpp index 671dede2a1058d..f97804cfe48b4e 100644 --- a/libc/src/sys/epoll/linux/epoll_pwait2.cpp +++ b/libc/src/sys/epoll/linux/epoll_pwait2.cpp @@ -10,16 +10,12 @@ #include "src/__support/OSUtil/syscall.h" // For internal syscall function. #include "src/__support/common.h"

#include "src/errno/libc_errno.h" -#include <sys/syscall.h> // For syscall numbers. +#include "src/signal/sigset_t.h" +#include "src/sys/epoll/struct_epoll_event.h" +#include "src/time/struct_timespec.h" -// TODO: Use this include once the include headers are also using quotes. -// #include "include/llvm-libc-types/sigset_t.h" -// #include "include/llvm-libc-types/struct_epoll_event.h" -// #include "include/llvm-libc-types/struct_timespec.h"

-#include <sys/epoll.h> +#include <sys/syscall.h> // For syscall numbers.

namespace LIBC_NAMESPACE {

diff --git a/libc/src/sys/epoll/linux/epoll_wait.cpp b/libc/src/sys/epoll/linux/epoll_wait.cpp index 0c43edf7645454..a025bfb2e8d783 100644 --- a/libc/src/sys/epoll/linux/epoll_wait.cpp +++ b/libc/src/sys/epoll/linux/epoll_wait.cpp @@ -11,13 +11,10 @@ #include "src/__support/OSUtil/syscall.h" // For internal syscall function. #include "src/__support/common.h" #include "src/errno/libc_errno.h" -#include <sys/syscall.h> // Fo... [truncated]