llvm.org GIT mirror llvm / 53583ed
[Documentation] Proposal to change variable names Differential Revision: https://reviews.llvm.org/D59251 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@357174 91177308-0d34-0410-b5e6-96231b3b80d8 Michael Platings 6 months ago
2 changed file(s) with 403 addition(s) and 0 deletion(s). Raw diff Collapse all Expand all
0 ===================
1 Variable Names Plan
2 ===================
3
4 .. contents::
5 :local:
6
7 This plan is *provisional*. It is not agreed upon. It is written with the
8 intention of capturing the desires and concerns of the LLVM community, and
9 forming them into a plan that can be agreed upon.
10 The original author is somewhat naïve in the ways of LLVM so there will
11 inevitably be some details that are flawed. You can help - you can edit this
12 page (preferably with a Phabricator review for larger changes) or reply to the
13 `Request For Comments thread
14 `_.
15
16 Too Long; Didn't Read
17 =====================
18
19 Improve the readability of LLVM code.
20
21 Introduction
22 ============
23
24 The current `variable naming rule
25 <../CodingStandards.html#name-types-functions-variables-and-enumerators-properly>`_
26 states:
27
28 Variable names should be nouns (as they represent state). The name should be
29 camel case, and start with an upper case letter (e.g. Leader or Boats).
30
31 This rule is the same as that for type names. This is a problem because the
32 type name cannot be reused for a variable name [*]_. LLVM developers tend to
33 work around this by either prepending ``The`` to the type name::
34
35 Triple TheTriple;
36
37 ... or more commonly use an acronym, despite the coding standard stating "Avoid
38 abbreviations unless they are well known"::
39
40 Triple T;
41
42 The proliferation of acronyms leads to hard-to-read code such as `this
43 `_::
44
45 InnerLoopVectorizer LB(L, PSE, LI, DT, TLI, TTI, AC, ORE, VF.Width, IC,
46 &LVL, &CM);
47
48 Many other coding guidelines [LLDB]_ [Google]_ [WebKit]_ [Qt]_ [Rust]_ [Swift]_
49 [Python]_ require that variable names begin with a lower case letter in contrast
50 to class names which begin with a capital letter. This convention means that the
51 most readable variable name also requires the least thought::
52
53 Triple triple;
54
55 There is some agreement that the current rule is broken [LattnerAgree]_
56 [ArsenaultAgree]_ [RobinsonAgree]_ and that acronyms are an obstacle to reading
57 new code [MalyutinDistinguish]_ [CarruthAcronym]_ [PicusAcronym]_. There are
58 some opposing views [ParzyszekAcronym2]_ [RicciAcronyms]_.
59
60 This work-in-progress proposal is to change the coding standard for variable
61 names to require that they start with a lower case letter.
62
63 .. [*] In `some cases
64 `_
65 the type name *is* reused as a variable name, but this shadows the type name
66 and confuses many debuggers [DenisovCamelBack]_.
67
68 Variable Names Coding Standard Options
69 ======================================
70
71 There are two main options for variable names that begin with a lower case
72 letter: ``camelBack`` and ``lower_case``. (These are also known by other names
73 but here we use the terminology from clang-tidy).
74
75 ``camelBack`` is consistent with [WebKit]_, [Qt]_ and [Swift]_ while
76 ``lower_case`` is consistent with [LLDB]_, [Google]_, [Rust]_ and [Python]_.
77
78 ``camelBack`` is already used for function names, which may be considered an
79 advantage [LattnerFunction]_ or a disadvantage [CarruthFunction]_.
80
81 Approval for ``camelBack`` was expressed by [DenisovCamelBack]_
82 [LattnerFunction]_ [IvanovicDistinguish]_.
83 Opposition to ``camelBack`` was expressed by [CarruthCamelBack]_
84 [TurnerCamelBack]_.
85 Approval for ``lower_case`` was expressed by [CarruthLower]_
86 [CarruthCamelBack]_ [TurnerLLDB]_.
87 Opposition to ``lower_case`` was expressed by [LattnerLower]_.
88
89 Differentiating variable kinds
90 ------------------------------
91
92 An additional requested change is to distinguish between different kinds of
93 variables [RobinsonDistinguish]_ [RobinsonDistinguish2]_ [JonesDistinguish]_
94 [IvanovicDistinguish]_ [CarruthDistinguish]_ [MalyutinDistinguish]_.
95
96 Others oppose this idea [HähnleDistinguish]_ [GreeneDistinguish]_
97 [HendersonPrefix]_.
98
99 A possibility is for member variables to be prefixed with ``m_`` and for global
100 variables to be prefixed with ``g_`` to distinguish them from local variables.
101 This is consistent with [LLDB]_. The ``m_`` prefix is consistent with [WebKit]_.
102
103 A variation is for member variables to be prefixed with ``m``
104 [IvanovicDistinguish]_ [BeylsDistinguish]_. This is consistent with [Mozilla]_.
105
106 Another option is for member variables to be suffixed with ``_`` which is
107 consistent with [Google]_ and similar to [Python]_. Opposed by
108 [ParzyszekDistinguish]_.
109
110 Reducing the number of acronyms
111 ===============================
112
113 While switching coding standard will make it easier to use non-acronym names for
114 new code, it doesn't improve the existing large body of code that uses acronyms
115 extensively to the detriment of its readability. Further, it is natural and
116 generally encouraged that new code be written in the style of the surrounding
117 code. Therefore it is likely that much newly written code will also use
118 acronyms despite what the coding standard says, much as it is today.
119
120 As well as changing the case of variable names, they could also be expanded to
121 their non-acronym form e.g. ``Triple T`` → ``Triple triple``.
122
123 There is support for expanding many acronyms [CarruthAcronym]_ [PicusAcronym]_
124 but there is a preference that expanding acronyms be deferred
125 [ParzyszekAcronym]_ [CarruthAcronym]_.
126
127 The consensus within the community seems to be that at least some acronyms are
128 valuable [ParzyszekAcronym]_ [LattnerAcronym]_. The most commonly cited acronym
129 is ``TLI`` however that is used to refer to both ``TargetLowering`` and
130 ``TargetLibraryInfo`` [GreeneDistinguish]_.
131
132 The following is a list of acronyms considered sufficiently useful that the
133 benefit of using them outweighs the cost of learning them. Acronyms that are
134 either not on the list or are used to refer to a different type should be
135 expanded.
136
137 ============================ =============
138 Class name Variable name
139 ============================ =============
140 DeterministicFiniteAutomaton dfa
141 DominatorTree dt
142 LoopInfo li
143 MachineFunction mf
144 MachineInstr mi
145 MachineRegisterInfo mri
146 ScalarEvolution se
147 TargetInstrInfo tii
148 TargetLibraryInfo tli
149 TargetRegisterInfo tri
150 ============================ =============
151
152 In some cases renaming acronyms to the full type name will result in overly
153 verbose code. Unlike most classes, a variable's scope is limited and therefore
154 some of its purpose can implied from that scope, meaning that fewer words are
155 necessary to give it a clear name. For example, in an optization pass the reader
156 can assume that a variable's purpose relates to optimization and therefore an
157 ``OptimizationRemarkEmitter`` variable could be given the name ``remarkEmitter``
158 or even ``remarker``.
159
160 The following is a list of longer class names and the associated shorter
161 variable name.
162
163 ========================= =============
164 Class name Variable name
165 ========================= =============
166 BasicBlock block
167 ConstantExpr expr
168 ExecutionEngine engine
169 MachineOperand operand
170 OptimizationRemarkEmitter remarker
171 PreservedAnalyses analyses
172 PreservedAnalysesChecker checker
173 TargetLowering lowering
174 TargetMachine machine
175 ========================= =============
176
177 Transition Options
178 ==================
179
180 There are three main options for transitioning:
181
182 1. Keep the current coding standard
183 2. Laissez faire
184 3. Big bang
185
186 Keep the current coding standard
187 --------------------------------
188
189 Proponents of keeping the current coding standard (i.e. not transitioning at
190 all) question whether the cost of transition outweighs the benefit
191 [EmersonConcern]_ [ReamesConcern]_ [BradburyConcern]_.
192 The costs are that ``git blame`` will become less usable; and that merging the
193 changes will be costly for downstream maintainers. See `Big bang`_ for potential
194 mitigations.
195
196 Laissez faire
197 -------------
198
199 The coding standard could allow both ``CamelCase`` and ``camelBack`` styles for
200 variable names [LattnerTransition]_.
201
202 A code review to implement this is at https://reviews.llvm.org/D57896.
203
204 Advantages
205 **********
206
207 * Very easy to implement initially.
208
209 Disadvantages
210 *************
211
212 * Leads to inconsistency [BradburyConcern]_ [AminiInconsistent]_.
213 * Inconsistency means it will be hard to know at a guess what name a variable
214 will have [DasInconsistent]_ [CarruthInconsistent]_.
215 * Some large-scale renaming may happen anyway, leading to its disadvantages
216 without any mitigations.
217
218 Big bang
219 --------
220
221 With this approach, variables will be renamed by an automated script in a series
222 of large commits.
223
224 The principle advantage of this approach is that it minimises the cost of
225 inconsistency [BradburyTransition]_ [RobinsonTransition]_.
226
227 It goes against a policy of avoiding large-scale reformatting of existing code
228 [GreeneDistinguish]_.
229
230 It has been suggested that LLD would be a good starter project for the renaming
231 [Ueyama]_.
232
233 Keeping git blame usable
234 ************************
235
236 ``git blame`` (or ``git annotate``) permits quickly identifying the commit that
237 changed a given line in a file. After renaming variables, many lines will show
238 as being changed by that one commit, requiring a further invocation of ``git
239 blame`` to identify prior, more interesting commits [GreeneGitBlame]_
240 [RicciAcronyms]_.
241
242 **Mitigation**: `git-hyper-blame
243 `_
244 can ignore or "look through" a given set of commits.
245 A ``.git-blame-ignore-revs`` file identifying the variable renaming commits
246 could be added to the LLVM git repository root directory.
247 It is being `investigated
248 `_
249 whether similar functionality could be added to ``git blame`` itself.
250
251 Minimising cost of downstream merges
252 ************************************
253
254 There are many forks of LLVM with downstream changes. Merging a large-scale
255 renaming change could be difficult for the fork maintainers.
256
257 **Mitigation**: A large-scale renaming would be automated. A fork maintainer can
258 merge from the commit immediately before the renaming, then apply the renaming
259 script to their own branch. They can then merge again from the renaming commit,
260 resolving all conflicts by choosing their own version. This could be tested on
261 the [SVE]_ fork.
262
263 Provisional Plan
264 ================
265
266 This is a provisional plan for the `Big bang`_ approach. It has not been agreed.
267
268 #. Investigate improving ``git blame``. The extent to which it can be made to
269 "look through" commits may impact how big a change can be made.
270
271 #. Write a script to expand acronyms.
272
273 #. Experiment and perform dry runs of the various refactoring options.
274 Results can be published in forks of the LLVM Git repository.
275
276 #. Consider the evidence and agree on the new policy.
277
278 #. Agree & announce a date for the renaming of the starter project (LLD).
279
280 #. Update the `policy page <../CodingStandards.html>`_. This will explain the
281 old and new rules and which projects each applies to.
282
283 #. Refactor the starter project in two commits:
284
285 1. Add or change the project's .clang-tidy to reflect the agreed rules.
286 (This is in a separate commit to enable the merging process described in
287 `Minimising cost of downstream merges`_).
288 Also update the project list on the policy page.
289 2. Apply ``clang-tidy`` to the project's files, with only the
290 ``readability-identifier-naming`` rules enabled. ``clang-tidy`` will also
291 reformat the affected lines according to the rules in ``.clang-format``.
292 It is anticipated that this will be a good dog-fooding opportunity for
293 clang-tidy, and bugs should be fixed in the process, likely including:
294
295 * `readability-identifier-naming incorrectly fixes lambda capture
296 `_.
297 * `readability-identifier-naming incorrectly fixes variables which
298 become keywords `_.
299 * `readability-identifier-naming misses fixing member variables in
300 destructor `_.
301
302 #. Gather feedback and refine the process as appropriate.
303
304 #. Apply the process to the following projects, with a suitable delay between
305 each (at least 4 weeks after the first change, at least 2 weeks subsequently)
306 to allow gathering further feedback.
307 This list should exclude projects that must adhere to an externally defined
308 standard e.g. libcxx.
309 The list is roughly in chronological order of renaming.
310 Some items may not make sense to rename individually - it is expected that
311 this list will change following experimentation:
312
313 * TableGen
314 * llvm/tools
315 * clang-tools-extra
316 * clang
317 * ARM backend
318 * AArch64 backend
319 * AMDGPU backend
320 * ARC backend
321 * AVR backend
322 * BPF backend
323 * Hexagon backend
324 * Lanai backend
325 * MIPS backend
326 * NVPTX backend
327 * PowerPC backend
328 * RISC-V backend
329 * Sparc backend
330 * SystemZ backend
331 * WebAssembly backend
332 * X86 backend
333 * XCore backend
334 * libLTO
335 * Debug Information
336 * Remainder of llvm
337 * compiler-rt
338 * libunwind
339 * openmp
340 * parallel-libs
341 * polly
342 * lldb
343
344 #. Remove the old variable name rule from the policy page.
345
346 #. Repeat many of the steps in the sequence, using a script to expand acronyms.
347
348 References
349 ==========
350
351 .. [LLDB] LLDB Coding Conventions https://llvm.org/svn/llvm-project/lldb/branches/release_39/www/lldb-coding-conventions.html
352 .. [Google] Google C++ Style Guide https://google.github.io/styleguide/cppguide.html#Variable_Names
353 .. [WebKit] WebKit Code Style Guidelines https://webkit.org/code-style-guidelines/#names
354 .. [Qt] Qt Coding Style https://wiki.qt.io/Qt_Coding_Style#Declaring_variables
355 .. [Rust] Rust naming conventions https://doc.rust-lang.org/1.0.0/style/style/naming/README.html
356 .. [Swift] Swift API Design Guidelines https://swift.org/documentation/api-design-guidelines/#general-conventions
357 .. [Python] Style Guide for Python Code https://www.python.org/dev/peps/pep-0008/#function-and-variable-names
358 .. [Mozilla] Mozilla Coding style: Prefixes https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Prefixes
359 .. [SVE] LLVM with support for SVE https://github.com/ARM-software/LLVM-SVE
360 .. [AminiInconsistent] Mehdi Amini, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130329.html
361 .. [ArsenaultAgree] Matt Arsenault, http://lists.llvm.org/pipermail/llvm-dev/2019-February/129934.html
362 .. [BeylsDistinguish] Kristof Beyls, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130292.html
363 .. [BradburyConcern] Alex Bradbury, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130266.html
364 .. [BradburyTransition] Alex Bradbury, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130388.html
365 .. [CarruthAcronym] Chandler Carruth, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130313.html
366 .. [CarruthCamelBack] Chandler Carruth, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130214.html
367 .. [CarruthDistinguish] Chandler Carruth, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130310.html
368 .. [CarruthFunction] Chandler Carruth, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130309.html
369 .. [CarruthInconsistent] Chandler Carruth, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130312.html
370 .. [CarruthLower] Chandler Carruth, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130430.html
371 .. [DasInconsistent] Sanjoy Das, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130304.html
372 .. [DenisovCamelBack] Alex Denisov, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130179.html
373 .. [EmersonConcern] Amara Emerson, http://lists.llvm.org/pipermail/llvm-dev/2019-February/129894.html
374 .. [GreeneDistinguish] David Greene, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130425.html
375 .. [GreeneGitBlame] David Greene, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130228.html
376 .. [HendersonPrefix] James Henderson, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130465.html
377 .. [HähnleDistinguish] Nicolai Hähnle, http://lists.llvm.org/pipermail/llvm-dev/2019-February/129923.html
378 .. [IvanovicDistinguish] Nemanja Ivanovic, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130249.html
379 .. [JonesDistinguish] JD Jones, http://lists.llvm.org/pipermail/llvm-dev/2019-February/129926.html
380 .. [LattnerAcronym] Chris Lattner, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130353.html
381 .. [LattnerAgree] Chris Latter, http://lists.llvm.org/pipermail/llvm-dev/2019-February/129907.html
382 .. [LattnerFunction] Chris Lattner, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130630.html
383 .. [LattnerLower] Chris Lattner, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130629.html
384 .. [LattnerTransition] Chris Lattner, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130355.html
385 .. [MalyutinDistinguish] Danila Malyutin, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130320.html
386 .. [ParzyszekAcronym] Krzysztof Parzyszek, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130306.html
387 .. [ParzyszekAcronym2] Krzysztof Parzyszek, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130323.html
388 .. [ParzyszekDistinguish] Krzysztof Parzyszek, http://lists.llvm.org/pipermail/llvm-dev/2019-February/129941.html
389 .. [PicusAcronym] Diana Picus, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130318.html
390 .. [ReamesConcern] Philip Reames, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130181.html
391 .. [RicciAcronyms] Bruno Ricci, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130328.html
392 .. [RobinsonAgree] Paul Robinson, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130111.html
393 .. [RobinsonDistinguish] Paul Robinson, http://lists.llvm.org/pipermail/llvm-dev/2019-February/129920.html
394 .. [RobinsonDistinguish2] Paul Robinson, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130229.html
395 .. [RobinsonTransition] Paul Robinson, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130415.html
396 .. [TurnerCamelBack] Zachary Turner, https://reviews.llvm.org/D57896#1402264
397 .. [TurnerLLDB] Zachary Turner, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130213.html
398 .. [Ueyama] Rui Ueyama, http://lists.llvm.org/pipermail/llvm-dev/2019-February/130435.html
571571 CodeOfConduct
572572 Proposals/GitHubMove
573573 Proposals/TestSuite
574 Proposals/VariableNames
574575 Proposals/VectorizationPlan
575576
576577 :doc:`CodeOfConduct`
583584 :doc:`Proposals/TestSuite`
584585 Proposals for additional benchmarks/programs for llvm's test-suite.
585586
587 :doc:`Proposals/VariableNames`
588 Proposal to change the variable names coding standard.
589
586590 :doc:`Proposals/VectorizationPlan`
587591 Proposal to model the process and upgrade the infrastructure of LLVM's Loop Vectorizer.
588592