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

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 3 02:57:18 PDT 2019


ruiu added a comment.

In D64121#1567970 <https://reviews.llvm.org/D64121#1567970>, @peter.smith wrote:

> In general I think a move to camelCase or snake_case is an improvement, don't have a particularly strong opinion over which one. I suspect that the camelCase transition is easier to automate. With just a quick scan of some of the output there are a few cases where some of the variable names are used as acronyms ISD -> InputSectionDescription. Or IS -> InputSection. I don't think that these survive the transition well, in particular IS -> is. I've not got a great idea for what to do about these right now, some possible options:


You are right; converting to camelCase is easier to automate than underscore. For some variables such as IsLMAAddr, it is not obvious where we should insert underscores and how to capitalize them. In addition to that, inserting underscores changes lengths of virtually all variables, so we need to run clang-format after running my tool, which effectively runs clang-format to the entire code base. I didn't want to do that, because for some files it looks like it drastically changes how source code is formatted.

> - Do the full renaming as above and change the variable names later. Presumably it would help to be consistent.
> - Don't rename common acronyms like IS and change the variable names later.
> - Allow common acronyms to stay capitalised.
> - Have some custom renames for acronyms like IS. I don't have a particularly strong opinion on which option. It would be helpful to be consistent though.

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).

> 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64121





More information about the llvm-commits mailing list