<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 14 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri","sans-serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
pre
{mso-style-priority:99;
mso-style-link:"HTML Preformatted Char";
margin:0in;
margin-bottom:.0001pt;
font-size:10.0pt;
font-family:"Courier New";}
span.HTMLPreformattedChar
{mso-style-name:"HTML Preformatted Char";
mso-style-priority:99;
mso-style-link:"HTML Preformatted";
font-family:Consolas;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri","sans-serif";}
span.EmailStyle20
{mso-style-type:personal;
font-family:"Calibri","sans-serif";
color:windowtext;}
span.EmailStyle21
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal">Krzysztof Parzyszek wrote:<o:p></o:p></p>
<p class="MsoNormal" style="margin-left:.5in">LLVM’s naming style is _<i>consistent</i>_<span style="color:#1F497D"><o:p></o:p></span></p>
<p class="MsoNormal"><a name="_MailEndCompose"><span style="color:#1F497D"><o:p> </o:p></span></a></p>
<p class="MsoNormal"><span style="color:#1F497D">Sorry, but *none* of the LLVM naming conventions are consistently used project-wide. There are lots of "historical" exceptions, and that is mainly because people got excited about "too much churn" which *enshrines
inconsistency.*<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">Applying a tool project-wide will be a step forward in consistency, which I personally think is a good thing. I had the sad experience, early in my career, of maintaining a code base where you might see 3 different
conventions used in the space of a dozen lines of code. LLVM isn't *that* bad, but moving from one library to another can make it easy to forget that there is, if only in principle, a naming convention.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">As a downstream maintainer, I'm well aware any change can cause trouble. People who do big-bang merges once a release (which we used to do) already have a lot to contend with; IMO the incremental pain is small
in that case. Rui's procedure for fixing up identifiers in a downstream repo did work for us, so I am comfortable saying the cost is annoying but not intolerable.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">Re. the upstream release branches, certainly waiting until after 9.0 is final would be best.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">--paulr<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> llvm-dev [mailto:llvm-dev-bounces@lists.llvm.org]
<b>On Behalf Of </b>Krzysztof Parzyszek via llvm-dev<br>
<b>Sent:</b> Monday, September 09, 2019 9:42 AM<br>
<b>To:</b> Chris Lattner; Philip Reames; llvm-dev@lists.llvm.org<br>
<b>Subject:</b> Re: [llvm-dev] [RFC] changing variable naming rules<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">LLVM has hundreds of downstream repos, including cases where LLVM is a part of some other project. There is no zero cost here.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Whether the changes result in meaningful improvements is debatable. They were motivated by arguments about code readability for people new to the project. While that’s important, getting acquainted with every new code base is a bit of
a challenge. LLVM’s naming style is _<i>consistent</i>_ and is easy to get used to (unless someone’s harboring a resentment for it).<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">“Too much churn” can be used as an excuse, but that doesn’t invalidate it as an argument. I think that in this case it is justified. Renaming variables is nothing like upgrading the code to use a C++14 feature, for example.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">If there is a last-ditch effort to stop this, I’m joining it.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal"><span style="font-size:9.0pt;font-family:Consolas">-- <o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:8.0pt;font-family:Consolas">Krzysztof Parzyszek
<a href="mailto:kparzysz@quicinc.com"><span style="color:#0563C1">kparzysz@quicinc.com</span></a> AI tools development<o:p></o:p></span></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> llvm-dev <llvm-dev-bounces@lists.llvm.org> <b>On Behalf Of
</b>Chris Lattner via llvm-dev<br>
<b>Sent:</b> Sunday, September 8, 2019 12:35 AM<br>
<b>To:</b> Philip Reames <listmail@philipreames.com><br>
<b>Cc:</b> llvm-dev <llvm-dev@lists.llvm.org><br>
<b>Subject:</b> [EXT] Re: [llvm-dev] [RFC] changing variable naming rules<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">What cost do you see here? Rui has done a significant amount of work to make this effectively zero cost.<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">The improvements are meaningful, and (as was discussed on the other threads) pretty much every large scale change in the LLVM world has been shot down with objections like “it is too much churn”.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">This is a huge problem, because it leads to stagnation in the codebase and does not allow modernization. LLVM has always had the development philosophy of "trying to be the best”, even if it comes at a cost. The unwillingness to maintain
a stable C++ API is one very significant aspect of this.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">I don’t see how this case is any different.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">-Chris<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><o:p> </o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">On Sep 7, 2019, at 3:32 PM, Philip Reames via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">I do not support this. I feel the benefit is low, and the churn cost is high.
<o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">I'm not strongly opposed or anything, I just don't believe this is worthwhile.<o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Philip<o:p></o:p></p>
<div>
<p class="MsoNormal">On 9/3/2019 8:14 PM, Rui Ueyama via llvm-dev wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">Hi all,<br>
<br>
To get wider visibility, build a broader consensus and address concerns on this topic, I'm again raising this as an RFC. This is a proposal to change the rule for variable names from CamelCase to camelBack _really this time_.<br>
<br>
Background:<br>
<br>
This has been proposed several times on this mailing list in the past. Most recent one was by Michael Platings in February this year [1], and there seems to be a general consensus that the status quo is not ideal.<br>
<br>
In the previous RFC thread, I nominated lld [2] as a starter project for renaming and made a sweeping change to rename variables in a few commits. This renaming went well -- even though it broke buildbots, I managed to unbreak them in a timely manner, and more
importantly, it has been reported that several downstream repos have successfully migrated to the new naming scheme using a tool that I wrote to create sweeping changes. That being said, some claimed that the renaming attempt didn't get enough attention, despite
being discussed in a thread that has 100+ emails. So I'm raising this topic as a new thread.<br>
<br>
I propose we do the same thing to another relatively small subproject, clang-tools-extras, to gain more experience, and then migrate the entire LLVM codebase to the new style. It seems technically doable, and even though it would cause a short-term pain, people
seem to feel more comfortable with the new naming scheme than the current one. I also believe that the migration won't be that painful.<br>
<br>
Objectives:<br>
<br>
- Migrating the entire LLVM repo including subprojects to the new naming scheme without breaking them.<br>
- Many projects, especially LLVM and Clang have lots of out-of-tree downstream repos. We need to provide a tool to rebase such repos to a commit after a renaming.<br>
- The sweeping change shouldn't break `git blame`.<br>
<br>
What I learned from the lld's naming scheme change:<br>
<br>
- There are many member variables in the LLVM/lld codebase that have the same name as accessors ignoring case (i.e. many classes define foo() as an accessor to a member variable Foo). Such variables would conflict with functions after renaming, so we had to
rename accessors by prepending `get`. <o:p></o:p></p>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><br>
- A single large sweeping change seemed to work better than small incremental changes for downstream repos. Downstream repo maintainers rebased their trees to a commit just prior to the sweeping change, apply my tool to rename all variables in their trees,
and then rebase the trees onto the sweeping change. Because the tool creates the same diffs for existing code, downstream maintainers basically only had to merge their diffs at the last step.<o:p></o:p></p>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"> - Even though my tool worked satisfactory, it couldn't rewrite code that are excluded by #if, #ifdef and the like, because the clang-based tool doesn't really see the code excluded by the preprocessor. That
caused several buildbot breakages.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"> - git 2.23 (released in August) added a new option `--ignore-revs` to `git blame` so that the command can take a list of commits that need to be ignored by blame. Developers can set a default ignore file (typically
named `.git-blame-ignore-revs`) using `git config` so that blame automatically ignores commits listed in the file. As far as I tried, that command worked pretty well to ignore the sweeping change I made to lld, so the `git blame` issue seems a solved problem
now.<br>
<br>
Migration plan:<br>
<br>
Given the above findings, I propose we migrate to the new coding style in the following steps.<br>
<br>
1. Change the codebase to eliminate name duplication between accessors and members. This can be done incrementally with as many commits as we want.<br>
2. Complete the tool and apply it to the entire LLVM tree. I'll publish it at GitHub so that people can take a look and try it out.<br>
3. Setup buildbots so that they checkout my GitHub tree, build it and run its tests, to make sure that a sweeping change won't break them. (I don't know how to configure buildbots, but I presume this step is doable.)<br>
4. Give a heads-up and submit a sweeping change to clang-tools-extras, and make sure that that won't break anything.<br>
5. Give a heads-up and submit a sweeping change to the entire LLVM.<br>
<br>
I'd like to submit a sweeping change after LLVM migrates to GitHub to minimize confusion.<br>
<br>
[1] <a href="http://lists.llvm.org/pipermail/llvm-dev/2019-February/130083.html" target="_blank">
http://lists.llvm.org/pipermail/llvm-dev/2019-February/130083.html</a><br>
[2] <a href="https://github.com/llvm/llvm-project/tree/master/lld" target="_blank">
https://github.com/llvm/llvm-project/tree/master/lld</a><br>
[3] <a href="https://github.com/llvm/llvm-project/commit/3837f4273fcc40cc519035479aefe78e5cbd3055" target="_blank">
https://github.com/llvm/llvm-project/commit/3837f4273fcc40cc519035479aefe78e5cbd3055</a><o:p></o:p></p>
</div>
</div>
</div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><o:p> </o:p></p>
<pre>_______________________________________________<o:p></o:p></pre>
<pre>LLVM Developers mailing list<o:p></o:p></pre>
<pre><a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a><o:p></o:p></pre>
<pre><a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><o:p></o:p></pre>
</blockquote>
</div>
<p class="MsoNormal">_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><o:p></o:p></p>
</div>
</blockquote>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</div>
</div>
</body>
</html>