[libc-commits] [PATCH] D84848: [libc] Add a tool called WrapperGen.

Thu Jul 30 12:58:12 PDT 2020

abrachet accepted this revision.
abrachet added a comment.
This revision is now accepted and ready to land.

In D84848#2182500 <https://reviews.llvm.org/D84848#2182500>, @sivachandra wrote:

> In D84848#2182387 <https://reviews.llvm.org/D84848#2182387>, @abrachet wrote:
>
>> This looks good. I wonder though how we will deal with global variables. Those can't be easily wrapped like functions can.
>
> Yes, global variables question is open if we have to support them in some manner. The one we have currently, `errno` which not really global but like one, is accessed by the extension function `__errno_location`. So, we are good in this case.  If there are other global variables we need to support (nothing comes to my mind), we can take that up at that time.

There are the stupid globals related to `getopt` :). Indeed solvable later.

================
Comment at: libc/utils/tools/WrapperGen/Main.cpp:58
+
+  // TODO: Arg types of the C++ implementation functions need not
+  // match the standard types. Either handle such differences here, or
----------------
sivachandra wrote:
> abrachet wrote:
> > I think this should be fine because all we care about it ABI compatibility, and we're only including the src/ headers not the include/ ones so the compiler wont complain there.
> Right. In the existing code, there are no examples which break. But, added this `TODO` just in case. Does not add value you think?
No preference either way.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84848