<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">What cost do you see here? Rui has done a significant amount of work to make this effectively zero cost.<div class=""><br class=""></div><div class="">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”.</div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">I don’t see how this case is any different.</div><div class=""><br class=""></div><div class="">-Chris</div><div class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Sep 7, 2019, at 3:32 PM, Philip Reames via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" class="">
<div text="#000000" bgcolor="#FFFFFF" class=""><p class="">I do not support this. I feel the benefit is low, and the churn
cost is high. <br class="">
</p><p class="">I'm not strongly opposed or anything, I just don't believe this
is worthwhile.</p><p class="">Philip<br class="">
</p>
<div class="moz-cite-prefix">On 9/3/2019 8:14 PM, Rui Ueyama via
llvm-dev wrote:<br class="">
</div>
<blockquote type="cite" cite="mid:CAJENXgsJnc9uhg4WWfz+z3UgKmopj5BcdpYXuH6QXbAZaM4t-A@mail.gmail.com" class="">
<meta http-equiv="content-type" content="text/html; charset=UTF-8" class="">
<div dir="ltr" class="">Hi all,<br class="">
<br class="">
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 class="">
<br class="">
Background:<br class="">
<br class="">
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 class="">
<br class="">
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 class="">
<br class="">
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 class="">
<br class="">
Objectives:<br class="">
<br class="">
- Migrating the entire LLVM repo including subprojects to the
new naming scheme without breaking them.<br class="">
- 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 class="">
- The sweeping change shouldn't break `git blame`.<br class="">
<br class="">
What I learned from the lld's naming scheme change:<br class="">
<br class="">
- 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`.
<div class=""><br class="">
- 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.<br class="">
<br class="">
<div class=""> - 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.<br class="">
<br class="">
</div>
<div class=""> - 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 class="">
<br class="">
Migration plan:<br class="">
<br class="">
Given the above findings, I propose we migrate to the new
coding style in the following steps.<br class="">
<br class="">
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 class="">
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 class="">
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 class="">
4. Give a heads-up and submit a sweeping change to
clang-tools-extras, and make sure that that won't break
anything.<br class="">
5. Give a heads-up and submit a sweeping change to the
entire LLVM.<br class="">
<br class="">
I'd like to submit a sweeping change after LLVM migrates to
GitHub to minimize confusion.<br class="">
<br class="">
[1] <a href="http://lists.llvm.org/pipermail/llvm-dev/2019-February/130083.html" target="_blank" moz-do-not-send="true" class="">http://lists.llvm.org/pipermail/llvm-dev/2019-February/130083.html</a><br class="">
[2] <a href="https://github.com/llvm/llvm-project/tree/master/lld" target="_blank" moz-do-not-send="true" class="">https://github.com/llvm/llvm-project/tree/master/lld</a><br class="">
[3] <a href="https://github.com/llvm/llvm-project/commit/3837f4273fcc40cc519035479aefe78e5cbd3055" target="_blank" moz-do-not-send="true" class="">https://github.com/llvm/llvm-project/commit/3837f4273fcc40cc519035479aefe78e5cbd3055</a><br class="">
<br class="">
</div>
</div>
</div>
<br class="">
<fieldset class="mimeAttachmentHeader"></fieldset>
<pre class="moz-quote-pre" wrap="">_______________________________________________
LLVM Developers mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>
<a class="moz-txt-link-freetext" href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>
</pre>
</blockquote>
</div>
_______________________________________________<br class="">LLVM Developers mailing list<br class=""><a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a><br class="">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev<br class=""></div></blockquote></div><br class=""></div></body></html>