(original) (raw)
On Fri, Feb 22, 2019 at 5:59 AM via llvm-dev <llvm-dev@lists.llvm.org> wrote:
- Local variables and formal parameters should be lower\_case, with
one exception: Variables/parameters that have lambda/function
type should follow the function-name spelling rules.
I really dislike this exception. Callable objects are \*objects\* and locally scoped, I would much prefer they look like variables.
Also, what about callable objects that aren't lambdas? Or that use operator() for something other than emulating a function call?
I think the simple rule is superior.
- Initialisms and other abbreviations would be considered words for
this purpose, so we have names such as:
tli // Local variable for TargetLoweringInfo
m\_cgm // Data member for CodeGenModule
Agreed.
\- I don't have a good suggestion for file-static/global variables.
Some people have suggested a "g\_" prefix for globals, or possibly
an "s\_" prefix for class-static data.
These are rare enough that I'm not sure we need special rules for naming them. They should also should typically be wrapped up in an actual API limiting how widely they are referenced.
Regarding the transition:
Some people have worried that the churn will cause blame issues.
I respectfully point out that in my own archaeology I have to deal
with lots of clang-format/indentation/other random semantically
meaningless refactoring, this is just one more. Also the point is
not to optimize for git-blame but to optimize for reading what is
there at the moment.
A more focused and shorter transition period will create a lot of
short-term churn but get us to the good endpoint sooner. Doing
conversions per-file or per-class (rather than per-function \[too
small\] or per-library \[too big\]) are probably the way to go.
Given we are changing the names used for \_data\_, and we try to
practice good data-hiding, the impact of the conversion of any
given class \*ought\* to be reasonably confined.
I generally agree with this strategy. That said, I would still do it somewhat lazily rather than eagerly, but batched much as you're suggesting.
If someone can make clang-tidy help with this, that's awesome.
I'm almost afraid to make the next suggestion, but here goes:
In more complicated/wide-impact cases, it would be possible to
stage a data-member name conversion into "small-bang" iterations
using a C++ tactic like this:
class Foo {
int m\_bar; // The renamed member.
int &Bar = m\_bar; // TEMPORARY alias using the old name.
};
This would have to be done sparingly and for good reason, such as
when the names are known across many components/subprojects and
doing them all at once would be really too much. Someone would
have to commit to getting it all done and removing the aliases in
a reasonably short period of time. Needing to do this trick would
be (IMO) strong evidence of poor software design and a place to
focus some refactoring effort.
Honestly, I don't think we need to do this. We routinely make wide-ranging API updates. If we need to do that, we do that.
What we \*should\* do is encourage anyone that before they decide to do this to discuss it and see if there is a good way to hide this usage of a variable name behind a better API and make \*that\* widespread change instead. Then the name change is more local again.