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

[Alex Brachet via Phabricator via libc-commits]

abrachet added a comment.

In D79192#2015295 <https://reviews.llvm.org/D79192#2015295>, @sivachandra wrote:

> Few high level comments and questions:
>
> - Instead of creating a target for every entrypoint, have you considered generating a single file which:
>   1. Includes all header files listed in the `api.td`.
>   2. Lists `cpp:Function` pointers to all functions as required by `api.td`.
>
>     There definitely are problems that need to be solved with this approach. For example, how do you ensure all public header files are already present in the `build/.../libc/include` directory. But, the appeal of this approach is that one does not have to create explicit or implicit targets for "integration tests". There would be just one target which would test everything in one go.


+1 to doing this just from `api.td`. As for the includes, I think it would be wise for us to make the header files dependencies of `llvmlibc` or maybe better create a `libc_headers` target to generate all of our headers.

> - In general, I don't think we should expect that internal API to match the public API. For example, we might choose to give internal types names which are different from the public names. So, I think the second kind of tests you mention in your description is not required as it is restrictive.

Agreed. I think there are a few places already we use an internal `__llvm_libc` types I think these would break. That being said because we are doing that, it is important to test that the external api is compatible with the internal one, I cant think of a great way to do that though.



================
Comment at: libc/utils/HdrGen/PrototypeTestGen/PrototypeTestGen.cpp:48
+  OS << "int main() {\n";
+  OS << "  __llvm_libc::cpp::function<";
+  auto G = makeAPIReader(PublicHeaderFileOption, records);
----------------
It was just recently renamed to `Function` But I think it makes more semantic sense to use `cpp::IsSame<[type], decltype(EntryPointNameOption)>`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79192