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

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

In D142660#4535693 <https://reviews.llvm.org/D142660#4535693>, @stephenpeckham wrote:

> I don't see any reason to check the OBJECT_MODE environment variable if the -X flag is used.  What would the error be:  "You specified a valid -X flag, but by the way, OBJECT_MODE is set to an invalid value"?

The error would be "invalid value for OBJECT_MODE environment variable" (or something to that effect), which would mean the user fixes their environment, just the same as if they hadn't used -X. I just want to emphasise though that my main concern is that llvm-ar and llvm-ranlib are being inconsistent - one of them checks the environment variable first, the other checks the command-line option first, and this seems wrong - they should do the same thing. By reading and checking the environment variable first, it simplifies the code logic (no need for `HasAIXXOption` for example).

As an alternative (but I think adds unnecessary complexity, due to needing an extra variable), you could have both tools read the environment variable into a string variable, then, if the -X option is present, overwrite that variable, and finally feed that string into the parsing code that converts into a `BitMode` value. If the string is invalid, the parsing code could report an error along the lines of "invalid OBJECT_MODE or -X option value".

> I think all the commands that examine XCOFF files (llvm-ar, lllvm-ranlib, llvm-readobj, llvm-objdump, llvm-nm, etc.) should recognize "32", "64", "32_64", and "any".  I don't think it's necessary to recognize "d64", even if AIX commands do.  In addition, I wouldn't bother recognizing an XCOFF file with the magic number for a discontinued 64-bit object.  That means that "32_64" and "any" have the same behavior.   If -X is specified and does not have one of the 4 specified values, a usage message should be printed.  If -X is not specified but OBJECT_MODE is in the environment, a message should be printed if the value is not one of the 4 specified values.

Yes, to be clear, I'm not advocating that "d64" support should be added (I'm not opposed to it, should there be a use-case for it either).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142660