[PATCH] D82677: [Clang] Handle AIX Include management in the driver

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:179
+                                    ArgStringList &CC1Args) const {
+  // return if -nostdinc/-nostdlibinc is specified as a driver option.
+  if (DriverArgs.hasArg(options::OPT_nostdinc) ||
----------------
Minor nit: Use full sentences in comments with proper capitalization for English writing.


================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:181
+  if (DriverArgs.hasArg(options::OPT_nostdinc) ||
+      DriverArgs.hasArg(options::OPT_nostdlibinc))
+    return;
----------------
My understanding is that `-nostdlibinc` does not suppress the inclusion of the "builtin" headers.


================
Comment at: clang/test/Driver/aix-toolchain-include.cpp:1
+// Tests that AIX toolchain adds system includes to its search path.
+
----------------
s/that AIX/that the AIX/;


================
Comment at: clang/test/Driver/aix-toolchain-include.cpp:3
+
+// Check powerpc-ibm-aix, 32-bit/ 64-bit.
+// RUN: %clangxx -### -no-canonical-prefixes %s 2>&1 \
----------------
Minor nit: Remove the space after the slash.


================
Comment at: clang/test/Driver/aix-toolchain-include.cpp:40
+// CHECK-NOSTDINC-INCLUDE:	"-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-NOSTDINC-INCLUDE-NOT:	"-internal-isystem"
----------------
There should be testing for `-nostdlibinc` as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82677