[PATCH] D33880: COFF: Introduce LD shim around LINK

Martell Malone via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 22 18:33:13 PDT 2017


martell added inline comments.


================
Comment at: MinGW/Driver.cpp:53
+// Create table mapping all options defined in Options.td
+static const opt::OptTable::Info infoTable[] = {
+#define OPTION(X1, X2, ID, KIND, GROUP, ALIAS, X7, X8, X9, X10, X11, X12)      \
----------------
ruiu wrote:
> infoTable -> InfoTable
> 
> (Yet another style issue, and honestly I'm a bit tired of pointing them out, so please please fix them before sending your patch to review. If you do not pay attention to the documented and obvious coding style rules, I would probably suspend doing "real" code review and instead pointing out only style issues until your code follows the coding style. I understand that we cannot and do not want to reduce the error rate to zero, and I really appreciate your work on lld/mingw, but pointing out that variables are in CamelCase in the LLVM coding style on virtually every patch update is a waste of both your and my time.)
I addressed ALL the CamelCase variables in the patch and left a note for this one specifically before you reviewed it again. You should see a comment a few lines below where `infoTable` is used with the following text
`infoTable doesn't seem to follow the variable naming guide but this is done consistently across LLVM`

Here are references to the locations I found using this exact spelling which is why I left it.

LLD

COFF/DriverUtils.cpp
lib/Driver/DarwinLdDriver.cpp

LLVM

tools/llvm-mt/llvm-mt.cpp
tools/llvm-rc/llvm-rc.cpp
lib/ToolDrivers/llvm-lib/LibDriver.cpp
tools/llvm-cvtres/llvm-cvtres.cpp
unittests/Option/OptionParsingTest.cpp
etc

Do you still want me to change it?


Repository:
  rL LLVM

https://reviews.llvm.org/D33880





More information about the llvm-commits mailing list