[PATCH] D97138: [Driver] replace argc_ with argc

Andrzej Warzynski via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 22 03:14:01 PST 2021


awarzynski requested changes to this revision.
awarzynski added a comment.
This revision now requires changes to proceed.

Thank you for submitting this @achieveartificialintelligence !

The aim of these changes is to improve the consistency of the code-base with respect to the coding standards documented separately for Clang <https://llvm.org/docs/CodingStandards.html#llvm-coding-standards> and Flang <https://github.com/llvm/llvm-project/blob/main/flang/docs/C%2B%2Bstyle.md>. I think that this is a very positive suggestion, but would like to request a few refinements before this is ready:

- Please note that in LLVM/Clang, variables names start with upper case (so it should be int `main(int Argc, const char **Argv)` instead of `main(int argc, const char **argv)` ). link <https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly>
- In Flang the rules are slightly different, and the variable names start with a lower case (so it should be int `main(int Argc, const char **Argv)` instead of `main(int argc, const char **argv)` ). link <https://github.com/llvm/llvm-project/blob/main/flang/docs/C%2B%2Bstyle.md#naming>
- Your commit message refers to _a driver_, but in fact you are updating 2 specific drivers: Clang compiler driver and Flang compiler driver - could you clarify?
- Your commit message refers to `argc_` specifically, but you are updating `argc_`, `argv_` and `Argv/argv` too.

Personally I'd prefer `ArgC/argC` and `ArgV/argV` for input parameters and `ArgValues/argValues` for the local variable. This way it would be more self-explanatory. But that's a matter of style. As long as this is consistent with the coding standards, I think that this should be accepted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97138



More information about the cfe-commits mailing list