This is an archive of the discontinued LLVM Phabricator instance.

[AIX][libcxx] AIX system headers need stdint.h and inttypes.h to be re-enterable when macro _STD_TYPES_T is defined
ClosedPublic

Authored by xingxue on Mar 12 2019, 7:11 AM.

Diff Detail

Event Timeline

xingxue created this revision.Mar 12 2019, 7:11 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
mclow.lists requested changes to this revision.Mar 12 2019, 7:48 AM

I see no tests here.

This revision now requires changes to proceed.Mar 12 2019, 7:48 AM
xingxue updated this revision to Diff 200967.May 23 2019, 7:13 AM

Added test case stdint_h.sh.cpp.

mclow.lists added inline comments.May 23 2019, 9:54 AM
libcxx/include/stdint.h
17

I don't think that this will do what you want it to.
Is this a supported use case?

#include <stdint.h>
#define _STD_TYPES_T
#include <stdint.h>
libcxx/include/stdint.h
17

The comment is perhaps not clear. We need <stdint.h> to be re-enterable only as long as all previous inclusions have occurred while _STD_TYPES_T is defined. The effect of including <stdint.h> without defining _STD_TYPES_T is a strict superset of the effects of including <stdint.h> in other circumstances. Should we adjust the comment?

xingxue updated this revision to Diff 201746.May 28 2019, 1:02 PM

Updated comments explaining the scenario of the changes.

xingxue marked 2 inline comments as done.May 28 2019, 1:25 PM
xingxue marked an inline comment as done.May 28 2019, 1:39 PM
xingxue added inline comments.
libcxx/include/stdint.h
17

Updated comments as suggested. This patch helps scenarios such as the following on AIX.

#include <sys/types.h>
#include <stdint.h>

where <sys/types.h> included in the first line defines macro _STD_TYPES_T, includes <stdint.h>, and then undefines _STD_TYPES_T. <stdint.h> included in the second line can be entered to get to macros such as UINT32_MAX. Since _STD_TYPES_T is undefined with the second inclusion, the header guard macro is defined and <stdint.h> is no longer re-enterable.

LGTM with minor comment.

libcxx/test/std/depr/depr.c.headers/stdint_h.sh.cpp
22

Typo:
s/_XOPEN_SOUECE/_XOPEN_SOURCE/;

xingxue updated this revision to Diff 202734.Jun 3 2019, 9:08 AM

Addressed comments:

  • Fixed typo _XOPEN_SOUECE->_XOPEN_SOURCE
xingxue marked 2 inline comments as done.Jun 3 2019, 9:09 AM
xingxue added inline comments.
libcxx/test/std/depr/depr.c.headers/stdint_h.sh.cpp
22

Good catch! Fixed.

xingxue marked an inline comment as done.Jun 3 2019, 9:10 AM

Hi @mclow.lists, Do you have any further comments?

This revision was not accepted when it landed; it landed in state Needs Review.Jun 20 2019, 8:33 AM
This revision was automatically updated to reflect the committed changes.

FYI, I had similar patches for Linux/Debian packaging.
Thanks!