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

[Alex Brachet via Phabricator via libc-commits]

abrachet marked an inline comment as done.
abrachet added inline comments.


================
Comment at: libc/config/linux/api.td:34
+  let Defn = [{
+    int *__errno_location() __attribute__((const));
+    #define errno *__errno_location()
----------------
MaskRay wrote:
> sivachandra wrote:
> > abrachet wrote:
> > > I don't have a Windows machine to test this, but from godbolt, `__attribute__`'s seem to be unrecognized by MSVC or at least const was not a known one. I see other libcs conditionally defining (on `__GNUC__` according to @MaskRay) an `__attribute_const` or something like that.
> > > 
> > > FWIW I don't favor adding this attribute. How useful will this even be really?
> > We are adding the attribute in a Linux only file so should not be a problem because Clang and GCC on Linux support on this attribute?
> Using unprotected `__attribute__` will break cross compilability. MSVC does not accept it. Or you can say llvm-libc doesn't support MSVC. I'm totally fine with it.
> 
> > FWIW I don't favor adding this attribute. How useful will this even be really?
> 
> This matters when you run lots of functions that may potentially modify `errno` in a function.
> 
> ```
> foo();
> if (errno)
>   ...
> foo();
> if (errno)
>   ...
> foo();
> if (errno)
>   ...
> ```
> Using unprotected __attribute__ will break cross compilability. MSVC does not accept it. Or you can say llvm-libc doesn't support MSVC. I'm totally fine with it.
Like @sivachandra said earlier, this is just the config file for Linux, if we support Windows later then it will not use this attribute in its config file.

>This matters when you run lots of functions that may potentially modify `errno` in a function.
Hmm. I'm not trying to argue, this is purely out of curiosity at this point, because a lot of libcs use this attribute. If I'm understanding correctly, that code will call `__errno_location()` once and use the first value each time after. But can't different calls to `foo` set `errno` differently?


================
Comment at: libc/config/linux/api.td:106
+
+    // POSIX
+    SimpleMacroDef<"EPERM", "4">,
----------------
MaskRay wrote:
> This comment may give a false impression that these constants are defined by POSIX but they are not.
> 
> On FreeBSD, the values are entirely different.
> 
> Apparently this will soon be a Linux only project after more Linux specific things are unconsciously baked in.
Like above, this is just the Linux config file, thee FreeBSD one will have different values.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71094