[PATCH] D69665: [llvm-ar] Fix llvm-ar response file reading on Windows

Owen Reynolds via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 4 10:59:23 PST 2019


gbreynoo marked an inline comment as done.
gbreynoo added inline comments.


================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:182
 
-static std::string Options;
+static std::string KeyLetters;
 
----------------
MaskRay wrote:
> Why `KeyLetters`? I think the ar terminology uses operations and modifiers. Collectively they can be called options.
I wanted to make the use of the variable clearer as the term `Options` differs in meaning:

  - In the help text the term `Options` is used for flags (non-single letter) that are not `Operations` or `Modifiers`.
  - In the documentation `Options` is for `Operations` , `Modifiers` and  `Other` (non-single letter)
  - In the code the `Options` string only holds `Operations` and `Modifiers`, i.e. single letter arguments. Longer flags (not-single letter) are handled by other methods.

I hoped that changing the variable name would lessen confusion but can change it back if required.



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

https://reviews.llvm.org/D69665





More information about the llvm-commits mailing list