[libc-commits] [PATCH] D71094: [libc] Add implementation of errno and define the other macros of errno.h.

Mon Dec 9 11:29:54 PST 2019

sivachandra added a comment.

In D71094#1775667 <https://reviews.llvm.org/D71094#1775667>, @MaskRay wrote:

> errno is a fundamental component baked in almost every component of a libc and contributors/reviewers should really be on the same page. Both abrachet and stanshebs accepted the patch and they were the initially-added reviewers. I feel a bit lonely to be the only one in the "Request Changes" camp, but I hope you do not commit the patch with these open issues not resolved. To reiterate, I feel strong about my suggested items:
>
> - `__attribute__((visibility("hidden")))` in the internal header.
> - `__attribute__((const))` in the public header.

To be clarify here, I have added the const attribute as soon as I have seen your suggestion.

> They are adopted by both glibc and musl.
> 
> You are reluctant to add hidden visibility because you feel that it may make overlaying llvm-libc on top of system libc harder? Can you discuss more about the details somewhere, e.g. libc-dev or llvm-dev?

As I have said previously, we want to add visibility attributes in a more principled manner. I have also said previously that your suggestion is very valid, and that it was considered even before putting this patch out.

A discussion on libc-dev or llvm-dev will definitely be started when I am prepared with all the details I want to bring up. Or, if someone wants to champion the whole area of interop with other libcs, I would be glad to lean on their help. I do not see why such a discussion should block this patch. FWIW, this patch helps me add more of the library, which I think is of greater importance to keep the development of this library running.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71094