[PATCH] D97138: [clang][flang] Improve the consistency of the code-base
Shao-Ce Sun via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 22 16:41:49 PST 2021
achieveartificialintelligence added a comment.
In D97138#2578563 <https://reviews.llvm.org/D97138#2578563>, @awarzynski wrote:
> 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.
Thank you very much for your comment.
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