[PATCH] D64121: Rename variables so that they start with a lowercase letter.

Wed Jul 3 03:45:52 PDT 2019

peter.smith added a comment.

> Currently, `IS` is just renamed `is`, but variables starting with `IS` (such as `ISAddr`) is renamed `isecAddr` by a special rule. Otherwise it would have looked like a predicate. I made a few special rules other than that, which you can see at https://reviews.llvm.org/D64123 (look for `lowercase` function).

OK, thanks for a link to the rules. Reading https://reviews.llvm.org/D57896 (Variable Name Rules) it mentions that acronyms should be avoided but should favour upper case. There did seem to be some disagreement in the comments though and I don't know how strictly the guidelines will be enforced. There are trade-offs either way.

>> The other case where we may want to look at is use of capitals like S, P and A in relocation processing as these match the ELF specification. Of course we could use sym, place, addend instead.
> 
> They are currently just renamed s, p and a, which doesn't look too awful to me.

Agreed, I have a mild preference for the acronyms in upper case, but I can see the benefit of keeping the rule simple.

================
Comment at: lld/ELF/Arch/MipsArchTree.cpp:77

-    uint32_t ABI2 = F.Flags & (EF_MIPS_ABI | EF_MIPS_ABI2);
-    if (ABI != ABI2)
-      error(toString(F.File) + ": ABI '" + getAbiName(ABI2) +
-            "' is incompatible with target ABI '" + getAbiName(ABI) + "'");
+    uint32_t ABI2 = f.flags & (EF_MIPS_ABI | EF_MIPS_ABI2);
+    if (abi != ABI2)
----------------
Looks like this got missed by the rename. I've not found any others yet.

================
Comment at: lld/ELF/MapFile.cpp:90
+    raw_string_ostream os(str[i]);
+    OutputSection *oSec = syms[i]->getOutputSection();
+    uint64_t vma = syms[i]->getVA();
----------------
maybe worth a special case for OSec -> osec to match other uses of isec.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64121