<div dir="ltr">Our process for trying to get the SDK headers in shape for modules with respect to duplicate definitions was to mechanically break out the definition into a separate file and include that file in the affected header.  This was done without really contemplating why there were duplicate definitions in the fist place.  In this case, the <span style="font-size:12.8000001907349px">_gnuc_va_list symbol was defined in both BSD's machine/_types.h and Eli Friedman's stdarg.h header, and the modularize tool flagged this.</span><div><span style="font-size:12.8000001907349px"><br></span></div><div><span style="font-size:12.8000001907349px">It might very well be and probably is the case that it doesn't need to be in machine/_types.h.  I can compile the headers at least with it missing, and later we could try a full system build.  I was told that a standard header can't include other standard headers if it's not already done, so I assumed including stdarg.h in machine/_types.h was out.</span></div><div><span style="font-size:12.8000001907349px"><br></span></div><div><span style="font-size:12.8000001907349px">So the intrinsics are meant to be above the standard headers in layering?  I assumed that because of their low-level nature, they were at the bottom.  We actually put the intrinsics (with stdarg.h and mm_malloc.h) in a totally separate directory (host_tools/lib/clang/include instead of target/include), and indeed, some of the intrinsic headers are included by headers in both of our other SDK include directories.  So having mm_malloc.h include stdlib.h was problematic, hence my changes to mm_malloc to remove the dependence.</span></div><div><span style="font-size:12.8000001907349px"><br></span></div><div><span style="font-size:12.8000001907349px"> Any opinions on how to handle this?</span><span style="font-size:12.8000001907349px"><br></span></div><div><span style="font-size:12.8000001907349px"><br></span></div><div><span style="font-size:12.8000001907349px">-John</span></div><div><span style="font-size:12.8000001907349px"><br></span></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Apr 24, 2015 at 4:06 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">================<br>
Comment at: lib/Headers/mm_malloc.h:27-33<br>
@@ -26,3 +26,9 @@<br>
<br>
-#include <stdlib.h><br>
+#ifdef __cplusplus<br>
+extern "C" void free(void *);<br>
+extern "C" void *malloc(__SIZE_TYPE__);<br>
+#else<br>
+void free(void *);<br>
+void *malloc(__SIZE_TYPE__);<br>
+#endif<br>
<br>
----------------<br>
</span><span class="">rsmith wrote:<br>
> The intended layering is that Clang's `_Builtin_intrinsics` module is a layer on top of the C standard library module. Does that not work in your setup? (Does the module defining <stdlib.h> contain a header that includes these intrinsics headers?)<br>
</span>I think this is a quirk of how I bootstrapped modules on PS4.<br>
<br>
My first target for modularization (after measuring where we were spending time parsing) was a vector math library that only depended on the intrinsics (not mm_malloc though). So initially I just needed vector math lib -> _Builtin_intrinsics and this was a way to work around the issues I was seeing (and I guess we've just been doing this ever since). Going back now, it looks like there is a relatively self-contained part of the system headers that reference the intrinsics and can be split out into their own top-level module so that we can use the layering you describe.<br>
<span class=""><br>
================<br>
Comment at: lib/Headers/stdarg.h:29<br>
@@ -28,1 +28,3 @@<br>
<br>
+#include <_gnuc_va_list.h><br>
+<br>
----------------<br>
</span><span class="">rsmith wrote:<br>
> Can you say a bit more about why this split is useful to you? Do you want to include this part of <stdarg.h> and not the rest, and if so, why?<br>
><br>
> It looks to me like we have a more fundamental problem with our <stdarg.h>; like <stddef.h> it appears to be intended that glibc can request individual pieces of it with `__need_*` macros -- or at least, if `__need___va_list` is defined, we're supposed to vend a `__gnuc_va_list` typedef and nothing else.<br>
><br>
> So I think we should rearrange this header to properly handle `__need___va_list` in general, and the behavior you want here (to be able to produce `__gnuc_va_list` only) will naturally fall out from that.<br>
</span>John, what error were you seeing that necessitated this? I just did some micro-testing and it seems that clang will actually merge a duplicated definition here.<br>
<div class="HOEnZb"><div class="h5"><br>
<a href="http://reviews.llvm.org/D9229" target="_blank">http://reviews.llvm.org/D9229</a><br>
<br>
EMAIL PREFERENCES<br>
  <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
<br>
<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature">John Thompson<br><a href="mailto:John.Thompson.JTSoftware@gmail.com" target="_blank">John.Thompson.JTSoftware@gmail.com</a><br></div>
</div>