[PATCH] D119147: [AIX][clang][driver] Check the command string to the linker for exportlist opts and

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:196
+    const char *CreateExportListExec = Args.MakeArgString(
+        llvm::sys::path::parent_path(ToolChain.getDriver().ClangExecutable) +
+        "/llvm-nm");
----------------
Use sys::path::append


================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:200
+
+    auto CreateExportListPath =
+        C.getDriver().GetTemporaryPath("CreateExportList", "exp");
----------------
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable


================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:205
+
+    for (const auto &II : Inputs) {
+      if (II.isFilename())
----------------
omit braces


================
Comment at: clang/test/Driver/aix-ld.c:611
+// RUN:        -resource-dir=%S/Inputs/resource_dir \
+// RUN:        -shared \
+// RUN:        --target=powerpc-ibm-aix7.1.0.0 \
----------------
You may pack more options on one line.
The current style may make the file unnecessarily long.


================
Comment at: clang/test/Driver/aix-ld.c:985
+// CHECK-LD64-SHARED-EXPFULL:     "-bM:SRE"
+// CHECK-LD64-SHARED-EXPFULL:     "-bnoentry"
+// CHECK-LD64-SHARED-EXPFULL:     "-b64"
----------------
If these options are actually adjacent, check them on the same line to make the test more strict: you can catch issues if new options are somehow inserted in between.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119147