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 _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.
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. 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. Any opinions on how to handle this? -John On Fri, Apr 24, 2015 at 4:06 PM, Sean Silva <[email protected]> wrote: > ================ > Comment at: lib/Headers/mm_malloc.h:27-33 > @@ -26,3 +26,9 @@ > > -#include <stdlib.h> > +#ifdef __cplusplus > +extern "C" void free(void *); > +extern "C" void *malloc(__SIZE_TYPE__); > +#else > +void free(void *); > +void *malloc(__SIZE_TYPE__); > +#endif > > ---------------- > rsmith wrote: > > 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?) > I think this is a quirk of how I bootstrapped modules on PS4. > > 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. > > ================ > Comment at: lib/Headers/stdarg.h:29 > @@ -28,1 +28,3 @@ > > +#include <_gnuc_va_list.h> > + > ---------------- > rsmith wrote: > > 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? > > > > 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. > > > > 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. > 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. > > http://reviews.llvm.org/D9229 > > EMAIL PREFERENCES > http://reviews.llvm.org/settings/panel/emailpreferences/ > > > -- John Thompson [email protected]
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
