[libc-commits] [PATCH] D79192: [libc] Add integration tests.

Siva Chandra via Phabricator via libc-commits libc-commits at lists.llvm.org
Fri May 8 14:31:36 PDT 2020


sivachandra added a comment.

In D79192#2024575 <https://reviews.llvm.org/D79192#2024575>, @PaulkaToast wrote:

>   ...
>   #include <unistd.h>
>   #include <sys/mman.h>
>   #include <math.h>
>  
>   int main() {
>     static_assert(__llvm_libc::cpp::IsSame<int(int), decltype(raise)>::Value, "raise prototype in TableGen does not match public header");
>     static_assert(__llvm_libc::cpp::IsSame<int(int, const struct sigaction *__restrict, struct sigaction *__restrict), decltype(sigaction)>::Value, "sigaction prototype in TableGen does not match public header");
>     static_assert(__llvm_libc::cpp::IsSame<int(sigset_t *, int), decltype(sigdelset)>::Value, "sigdelset prototype in TableGen does not match public header");
>   ...
>
>
> Thanks to @abrachet's suggestion of using `IsSame` we also get nice error messages. As for ensuring all public header files are present I added a top-level target for the headers that we can depend on here (same as @abrachet mentioned)


LG

>> - Using `-nostdinc` would require us to explicitly list path to the linux headers for entrypoints which depend on the linux header files, which will very likely bring all headers into the include path. Do you have any ideas on how to get around it?
> 
> For now I've commented out `-nostdinc` with a TODO, the most straightforward way would be to have our public headers not depend on Linux dev headers but another approach could be to use our clang-tidy `RestrictSystemIncludes` check to ensure only the right Linux header gets included (this would add a dependency on `clang-tidy` for the integration tests though)

I think we should be complete here. Lets split this change into two changes:

1. First change will add a target to build the main object but not the full executable. In this change, we will make use of clang-tidy to ensure only LLVM-libc or platform or compiler headers are being included.
2. Second change will add a target to link this main object with the loader object making it a true integration test.

Before you make changes, I want to bring up few other high-level points for discussion:

1. The list of public headers should be determined by the target config. However, we don't have a way for a config to list the public headers it wants. We can take two approaches here again: provide a rule to list a target for public headers, or just list the targets for the headers in `.txt` file. I would prefer keeping it simple and just requiring `.txt` file which defines a CMake list of targets for public headers. The CMake file listing the integration test target will include this file to set up the dependencies. Something like this:

  # config/linux/headers.txt:
  
  set(PUBLIC_HEADERS
    libc.include.math
    libc.inlcude.signal
    ...
  )

And:

  # integration_test/CMakeLists.txt
  include("config/${LIBC_TARGET_OS}/headers.txt")
  ...



2. The list of entrypoints that go into a `.a` file should also be determined by the target config. This can also be done by listing the entrypoint targets in a, say `entrypoints.txt` file. The file `lib/CMakeLists.txt` can include this file and get the entrypoints for the various static libraries, just like above. This mechanism can also help us skip building and testing entrypoint targets if a config doesn't list it in its `entrypoints.txt` file.

WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79192





More information about the libc-commits mailing list