[PATCH] D62533: Build with _XOPEN_SOURCE defined on AIX

Hubert Tong via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 28 12:38:25 PDT 2019


hubert.reinterpretcast added inline comments.


================
Comment at: llvm/CMakeLists.txt:862
+# Build with _XOPEN_SOURCE on AIX, as stray macros in _ALL_SOURCE mode tend to
+# break things. In this case we need to enable the LARGE FILE API as well
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
----------------
Use either "code" or English style: `s/LARGE FILE API/large-file API/;`.
Add a period to the end of the sentence.


================
Comment at: llvm/cmake/config-ix.cmake:27
+# Do checks with _XOPEN_SOURCE and LARGE FILE API on AIX, as we will build with
+# that too
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
----------------
Same comment as above regarding "LARGE FILE API".
Use "because" in place of "as", and replace "that" with "those".
Add a period to the end of the sentence.


================
Comment at: llvm/utils/unittest/CMakeLists.txt:33
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+    remove_definitions("-D_XOPEN_SOURCE=700")
+endif()
----------------
Indent only two spaces.


================
Comment at: llvm/utils/unittest/googletest/src/gtest-port.cc:62
 #if GTEST_OS_AIX
+#define _ALL_SOURCE
 # include <procinfo.h>
----------------
It's generally a bad idea to define such macros after system headers have already been included.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62533





More information about the cfe-commits mailing list