[PATCH] D142660: [AIX] supporting -X options for llvm-ranlib in AIX OS

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

clang-format is complaining in the pre-merge CI.



================
Comment at: clang/test/lit.cfg.py:391
 if "system-aix" in config.available_features:
-    config.substitutions.append(("llvm-nm", "env OBJECT_MODE=any llvm-nm"))
-    config.substitutions.append(("llvm-ar", "env OBJECT_MODE=any llvm-ar"))
+   config.environment["OBJECT_MODE"] ="any"
 
----------------
It might be a good idea for you to run the `black` tool on python changes, to ensure they conform to the coding standard, much like you do with clang-format for C++ changes.


================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:901
   uint64_t NumSyms32 = 0; // Store symbol number of 32-bit member files.
+  bool DoesWriteSymtab = WriteSymtab != SymtabWritingMode::NoSymtab;
 
----------------
This makes the variable name more grammatically correct.


================
Comment at: llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp:488
+          Thin ? object::Archive::K_GNU : object::Archive::K_COFF,
+          /*Deterministic*/ true, Thin, nullptr, COFF::isArm64EC(LibMachine))) {
     handleAllErrors(std::move(E), [&](const ErrorInfoBase &EI) {
----------------
MaskRay wrote:
> 
Marked as done, but not fully addressed - in this specific style, there should be no space between the comment and thing it's referring to. This is a reason why clang-format will be failing (there may be others).


================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:74
+         << "  -X{32|64|32_64|any}   - Specify which archive symbol tables "
+            "should be generated if they do not already exist (AIX OS only)\n";
 }
----------------
Strictly speaking, this should be "Big Archive formats only" not "AIX OS only" since theoretically you could have Big Archive archives on other platforms. However, I'm not bothered if you don't want to change this. The same probably goes for other tools for that matter, but don't change them now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142660