Given that the header is arguably defective, imo this unfortunate hack is justifiable.  Please leave a comment explaining it though <br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Mar 15, 2019 at 3:54 PM Hubert Tong via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">hubert.reinterpretcast added inline comments.<br>
<br>
<br>
================<br>
Comment at: lib/Support/Unix/Path.inc:81<br>
+#include <sys/statfs.h><br>
+typedef uint_t uint;<br>
+#include <sys/vmount.h><br>
----------------<br>
krytarowski wrote:<br>
> hubert.reinterpretcast wrote:<br>
> > krytarowski wrote:<br>
> > > hubert.reinterpretcast wrote:<br>
> > > > krytarowski wrote:<br>
> > > > > hubert.reinterpretcast wrote:<br>
> > > > > > apaprocki wrote:<br>
> > > > > > > hubert.reinterpretcast wrote:<br>
> > > > > > > > apaprocki wrote:<br>
> > > > > > > > > `sys/statfs.h` includes `sys/types.h`, which contains this `typedef`. Was there a reason why it was put here between the two headers?<br>
> > > > > > > > Yes, the header is defective. It needs a typedef that is only conditionally defined.<br>
> > > > > > > I guess I am not seeing that.. is it OS version dependent?<br>
> > > > > > > <br>
> > > > > > > <br>
> > > > > > > ```<br>
> > > > > > > $ cat test.c<br>
> > > > > > > #include <sys/statfs.h><br>
> > > > > > > #include <sys/vmount.h><br>
> > > > > > > $ xlc -E -c test.c | grep "typedef.*uint;"<br>
> > > > > > > typedef         uint_t          uint;<br>
> > > > > > > $<br>
> > > > > > > ```<br>
> > > > > > Try with `-D_XOPEN_SOURCE=700` or anything that suppresses `_ALL_SOURCE`:<br>
> > > > > > ```<br>
> > > > > > > xlclang -D_XOPEN_SOURCE=700 test.c -c<br>
> > > > > > In file included from test.c:2:<br>
> > > > > > /usr/include/sys/vmount.h:195:2: error: unknown type name 'uint'; did you mean 'int'?<br>
> > > > > >         uint    vmt_revision;   /* I revision level, currently 1        */<br>
> > > > > >         ^<br>
> > > > > > /usr/include/sys/vmount.h:196:2: error: unknown type name 'uint'; did you mean 'int'?<br>
> > > > > >         uint    vmt_length;     /* I total length of structure and data */<br>
> > > > > >         ^<br>
> > > > > > /usr/include/sys/vmount.h:203:2: error: unknown type name 'uint'; did you mean 'int'?<br>
> > > > > >         uint    vmt_time;       /* O time of mount                      */<br>
> > > > > >         ^<br>
> > > > > > /usr/include/sys/vmount.h:204:2: error: unknown type name 'uint'; did you mean 'int'?<br>
> > > > > >         uint    vmt_timepad;    /* O (in future, time is 2 longs)       */<br>
> > > > > >         ^<br>
> > > > > > 4 errors generated.<br>
> > > > > > Error while processing test.c.<br>
> > > > > > Return:  0x01:1<br>
> > > > > > ```<br>
> > > > > This typedef looks suspicious, there shall be a way to workaround it.<br>
> > > > What is your proposed solution to otherwise provide a type name required by the system header that might not be defined? Note that this code is guarded under `defined(_AIX)`, and it has been demonstrated that this typedef might be defined anyway by the system headers on that platform to be an alias for `uint_t` (which is, in turn, an alias for `unsigned int`).<br>
> > > I don't have access right now to AIX to check it myself, but on UNIX it's typically needed to include `<sys/types.h>` and/or specify `-D` flags like `-D_POSIX_SOURCE`.<br>
> > Yes, and `<sys/types.h>` needs `-D_ALL_SOURCE` before `uint` is provided, and `_ALL_SOURCE` on AIX produces a lot of stray macros. I'd like to avoid needing `_ALL_SOURCE` for the build if at all possible.<br>
> How about restricting `_ALL_SOURCE` to this file only?<br>
> <br>
> ```<br>
> #define _ALL_SOURCE // Needed for uint typedef<br>
> #include <sys/statfs.h><br>
> ```<br>
That would not work if `sys/types.h` is included by an earlier header, and defining `_ALL_SOURCE` earlier in `Path.cpp` would expose the LLVM headers included by that file to the stray macros that come with `_ALL_SOURCE`.<br>
<br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D58801/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D58801/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D58801" rel="noreferrer" target="_blank">https://reviews.llvm.org/D58801</a><br>
<br>
<br>
<br>
</blockquote></div>