<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 22 Feb 2019 at 20:46, via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
> -----Original Message-----<br>
> From: David Greene [mailto:<a href="mailto:dag@cray.com" target="_blank">dag@cray.com</a>]<br>
> Sent: Friday, February 22, 2019 2:40 PM<br>
> To: Robinson, Paul<br>
> Cc: <a href="mailto:asb@lowrisc.org" target="_blank">asb@lowrisc.org</a>; <a href="mailto:clattner@nondot.org" target="_blank">clattner@nondot.org</a>; <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>;<br>
> <a href="mailto:nd@arm.com" target="_blank">nd@arm.com</a><br>
> Subject: Re: [llvm-dev] RFC: changing variable naming rules in LLVM<br>
> codebase<br>
> <br>
> via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> writes:<br>
> <br>
> > Looking at names of "variables" there's reasonable support for making<br>
> > them visually more distinct from other kinds of names.  Regarding<br>
> > making data-member names distinct from local variables, some people<br>
> > don't see why it should matter, others find it helpful; given this<br>
> > neutral-to-helpful spectrum, going with the kind-of helpful convention<br>
> > seems preferable.<br>
> <br>
> There are drawbacks to the "m_" prefix notation, though.  It makes it<br>
> more work to move entities between class and local scope.  It's extra<br>
> typing, it's hard to pronounce, etc.<br>
<br>
IMO these are pretty feeble objections. The "m_" is silent, for example. <br>
Moving entities between class and local scope isn't all that common, and <br>
the small bit of extra typing provides clarity to your colleagues who <br>
like to know what data is ephemeral and what is persistent.  It's equally<br>
true that using real words instead of shorthand abbreviations is more<br>
typing, but we accept that for the clarity it brings.<br></blockquote><div> </div><div>You might treat "m_" as silent, but I don't. It's just not how my mind works when reading code. As for moving entities between class and local scope - I've found myself regularly going from local scope to class scope in the past in other projects at least, although I can't say the same for LLVM. I do know I'd get annoyed by typing m_* before every member variable I have to write, whereas I don't for using real words, but I accept that might just be me.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> Now, these may be considered trivial complaints, but given that some of<br>
> them have already been raised, I think we need some discussion about it.<br>
> If there's overwhelming support for "m_" (and/or "g_" etc.) then fine,<br>
> but if there's about even opinions either way, we ought to go with the<br>
> status quo, at least for now.<br>
<br>
I don't think "overwhelming" is an appropriate barrier to adoption. I<br>
hear people say "yes, this would make it easier for me to read the code"<br>
and on the other side "meh, I don't see the benefit"; however I do *not*<br>
see anyone saying "No, this really would interfere with my code-reading."<br>
<br>
> <br>
> > So:<br>
> ><br>
> > - Local variables and formal parameters should be lower_case, with<br>
> >   one exception: Variables/parameters that have lambda/function<br>
> >   type should follow the function-name spelling rules.<br>
> <br>
> "lower_case" is a pretty drastic change from CamelCase and camelCase.<br>
> So far the only argument for it I've seen is, "lldb uses it all over the<br>
> place."  Having one subproject drive the standard for everything else<br>
> seems backward.  It's certainly possible I missed other opinions on this<br>
> though.<br>
<br>
You have. For example, here's a "significant improvement" comment:<br>
<a href="http://lists.llvm.org/pipermail/llvm-dev/2019-February/130214.html" rel="noreferrer" target="_blank">http://lists.llvm.org/pipermail/llvm-dev/2019-February/130214.html</a><br>
<br>
The debate suffers from being spread across multiple dev threads plus<br>
a Phabricator review.  Not optimal, but that's what it is.  <br>
<br>
> <br>
> > - Initialisms and other abbreviations would be considered words for<br>
> >   this purpose, so we have names such as:<br>
> >     tli   // Local variable for TargetLoweringInfo<br>
> >     m_cgm // Data member for CodeGenModule<br>
> <br>
> I actually think we need something stronger.  Acronyms should be<br>
> discouraged unless it's absolutely clear what it is.  As has been<br>
> pointed out, "tli" means at least two common and very different things:<br>
> "TargetLoweringInfo" and "TargetLibraryInfo."<br>
<br>
That's a separate discussion and would be something to pursue regardless<br>
of the other spelling conventions. Let's not drag that in here.  My<br>
inclusion of this is based on "we find these things in names today, and<br>
they aren't 'words' so what do we do with them" not on any assessment<br>
that they are good or bad on their own merits.<br>
<br>
> <br>
> > - I don't have a good suggestion for file-static/global variables.<br>
> >   Some people have suggested a "g_" prefix for globals, or possibly<br>
> >   an "s_" prefix for class-static data.<br>
> <br>
> Is this really helpful or does it just make the code more difficult to<br>
> work with?  I don't know since I've never used such conventions before.<br>
> But I'm hesitant to introduce really strong binding of scopes to names<br>
> because it make refactoring more tedious/difficult.<br>
<br>
I have, but in a project that had rather more global data than is good<br>
for anyone. I put it out there for discussion, and thanks for doing<br>
exactly that!<br>
<br>
> <br>
> > Some people have worried that the churn will cause blame issues.<br>
> > I respectfully point out that in my own archaeology I have to deal<br>
> > with lots of clang-format/indentation/other random semantically<br>
> > meaningless refactoring, this is just one more.  Also the point is<br>
> > not to optimize for git-blame but to optimize for reading what is<br>
> > there at the moment.<br>
> <br>
> I want to call out that we already have a policy on this in the current<br>
> coding standards:<br>
> <br>
>   Our long term goal is for the entire codebase to follow the<br>
>   convention, but we explicitly do not want patches that do large-scale<br>
>   reformatting of existing code.<br>
> <br>
> I could argue things either way, but we should realize that a mechanical<br>
> reformatting goes explicitly against current stated (and presumably<br>
> previously-agreed-upon) policy.<br>
> <br>
>                                -David<br>
<br>
Right, but my impression is that people who lived through the last update<br>
to the spelling conventions are less enthused about a slow transition,<br>
this time around, and that bit of the convention would necessarily also<br>
be up for revision.<br>
<br>
--paulr<br>
<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div></div>