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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.


================
Comment at: clang/lib/Driver/Job.cpp:304
 
+void Command::setRedirectFiles(std::vector<Optional<std::string>> Redirects) {
+  RedirectFiles = Redirects;
----------------



================
Comment at: clang/lib/Driver/Job.cpp:370
+                                     makeArrayRef(RedirectFilesOptional),
+                                     /*secondsToWait*/ 0, /*memoryLimit*/ 0,
+                                     ErrMsg, ExecutionFailed, &ProcStat);
----------------



================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:80
+static bool hasExportListLinkerOpts(const ArgStringList &CmdArgs) {
+  for (size_t i = 0; i < CmdArgs.size(); ++i) {
+    llvm::StringRef ArgString(CmdArgs[i]);
----------------
https://llvm.org/docs/CodingStandards.html#don-t-evaluate-end-every-time-through-a-loop

Prefer `!=`


================
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:219
+
+    std::unique_ptr<Command> ExpCommand = std::make_unique<Command>(
+        JA, *this, ResponseFileSupport::None(), CreateExportListExec,
----------------



================
Comment at: clang/test/Driver/aix-ld.c:609
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -shared (with exp option strings in other opt).
+// RUN: %clangxx -x c++ -no-canonical-prefixes %s 2>&1 -### \
+// RUN:        -resource-dir=%S/Inputs/resource_dir \
----------------
If you don't test `clang` in CHECK lines, you can omit `-no-canonical-prefixes`

Consider switching to `.cpp` if you always use `-x c++`.


================
Comment at: clang/test/Driver/aix-ld.c:615
+// RUN:        -ccc-install-dir %T/open_xl_aix_install/opt/IBM/openxlC/%open_xl_vrm/bin \
+// RUN:        -unwindlib=libunwind \
+// RUN:        -Wl,-Z/expall/expfull/a-bE:/a-bexport:/ \
----------------
`-u` prefixed long option form is not recommended. `-u` is a short option taking a value, therefore an option name typo cannot be detected.


================
Comment at: clang/test/Driver/aix-ld.c:624
+// CHECK-LD32-SHARED:     "--export-symbols"
+// CHECK-LD32-SHARED:     "-X"
+// CHECK-LD32-SHARED:     "32"
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119147