]>
Commit | Line | Data |
---|---|---|
223e47cc LB |
1 | ===================== |
2 | LLVM Coding Standards | |
3 | ===================== | |
4 | ||
5 | .. contents:: | |
6 | :local: | |
7 | ||
8 | Introduction | |
9 | ============ | |
10 | ||
11 | This document attempts to describe a few coding standards that are being used in | |
12 | the LLVM source tree. Although no coding standards should be regarded as | |
13 | absolute requirements to be followed in all instances, coding standards are | |
14 | particularly important for large-scale code bases that follow a library-based | |
15 | design (like LLVM). | |
16 | ||
1a4d82fc JJ |
17 | While this document may provide guidance for some mechanical formatting issues, |
18 | whitespace, or other "microscopic details", these are not fixed standards. | |
19 | Always follow the golden rule: | |
223e47cc LB |
20 | |
21 | .. _Golden Rule: | |
22 | ||
23 | **If you are extending, enhancing, or bug fixing already implemented code, | |
24 | use the style that is already being used so that the source is uniform and | |
25 | easy to follow.** | |
26 | ||
27 | Note that some code bases (e.g. ``libc++``) have really good reasons to deviate | |
28 | from the coding standards. In the case of ``libc++``, this is because the | |
29 | naming and other conventions are dictated by the C++ standard. If you think | |
30 | there is a specific good reason to deviate from the standards here, please bring | |
31 | it up on the LLVMdev mailing list. | |
32 | ||
33 | There are some conventions that are not uniformly followed in the code base | |
34 | (e.g. the naming convention). This is because they are relatively new, and a | |
35 | lot of code was written before they were put in place. Our long term goal is | |
36 | for the entire codebase to follow the convention, but we explicitly *do not* | |
37 | want patches that do large-scale reformating of existing code. On the other | |
38 | hand, it is reasonable to rename the methods of a class if you're about to | |
39 | change it in some other way. Just do the reformating as a separate commit from | |
40 | the functionality change. | |
41 | ||
42 | The ultimate goal of these guidelines is the increase readability and | |
43 | maintainability of our common source base. If you have suggestions for topics to | |
44 | be included, please mail them to `Chris <mailto:sabre@nondot.org>`_. | |
45 | ||
1a4d82fc JJ |
46 | Languages, Libraries, and Standards |
47 | =================================== | |
48 | ||
49 | Most source code in LLVM and other LLVM projects using these coding standards | |
50 | is C++ code. There are some places where C code is used either due to | |
51 | environment restrictions, historical restrictions, or due to third-party source | |
52 | code imported into the tree. Generally, our preference is for standards | |
53 | conforming, modern, and portable C++ code as the implementation language of | |
54 | choice. | |
55 | ||
56 | C++ Standard Versions | |
57 | --------------------- | |
58 | ||
59 | LLVM, Clang, and LLD are currently written using C++11 conforming code, | |
60 | although we restrict ourselves to features which are available in the major | |
61 | toolchains supported as host compilers. The LLDB project is even more | |
62 | aggressive in the set of host compilers supported and thus uses still more | |
63 | features. Regardless of the supported features, code is expected to (when | |
64 | reasonable) be standard, portable, and modern C++11 code. We avoid unnecessary | |
65 | vendor-specific extensions, etc. | |
66 | ||
67 | C++ Standard Library | |
68 | -------------------- | |
69 | ||
70 | Use the C++ standard library facilities whenever they are available for | |
71 | a particular task. LLVM and related projects emphasize and rely on the standard | |
72 | library facilities for as much as possible. Common support libraries providing | |
73 | functionality missing from the standard library for which there are standard | |
74 | interfaces or active work on adding standard interfaces will often be | |
75 | implemented in the LLVM namespace following the expected standard interface. | |
76 | ||
77 | There are some exceptions such as the standard I/O streams library which are | |
78 | avoided. Also, there is much more detailed information on these subjects in the | |
79 | :doc:`ProgrammersManual`. | |
80 | ||
81 | Supported C++11 Language and Library Features | |
82 | --------------------------------------------- | |
83 | ||
84 | While LLVM, Clang, and LLD use C++11, not all features are available in all of | |
85 | the toolchains which we support. The set of features supported for use in LLVM | |
86 | is the intersection of those supported in MSVC 2012, GCC 4.7, and Clang 3.1. | |
87 | The ultimate definition of this set is what build bots with those respective | |
88 | toolchains accept. Don't argue with the build bots. However, we have some | |
89 | guidance below to help you know what to expect. | |
90 | ||
91 | Each toolchain provides a good reference for what it accepts: | |
92 | ||
93 | * Clang: http://clang.llvm.org/cxx_status.html | |
94 | * GCC: http://gcc.gnu.org/projects/cxx0x.html | |
95 | * MSVC: http://msdn.microsoft.com/en-us/library/hh567368.aspx | |
96 | ||
97 | In most cases, the MSVC list will be the dominating factor. Here is a summary | |
98 | of the features that are expected to work. Features not on this list are | |
99 | unlikely to be supported by our host compilers. | |
100 | ||
101 | * Rvalue references: N2118_ | |
102 | ||
103 | * But *not* Rvalue references for ``*this`` or member qualifiers (N2439_) | |
104 | ||
105 | * Static assert: N1720_ | |
106 | * ``auto`` type deduction: N1984_, N1737_ | |
107 | * Trailing return types: N2541_ | |
108 | * Lambdas: N2927_ | |
109 | ||
110 | * But *not* lambdas with default arguments. | |
111 | ||
112 | * ``decltype``: N2343_ | |
113 | * Nested closing right angle brackets: N1757_ | |
114 | * Extern templates: N1987_ | |
115 | * ``nullptr``: N2431_ | |
116 | * Strongly-typed and forward declarable enums: N2347_, N2764_ | |
117 | * Local and unnamed types as template arguments: N2657_ | |
118 | * Range-based for-loop: N2930_ | |
119 | ||
120 | * But ``{}`` are required around inner ``do {} while()`` loops. As a result, | |
121 | ``{}`` are required around function-like macros inside range-based for | |
122 | loops. | |
123 | ||
124 | * ``override`` and ``final``: N2928_, N3206_, N3272_ | |
125 | * Atomic operations and the C++11 memory model: N2429_ | |
126 | ||
127 | .. _N2118: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2118.html | |
128 | .. _N2439: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2439.htm | |
129 | .. _N1720: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2004/n1720.html | |
130 | .. _N1984: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n1984.pdf | |
131 | .. _N1737: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2004/n1737.pdf | |
132 | .. _N2541: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2541.htm | |
133 | .. _N2927: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2009/n2927.pdf | |
134 | .. _N2343: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2343.pdf | |
135 | .. _N1757: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2005/n1757.html | |
136 | .. _N1987: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n1987.htm | |
137 | .. _N2431: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2431.pdf | |
138 | .. _N2347: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2347.pdf | |
139 | .. _N2764: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2764.pdf | |
140 | .. _N2657: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2657.htm | |
141 | .. _N2930: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2009/n2930.html | |
142 | .. _N2928: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2009/n2928.htm | |
143 | .. _N3206: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2010/n3206.htm | |
144 | .. _N3272: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2011/n3272.htm | |
145 | .. _N2429: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2429.htm | |
146 | .. _MSVC-compatible RTTI: http://llvm.org/PR18951 | |
147 | ||
148 | The supported features in the C++11 standard libraries are less well tracked, | |
149 | but also much greater. Most of the standard libraries implement most of C++11's | |
150 | library. The most likely lowest common denominator is Linux support. For | |
151 | libc++, the support is just poorly tested and undocumented but expected to be | |
152 | largely complete. YMMV. For libstdc++, the support is documented in detail in | |
153 | `the libstdc++ manual`_. There are some very minor missing facilities that are | |
154 | unlikely to be common problems, and there are a few larger gaps that are worth | |
155 | being aware of: | |
156 | ||
157 | * Not all of the type traits are implemented | |
158 | * No regular expression library. | |
159 | * While most of the atomics library is well implemented, the fences are | |
160 | missing. Fortunately, they are rarely needed. | |
161 | * The locale support is incomplete. | |
162 | * ``std::initializer_list`` (and the constructors and functions that take it as | |
163 | an argument) are not always available, so you cannot (for example) initialize | |
164 | a ``std::vector`` with a braced initializer list. | |
165 | * ``std::equal()`` (and other algorithms) incorrectly assert in MSVC when given | |
166 | ``nullptr`` as an iterator. | |
167 | ||
168 | Other than these areas you should assume the standard library is available and | |
169 | working as expected until some build bot tells you otherwise. If you're in an | |
170 | uncertain area of one of the above points, but you cannot test on a Linux | |
171 | system, your best approach is to minimize your use of these features, and watch | |
172 | the Linux build bots to find out if your usage triggered a bug. For example, if | |
173 | you hit a type trait which doesn't work we can then add support to LLVM's | |
174 | traits header to emulate it. | |
175 | ||
176 | .. _the libstdc++ manual: | |
177 | http://gcc.gnu.org/onlinedocs/gcc-4.7.3/libstdc++/manual/manual/status.html#status.iso.2011 | |
178 | ||
85aaf69f SL |
179 | Other Languages |
180 | --------------- | |
181 | ||
182 | Any code written in the Go programming language is not subject to the | |
183 | formatting rules below. Instead, we adopt the formatting rules enforced by | |
184 | the `gofmt`_ tool. | |
185 | ||
186 | Go code should strive to be idiomatic. Two good sets of guidelines for what | |
187 | this means are `Effective Go`_ and `Go Code Review Comments`_. | |
188 | ||
189 | .. _gofmt: | |
190 | https://golang.org/cmd/gofmt/ | |
191 | ||
192 | .. _Effective Go: | |
193 | https://golang.org/doc/effective_go.html | |
194 | ||
195 | .. _Go Code Review Comments: | |
196 | https://code.google.com/p/go-wiki/wiki/CodeReviewComments | |
197 | ||
223e47cc LB |
198 | Mechanical Source Issues |
199 | ======================== | |
200 | ||
201 | Source Code Formatting | |
202 | ---------------------- | |
203 | ||
204 | Commenting | |
205 | ^^^^^^^^^^ | |
206 | ||
207 | Comments are one critical part of readability and maintainability. Everyone | |
208 | knows they should comment their code, and so should you. When writing comments, | |
209 | write them as English prose, which means they should use proper capitalization, | |
210 | punctuation, etc. Aim to describe what the code is trying to do and why, not | |
211 | *how* it does it at a micro level. Here are a few critical things to document: | |
212 | ||
213 | .. _header file comment: | |
214 | ||
215 | File Headers | |
216 | """""""""""" | |
217 | ||
218 | Every source file should have a header on it that describes the basic purpose of | |
219 | the file. If a file does not have a header, it should not be checked into the | |
220 | tree. The standard header looks like this: | |
221 | ||
222 | .. code-block:: c++ | |
223 | ||
224 | //===-- llvm/Instruction.h - Instruction class definition -------*- C++ -*-===// | |
225 | // | |
226 | // The LLVM Compiler Infrastructure | |
227 | // | |
228 | // This file is distributed under the University of Illinois Open Source | |
229 | // License. See LICENSE.TXT for details. | |
230 | // | |
231 | //===----------------------------------------------------------------------===// | |
970d7e83 LB |
232 | /// |
233 | /// \file | |
234 | /// \brief This file contains the declaration of the Instruction class, which is | |
235 | /// the base class for all of the VM instructions. | |
236 | /// | |
223e47cc LB |
237 | //===----------------------------------------------------------------------===// |
238 | ||
239 | A few things to note about this particular format: The "``-*- C++ -*-``" string | |
240 | on the first line is there to tell Emacs that the source file is a C++ file, not | |
241 | a C file (Emacs assumes ``.h`` files are C files by default). | |
242 | ||
243 | .. note:: | |
244 | ||
245 | This tag is not necessary in ``.cpp`` files. The name of the file is also | |
246 | on the first line, along with a very short description of the purpose of the | |
247 | file. This is important when printing out code and flipping though lots of | |
248 | pages. | |
249 | ||
250 | The next section in the file is a concise note that defines the license that the | |
251 | file is released under. This makes it perfectly clear what terms the source | |
252 | code can be distributed under and should not be modified in any way. | |
253 | ||
970d7e83 LB |
254 | The main body is a ``doxygen`` comment describing the purpose of the file. It |
255 | should have a ``\brief`` command that describes the file in one or two | |
256 | sentences. Any additional information should be separated by a blank line. If | |
257 | an algorithm is being implemented or something tricky is going on, a reference | |
258 | to the paper where it is published should be included, as well as any notes or | |
259 | *gotchas* in the code to watch out for. | |
223e47cc LB |
260 | |
261 | Class overviews | |
262 | """"""""""""""" | |
263 | ||
264 | Classes are one fundamental part of a good object oriented design. As such, a | |
265 | class definition should have a comment block that explains what the class is | |
266 | used for and how it works. Every non-trivial class is expected to have a | |
267 | ``doxygen`` comment block. | |
268 | ||
269 | Method information | |
270 | """""""""""""""""" | |
271 | ||
272 | Methods defined in a class (as well as any global functions) should also be | |
273 | documented properly. A quick note about what it does and a description of the | |
274 | borderline behaviour is all that is necessary here (unless something | |
275 | particularly tricky or insidious is going on). The hope is that people can | |
276 | figure out how to use your interfaces without reading the code itself. | |
277 | ||
278 | Good things to talk about here are what happens when something unexpected | |
279 | happens: does the method return null? Abort? Format your hard disk? | |
280 | ||
281 | Comment Formatting | |
282 | ^^^^^^^^^^^^^^^^^^ | |
283 | ||
284 | In general, prefer C++ style (``//``) comments. They take less space, require | |
285 | less typing, don't have nesting problems, etc. There are a few cases when it is | |
286 | useful to use C style (``/* */``) comments however: | |
287 | ||
288 | #. When writing C code: Obviously if you are writing C code, use C style | |
289 | comments. | |
290 | ||
291 | #. When writing a header file that may be ``#include``\d by a C source file. | |
292 | ||
293 | #. When writing a source file that is used by a tool that only accepts C style | |
294 | comments. | |
295 | ||
296 | To comment out a large block of code, use ``#if 0`` and ``#endif``. These nest | |
297 | properly and are better behaved in general than C style comments. | |
298 | ||
970d7e83 LB |
299 | Doxygen Use in Documentation Comments |
300 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
301 | ||
302 | Use the ``\file`` command to turn the standard file header into a file-level | |
303 | comment. | |
304 | ||
305 | Include descriptive ``\brief`` paragraphs for all public interfaces (public | |
306 | classes, member and non-member functions). Explain API use and purpose in | |
307 | ``\brief`` paragraphs, don't just restate the information that can be inferred | |
308 | from the API name. Put detailed discussion into separate paragraphs. | |
309 | ||
310 | To refer to parameter names inside a paragraph, use the ``\p name`` command. | |
311 | Don't use the ``\arg name`` command since it starts a new paragraph that | |
312 | contains documentation for the parameter. | |
313 | ||
314 | Wrap non-inline code examples in ``\code ... \endcode``. | |
315 | ||
316 | To document a function parameter, start a new paragraph with the | |
317 | ``\param name`` command. If the parameter is used as an out or an in/out | |
318 | parameter, use the ``\param [out] name`` or ``\param [in,out] name`` command, | |
319 | respectively. | |
320 | ||
321 | To describe function return value, start a new paragraph with the ``\returns`` | |
322 | command. | |
323 | ||
324 | A minimal documentation comment: | |
325 | ||
326 | .. code-block:: c++ | |
327 | ||
328 | /// \brief Does foo and bar. | |
329 | void fooBar(bool Baz); | |
330 | ||
331 | A documentation comment that uses all Doxygen features in a preferred way: | |
332 | ||
333 | .. code-block:: c++ | |
334 | ||
335 | /// \brief Does foo and bar. | |
336 | /// | |
337 | /// Does not do foo the usual way if \p Baz is true. | |
338 | /// | |
339 | /// Typical usage: | |
340 | /// \code | |
341 | /// fooBar(false, "quux", Res); | |
342 | /// \endcode | |
343 | /// | |
344 | /// \param Quux kind of foo to do. | |
345 | /// \param [out] Result filled with bar sequence on foo success. | |
346 | /// | |
347 | /// \returns true on success. | |
348 | bool fooBar(bool Baz, StringRef Quux, std::vector<int> &Result); | |
349 | ||
350 | Don't duplicate the documentation comment in the header file and in the | |
351 | implementation file. Put the documentation comments for public APIs into the | |
352 | header file. Documentation comments for private APIs can go to the | |
353 | implementation file. In any case, implementation files can include additional | |
354 | comments (not necessarily in Doxygen markup) to explain implementation details | |
355 | as needed. | |
356 | ||
357 | Don't duplicate function or class name at the beginning of the comment. | |
358 | For humans it is obvious which function or class is being documented; | |
359 | automatic documentation processing tools are smart enough to bind the comment | |
360 | to the correct declaration. | |
361 | ||
362 | Wrong: | |
363 | ||
364 | .. code-block:: c++ | |
365 | ||
366 | // In Something.h: | |
367 | ||
368 | /// Something - An abstraction for some complicated thing. | |
369 | class Something { | |
370 | public: | |
371 | /// fooBar - Does foo and bar. | |
372 | void fooBar(); | |
373 | }; | |
374 | ||
375 | // In Something.cpp: | |
376 | ||
377 | /// fooBar - Does foo and bar. | |
378 | void Something::fooBar() { ... } | |
379 | ||
380 | Correct: | |
381 | ||
382 | .. code-block:: c++ | |
383 | ||
384 | // In Something.h: | |
385 | ||
386 | /// \brief An abstraction for some complicated thing. | |
387 | class Something { | |
388 | public: | |
389 | /// \brief Does foo and bar. | |
390 | void fooBar(); | |
391 | }; | |
392 | ||
393 | // In Something.cpp: | |
394 | ||
395 | // Builds a B-tree in order to do foo. See paper by... | |
396 | void Something::fooBar() { ... } | |
397 | ||
398 | It is not required to use additional Doxygen features, but sometimes it might | |
399 | be a good idea to do so. | |
400 | ||
401 | Consider: | |
402 | ||
403 | * adding comments to any narrow namespace containing a collection of | |
404 | related functions or types; | |
405 | ||
406 | * using top-level groups to organize a collection of related functions at | |
407 | namespace scope where the grouping is smaller than the namespace; | |
408 | ||
409 | * using member groups and additional comments attached to member | |
410 | groups to organize within a class. | |
411 | ||
412 | For example: | |
413 | ||
414 | .. code-block:: c++ | |
415 | ||
416 | class Something { | |
417 | /// \name Functions that do Foo. | |
418 | /// @{ | |
419 | void fooBar(); | |
420 | void fooBaz(); | |
421 | /// @} | |
422 | ... | |
423 | }; | |
424 | ||
223e47cc LB |
425 | ``#include`` Style |
426 | ^^^^^^^^^^^^^^^^^^ | |
427 | ||
428 | Immediately after the `header file comment`_ (and include guards if working on a | |
429 | header file), the `minimal list of #includes`_ required by the file should be | |
430 | listed. We prefer these ``#include``\s to be listed in this order: | |
431 | ||
432 | .. _Main Module Header: | |
433 | .. _Local/Private Headers: | |
434 | ||
435 | #. Main Module Header | |
436 | #. Local/Private Headers | |
970d7e83 | 437 | #. ``llvm/...`` |
223e47cc LB |
438 | #. System ``#include``\s |
439 | ||
970d7e83 | 440 | and each category should be sorted lexicographically by the full path. |
223e47cc LB |
441 | |
442 | The `Main Module Header`_ file applies to ``.cpp`` files which implement an | |
443 | interface defined by a ``.h`` file. This ``#include`` should always be included | |
444 | **first** regardless of where it lives on the file system. By including a | |
445 | header file first in the ``.cpp`` files that implement the interfaces, we ensure | |
446 | that the header does not have any hidden dependencies which are not explicitly | |
447 | ``#include``\d in the header, but should be. It is also a form of documentation | |
448 | in the ``.cpp`` file to indicate where the interfaces it implements are defined. | |
449 | ||
450 | .. _fit into 80 columns: | |
451 | ||
452 | Source Code Width | |
453 | ^^^^^^^^^^^^^^^^^ | |
454 | ||
455 | Write your code to fit within 80 columns of text. This helps those of us who | |
456 | like to print out code and look at your code in an ``xterm`` without resizing | |
457 | it. | |
458 | ||
459 | The longer answer is that there must be some limit to the width of the code in | |
460 | order to reasonably allow developers to have multiple files side-by-side in | |
461 | windows on a modest display. If you are going to pick a width limit, it is | |
462 | somewhat arbitrary but you might as well pick something standard. Going with 90 | |
463 | columns (for example) instead of 80 columns wouldn't add any significant value | |
464 | and would be detrimental to printing out code. Also many other projects have | |
465 | standardized on 80 columns, so some people have already configured their editors | |
466 | for it (vs something else, like 90 columns). | |
467 | ||
468 | This is one of many contentious issues in coding standards, but it is not up for | |
469 | debate. | |
470 | ||
471 | Use Spaces Instead of Tabs | |
472 | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
473 | ||
474 | In all cases, prefer spaces to tabs in source files. People have different | |
475 | preferred indentation levels, and different styles of indentation that they | |
476 | like; this is fine. What isn't fine is that different editors/viewers expand | |
477 | tabs out to different tab stops. This can cause your code to look completely | |
478 | unreadable, and it is not worth dealing with. | |
479 | ||
480 | As always, follow the `Golden Rule`_ above: follow the style of | |
481 | existing code if you are modifying and extending it. If you like four spaces of | |
482 | indentation, **DO NOT** do that in the middle of a chunk of code with two spaces | |
483 | of indentation. Also, do not reindent a whole source file: it makes for | |
484 | incredible diffs that are absolutely worthless. | |
485 | ||
486 | Indent Code Consistently | |
487 | ^^^^^^^^^^^^^^^^^^^^^^^^ | |
488 | ||
489 | Okay, in your first year of programming you were told that indentation is | |
1a4d82fc JJ |
490 | important. If you didn't believe and internalize this then, now is the time. |
491 | Just do it. With the introduction of C++11, there are some new formatting | |
492 | challenges that merit some suggestions to help have consistent, maintainable, | |
493 | and tool-friendly formatting and indentation. | |
494 | ||
495 | Format Lambdas Like Blocks Of Code | |
496 | """""""""""""""""""""""""""""""""" | |
497 | ||
498 | When formatting a multi-line lambda, format it like a block of code, that's | |
499 | what it is. If there is only one multi-line lambda in a statement, and there | |
500 | are no expressions lexically after it in the statement, drop the indent to the | |
501 | standard two space indent for a block of code, as if it were an if-block opened | |
502 | by the preceding part of the statement: | |
503 | ||
504 | .. code-block:: c++ | |
505 | ||
506 | std::sort(foo.begin(), foo.end(), [&](Foo a, Foo b) -> bool { | |
507 | if (a.blah < b.blah) | |
508 | return true; | |
509 | if (a.baz < b.baz) | |
510 | return true; | |
511 | return a.bam < b.bam; | |
512 | }); | |
513 | ||
514 | To take best advantage of this formatting, if you are designing an API which | |
515 | accepts a continuation or single callable argument (be it a functor, or | |
516 | a ``std::function``), it should be the last argument if at all possible. | |
517 | ||
518 | If there are multiple multi-line lambdas in a statement, or there is anything | |
519 | interesting after the lambda in the statement, indent the block two spaces from | |
520 | the indent of the ``[]``: | |
521 | ||
522 | .. code-block:: c++ | |
523 | ||
524 | dyn_switch(V->stripPointerCasts(), | |
525 | [] (PHINode *PN) { | |
526 | // process phis... | |
527 | }, | |
528 | [] (SelectInst *SI) { | |
529 | // process selects... | |
530 | }, | |
531 | [] (LoadInst *LI) { | |
532 | // process loads... | |
533 | }, | |
534 | [] (AllocaInst *AI) { | |
535 | // process allocas... | |
536 | }); | |
537 | ||
538 | Braced Initializer Lists | |
539 | """""""""""""""""""""""" | |
540 | ||
541 | With C++11, there are significantly more uses of braced lists to perform | |
542 | initialization. These allow you to easily construct aggregate temporaries in | |
543 | expressions among other niceness. They now have a natural way of ending up | |
544 | nested within each other and within function calls in order to build up | |
545 | aggregates (such as option structs) from local variables. To make matters | |
546 | worse, we also have many more uses of braces in an expression context that are | |
547 | *not* performing initialization. | |
548 | ||
549 | The historically common formatting of braced initialization of aggregate | |
550 | variables does not mix cleanly with deep nesting, general expression contexts, | |
551 | function arguments, and lambdas. We suggest new code use a simple rule for | |
552 | formatting braced initialization lists: act as-if the braces were parentheses | |
553 | in a function call. The formatting rules exactly match those already well | |
554 | understood for formatting nested function calls. Examples: | |
555 | ||
556 | .. code-block:: c++ | |
557 | ||
558 | foo({a, b, c}, {1, 2, 3}); | |
223e47cc | 559 | |
1a4d82fc JJ |
560 | llvm::Constant *Mask[] = { |
561 | llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 0), | |
562 | llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 1), | |
563 | llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 2)}; | |
564 | ||
565 | This formatting scheme also makes it particularly easy to get predictable, | |
566 | consistent, and automatic formatting with tools like `Clang Format`_. | |
567 | ||
568 | .. _Clang Format: http://clang.llvm.org/docs/ClangFormat.html | |
569 | ||
570 | Language and Compiler Issues | |
571 | ---------------------------- | |
223e47cc LB |
572 | |
573 | Treat Compiler Warnings Like Errors | |
574 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
575 | ||
576 | If your code has compiler warnings in it, something is wrong --- you aren't | |
577 | casting values correctly, you have "questionable" constructs in your code, or | |
578 | you are doing something legitimately wrong. Compiler warnings can cover up | |
579 | legitimate errors in output and make dealing with a translation unit difficult. | |
580 | ||
581 | It is not possible to prevent all warnings from all compilers, nor is it | |
582 | desirable. Instead, pick a standard compiler (like ``gcc``) that provides a | |
583 | good thorough set of warnings, and stick to it. At least in the case of | |
584 | ``gcc``, it is possible to work around any spurious errors by changing the | |
585 | syntax of the code slightly. For example, a warning that annoys me occurs when | |
586 | I write code like this: | |
587 | ||
588 | .. code-block:: c++ | |
589 | ||
590 | if (V = getValue()) { | |
591 | ... | |
592 | } | |
593 | ||
594 | ``gcc`` will warn me that I probably want to use the ``==`` operator, and that I | |
595 | probably mistyped it. In most cases, I haven't, and I really don't want the | |
596 | spurious errors. To fix this particular problem, I rewrite the code like | |
597 | this: | |
598 | ||
599 | .. code-block:: c++ | |
600 | ||
601 | if ((V = getValue())) { | |
602 | ... | |
603 | } | |
604 | ||
605 | which shuts ``gcc`` up. Any ``gcc`` warning that annoys you can be fixed by | |
606 | massaging the code appropriately. | |
607 | ||
608 | Write Portable Code | |
609 | ^^^^^^^^^^^^^^^^^^^ | |
610 | ||
611 | In almost all cases, it is possible and within reason to write completely | |
612 | portable code. If there are cases where it isn't possible to write portable | |
613 | code, isolate it behind a well defined (and well documented) interface. | |
614 | ||
615 | In practice, this means that you shouldn't assume much about the host compiler | |
616 | (and Visual Studio tends to be the lowest common denominator). If advanced | |
617 | features are used, they should only be an implementation detail of a library | |
618 | which has a simple exposed API, and preferably be buried in ``libSystem``. | |
619 | ||
620 | Do not use RTTI or Exceptions | |
621 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
622 | ||
623 | In an effort to reduce code and executable size, LLVM does not use RTTI | |
624 | (e.g. ``dynamic_cast<>;``) or exceptions. These two language features violate | |
625 | the general C++ principle of *"you only pay for what you use"*, causing | |
626 | executable bloat even if exceptions are never used in the code base, or if RTTI | |
627 | is never used for a class. Because of this, we turn them off globally in the | |
628 | code. | |
629 | ||
630 | That said, LLVM does make extensive use of a hand-rolled form of RTTI that use | |
1a4d82fc | 631 | templates like :ref:`isa\<>, cast\<>, and dyn_cast\<> <isa>`. |
970d7e83 LB |
632 | This form of RTTI is opt-in and can be |
633 | :doc:`added to any class <HowToSetUpLLVMStyleRTTI>`. It is also | |
223e47cc LB |
634 | substantially more efficient than ``dynamic_cast<>``. |
635 | ||
636 | .. _static constructor: | |
637 | ||
638 | Do not use Static Constructors | |
639 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
640 | ||
641 | Static constructors and destructors (e.g. global variables whose types have a | |
642 | constructor or destructor) should not be added to the code base, and should be | |
643 | removed wherever possible. Besides `well known problems | |
644 | <http://yosefk.com/c++fqa/ctors.html#fqa-10.12>`_ where the order of | |
645 | initialization is undefined between globals in different source files, the | |
646 | entire concept of static constructors is at odds with the common use case of | |
647 | LLVM as a library linked into a larger application. | |
648 | ||
649 | Consider the use of LLVM as a JIT linked into another application (perhaps for | |
650 | `OpenGL, custom languages <http://llvm.org/Users.html>`_, `shaders in movies | |
651 | <http://llvm.org/devmtg/2010-11/Gritz-OpenShadingLang.pdf>`_, etc). Due to the | |
652 | design of static constructors, they must be executed at startup time of the | |
653 | entire application, regardless of whether or how LLVM is used in that larger | |
654 | application. There are two problems with this: | |
655 | ||
656 | * The time to run the static constructors impacts startup time of applications | |
657 | --- a critical time for GUI apps, among others. | |
658 | ||
659 | * The static constructors cause the app to pull many extra pages of memory off | |
660 | the disk: both the code for the constructor in each ``.o`` file and the small | |
661 | amount of data that gets touched. In addition, touched/dirty pages put more | |
662 | pressure on the VM system on low-memory machines. | |
663 | ||
664 | We would really like for there to be zero cost for linking in an additional LLVM | |
665 | target or other library into an application, but static constructors violate | |
666 | this goal. | |
667 | ||
668 | That said, LLVM unfortunately does contain static constructors. It would be a | |
669 | `great project <http://llvm.org/PR11944>`_ for someone to purge all static | |
670 | constructors from LLVM, and then enable the ``-Wglobal-constructors`` warning | |
671 | flag (when building with Clang) to ensure we do not regress in the future. | |
672 | ||
673 | Use of ``class`` and ``struct`` Keywords | |
674 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
675 | ||
676 | In C++, the ``class`` and ``struct`` keywords can be used almost | |
677 | interchangeably. The only difference is when they are used to declare a class: | |
678 | ``class`` makes all members private by default while ``struct`` makes all | |
679 | members public by default. | |
680 | ||
681 | Unfortunately, not all compilers follow the rules and some will generate | |
682 | different symbols based on whether ``class`` or ``struct`` was used to declare | |
1a4d82fc JJ |
683 | the symbol (e.g., MSVC). This can lead to problems at link time. |
684 | ||
685 | * All declarations and definitions of a given ``class`` or ``struct`` must use | |
686 | the same keyword. For example: | |
687 | ||
688 | .. code-block:: c++ | |
689 | ||
690 | class Foo; | |
691 | ||
692 | // Breaks mangling in MSVC. | |
693 | struct Foo { int Data; }; | |
694 | ||
695 | * As a rule of thumb, ``struct`` should be kept to structures where *all* | |
696 | members are declared public. | |
697 | ||
698 | .. code-block:: c++ | |
699 | ||
700 | // Foo feels like a class... this is strange. | |
701 | struct Foo { | |
702 | private: | |
703 | int Data; | |
704 | public: | |
705 | Foo() : Data(0) { } | |
706 | int getData() const { return Data; } | |
707 | void setData(int D) { Data = D; } | |
708 | }; | |
709 | ||
710 | // Bar isn't POD, but it does look like a struct. | |
711 | struct Bar { | |
712 | int Data; | |
713 | Foo() : Data(0) { } | |
714 | }; | |
715 | ||
716 | Do not use Braced Initializer Lists to Call a Constructor | |
717 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
718 | ||
719 | In C++11 there is a "generalized initialization syntax" which allows calling | |
720 | constructors using braced initializer lists. Do not use these to call | |
721 | constructors with any interesting logic or if you care that you're calling some | |
722 | *particular* constructor. Those should look like function calls using | |
723 | parentheses rather than like aggregate initialization. Similarly, if you need | |
724 | to explicitly name the type and call its constructor to create a temporary, | |
725 | don't use a braced initializer list. Instead, use a braced initializer list | |
726 | (without any type for temporaries) when doing aggregate initialization or | |
727 | something notionally equivalent. Examples: | |
728 | ||
729 | .. code-block:: c++ | |
730 | ||
731 | class Foo { | |
732 | public: | |
733 | // Construct a Foo by reading data from the disk in the whizbang format, ... | |
734 | Foo(std::string filename); | |
735 | ||
736 | // Construct a Foo by looking up the Nth element of some global data ... | |
737 | Foo(int N); | |
738 | ||
739 | // ... | |
740 | }; | |
741 | ||
742 | // The Foo constructor call is very deliberate, no braces. | |
743 | std::fill(foo.begin(), foo.end(), Foo("name")); | |
744 | ||
745 | // The pair is just being constructed like an aggregate, use braces. | |
746 | bar_map.insert({my_key, my_value}); | |
747 | ||
748 | If you use a braced initializer list when initializing a variable, use an equals before the open curly brace: | |
749 | ||
750 | .. code-block:: c++ | |
751 | ||
752 | int data[] = {0, 1, 2, 3}; | |
753 | ||
754 | Use ``auto`` Type Deduction to Make Code More Readable | |
755 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
756 | ||
757 | Some are advocating a policy of "almost always ``auto``" in C++11, however LLVM | |
758 | uses a more moderate stance. Use ``auto`` if and only if it makes the code more | |
759 | readable or easier to maintain. Don't "almost always" use ``auto``, but do use | |
760 | ``auto`` with initializers like ``cast<Foo>(...)`` or other places where the | |
761 | type is already obvious from the context. Another time when ``auto`` works well | |
762 | for these purposes is when the type would have been abstracted away anyways, | |
763 | often behind a container's typedef such as ``std::vector<T>::iterator``. | |
764 | ||
765 | Beware unnecessary copies with ``auto`` | |
766 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
767 | ||
768 | The convenience of ``auto`` makes it easy to forget that its default behavior | |
769 | is a copy. Particularly in range-based ``for`` loops, careless copies are | |
770 | expensive. | |
771 | ||
772 | As a rule of thumb, use ``auto &`` unless you need to copy the result, and use | |
773 | ``auto *`` when copying pointers. | |
774 | ||
775 | .. code-block:: c++ | |
223e47cc | 776 | |
1a4d82fc JJ |
777 | // Typically there's no reason to copy. |
778 | for (const auto &Val : Container) { observe(Val); } | |
779 | for (auto &Val : Container) { Val.change(); } | |
780 | ||
781 | // Remove the reference if you really want a new copy. | |
782 | for (auto Val : Container) { Val.change(); saveSomewhere(Val); } | |
783 | ||
784 | // Copy pointers, but make it clear that they're pointers. | |
785 | for (const auto *Ptr : Container) { observe(*Ptr); } | |
786 | for (auto *Ptr : Container) { Ptr->change(); } | |
223e47cc LB |
787 | |
788 | Style Issues | |
789 | ============ | |
790 | ||
791 | The High-Level Issues | |
792 | --------------------- | |
793 | ||
794 | A Public Header File **is** a Module | |
795 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
796 | ||
797 | C++ doesn't do too well in the modularity department. There is no real | |
798 | encapsulation or data hiding (unless you use expensive protocol classes), but it | |
799 | is what we have to work with. When you write a public header file (in the LLVM | |
800 | source tree, they live in the top level "``include``" directory), you are | |
801 | defining a module of functionality. | |
802 | ||
803 | Ideally, modules should be completely independent of each other, and their | |
804 | header files should only ``#include`` the absolute minimum number of headers | |
805 | possible. A module is not just a class, a function, or a namespace: it's a | |
806 | collection of these that defines an interface. This interface may be several | |
807 | functions, classes, or data structures, but the important issue is how they work | |
808 | together. | |
809 | ||
810 | In general, a module should be implemented by one or more ``.cpp`` files. Each | |
811 | of these ``.cpp`` files should include the header that defines their interface | |
812 | first. This ensures that all of the dependences of the module header have been | |
813 | properly added to the module header itself, and are not implicit. System | |
814 | headers should be included after user headers for a translation unit. | |
815 | ||
816 | .. _minimal list of #includes: | |
817 | ||
818 | ``#include`` as Little as Possible | |
819 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
820 | ||
821 | ``#include`` hurts compile time performance. Don't do it unless you have to, | |
822 | especially in header files. | |
823 | ||
824 | But wait! Sometimes you need to have the definition of a class to use it, or to | |
825 | inherit from it. In these cases go ahead and ``#include`` that header file. Be | |
826 | aware however that there are many cases where you don't need to have the full | |
827 | definition of a class. If you are using a pointer or reference to a class, you | |
828 | don't need the header file. If you are simply returning a class instance from a | |
829 | prototyped function or method, you don't need it. In fact, for most cases, you | |
830 | simply don't need the definition of a class. And not ``#include``\ing speeds up | |
831 | compilation. | |
832 | ||
833 | It is easy to try to go too overboard on this recommendation, however. You | |
834 | **must** include all of the header files that you are using --- you can include | |
835 | them either directly or indirectly through another header file. To make sure | |
836 | that you don't accidentally forget to include a header file in your module | |
837 | header, make sure to include your module header **first** in the implementation | |
838 | file (as mentioned above). This way there won't be any hidden dependencies that | |
839 | you'll find out about later. | |
840 | ||
841 | Keep "Internal" Headers Private | |
842 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
843 | ||
844 | Many modules have a complex implementation that causes them to use more than one | |
845 | implementation (``.cpp``) file. It is often tempting to put the internal | |
846 | communication interface (helper classes, extra functions, etc) in the public | |
847 | module header file. Don't do this! | |
848 | ||
849 | If you really need to do something like this, put a private header file in the | |
850 | same directory as the source files, and include it locally. This ensures that | |
851 | your private interface remains private and undisturbed by outsiders. | |
852 | ||
853 | .. note:: | |
854 | ||
855 | It's okay to put extra implementation methods in a public class itself. Just | |
856 | make them private (or protected) and all is well. | |
857 | ||
858 | .. _early exits: | |
859 | ||
860 | Use Early Exits and ``continue`` to Simplify Code | |
861 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
862 | ||
863 | When reading code, keep in mind how much state and how many previous decisions | |
864 | have to be remembered by the reader to understand a block of code. Aim to | |
865 | reduce indentation where possible when it doesn't make it more difficult to | |
866 | understand the code. One great way to do this is by making use of early exits | |
867 | and the ``continue`` keyword in long loops. As an example of using an early | |
868 | exit from a function, consider this "bad" code: | |
869 | ||
870 | .. code-block:: c++ | |
871 | ||
872 | Value *doSomething(Instruction *I) { | |
873 | if (!isa<TerminatorInst>(I) && | |
874 | I->hasOneUse() && doOtherThing(I)) { | |
875 | ... some long code .... | |
876 | } | |
877 | ||
878 | return 0; | |
879 | } | |
880 | ||
881 | This code has several problems if the body of the ``'if'`` is large. When | |
882 | you're looking at the top of the function, it isn't immediately clear that this | |
883 | *only* does interesting things with non-terminator instructions, and only | |
884 | applies to things with the other predicates. Second, it is relatively difficult | |
885 | to describe (in comments) why these predicates are important because the ``if`` | |
886 | statement makes it difficult to lay out the comments. Third, when you're deep | |
887 | within the body of the code, it is indented an extra level. Finally, when | |
888 | reading the top of the function, it isn't clear what the result is if the | |
889 | predicate isn't true; you have to read to the end of the function to know that | |
890 | it returns null. | |
891 | ||
892 | It is much preferred to format the code like this: | |
893 | ||
894 | .. code-block:: c++ | |
895 | ||
896 | Value *doSomething(Instruction *I) { | |
897 | // Terminators never need 'something' done to them because ... | |
898 | if (isa<TerminatorInst>(I)) | |
899 | return 0; | |
900 | ||
901 | // We conservatively avoid transforming instructions with multiple uses | |
902 | // because goats like cheese. | |
903 | if (!I->hasOneUse()) | |
904 | return 0; | |
905 | ||
906 | // This is really just here for example. | |
907 | if (!doOtherThing(I)) | |
908 | return 0; | |
909 | ||
910 | ... some long code .... | |
911 | } | |
912 | ||
913 | This fixes these problems. A similar problem frequently happens in ``for`` | |
914 | loops. A silly example is something like this: | |
915 | ||
916 | .. code-block:: c++ | |
917 | ||
918 | for (BasicBlock::iterator II = BB->begin(), E = BB->end(); II != E; ++II) { | |
919 | if (BinaryOperator *BO = dyn_cast<BinaryOperator>(II)) { | |
920 | Value *LHS = BO->getOperand(0); | |
921 | Value *RHS = BO->getOperand(1); | |
922 | if (LHS != RHS) { | |
923 | ... | |
924 | } | |
925 | } | |
926 | } | |
927 | ||
928 | When you have very, very small loops, this sort of structure is fine. But if it | |
929 | exceeds more than 10-15 lines, it becomes difficult for people to read and | |
930 | understand at a glance. The problem with this sort of code is that it gets very | |
931 | nested very quickly. Meaning that the reader of the code has to keep a lot of | |
932 | context in their brain to remember what is going immediately on in the loop, | |
933 | because they don't know if/when the ``if`` conditions will have ``else``\s etc. | |
934 | It is strongly preferred to structure the loop like this: | |
935 | ||
936 | .. code-block:: c++ | |
937 | ||
938 | for (BasicBlock::iterator II = BB->begin(), E = BB->end(); II != E; ++II) { | |
939 | BinaryOperator *BO = dyn_cast<BinaryOperator>(II); | |
940 | if (!BO) continue; | |
941 | ||
942 | Value *LHS = BO->getOperand(0); | |
943 | Value *RHS = BO->getOperand(1); | |
944 | if (LHS == RHS) continue; | |
945 | ||
946 | ... | |
947 | } | |
948 | ||
949 | This has all the benefits of using early exits for functions: it reduces nesting | |
950 | of the loop, it makes it easier to describe why the conditions are true, and it | |
951 | makes it obvious to the reader that there is no ``else`` coming up that they | |
952 | have to push context into their brain for. If a loop is large, this can be a | |
953 | big understandability win. | |
954 | ||
955 | Don't use ``else`` after a ``return`` | |
956 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
957 | ||
958 | For similar reasons above (reduction of indentation and easier reading), please | |
959 | do not use ``'else'`` or ``'else if'`` after something that interrupts control | |
960 | flow --- like ``return``, ``break``, ``continue``, ``goto``, etc. For | |
961 | example, this is *bad*: | |
962 | ||
963 | .. code-block:: c++ | |
964 | ||
965 | case 'J': { | |
966 | if (Signed) { | |
967 | Type = Context.getsigjmp_bufType(); | |
968 | if (Type.isNull()) { | |
969 | Error = ASTContext::GE_Missing_sigjmp_buf; | |
970 | return QualType(); | |
971 | } else { | |
972 | break; | |
973 | } | |
974 | } else { | |
975 | Type = Context.getjmp_bufType(); | |
976 | if (Type.isNull()) { | |
977 | Error = ASTContext::GE_Missing_jmp_buf; | |
978 | return QualType(); | |
979 | } else { | |
980 | break; | |
981 | } | |
982 | } | |
983 | } | |
984 | ||
985 | It is better to write it like this: | |
986 | ||
987 | .. code-block:: c++ | |
988 | ||
989 | case 'J': | |
990 | if (Signed) { | |
991 | Type = Context.getsigjmp_bufType(); | |
992 | if (Type.isNull()) { | |
993 | Error = ASTContext::GE_Missing_sigjmp_buf; | |
994 | return QualType(); | |
995 | } | |
996 | } else { | |
997 | Type = Context.getjmp_bufType(); | |
998 | if (Type.isNull()) { | |
999 | Error = ASTContext::GE_Missing_jmp_buf; | |
1000 | return QualType(); | |
1001 | } | |
1002 | } | |
1003 | break; | |
1004 | ||
1005 | Or better yet (in this case) as: | |
1006 | ||
1007 | .. code-block:: c++ | |
1008 | ||
1009 | case 'J': | |
1010 | if (Signed) | |
1011 | Type = Context.getsigjmp_bufType(); | |
1012 | else | |
1013 | Type = Context.getjmp_bufType(); | |
1014 | ||
1015 | if (Type.isNull()) { | |
1016 | Error = Signed ? ASTContext::GE_Missing_sigjmp_buf : | |
1017 | ASTContext::GE_Missing_jmp_buf; | |
1018 | return QualType(); | |
1019 | } | |
1020 | break; | |
1021 | ||
1022 | The idea is to reduce indentation and the amount of code you have to keep track | |
1023 | of when reading the code. | |
1024 | ||
1025 | Turn Predicate Loops into Predicate Functions | |
1026 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
1027 | ||
1028 | It is very common to write small loops that just compute a boolean value. There | |
1029 | are a number of ways that people commonly write these, but an example of this | |
1030 | sort of thing is: | |
1031 | ||
1032 | .. code-block:: c++ | |
1033 | ||
1034 | bool FoundFoo = false; | |
970d7e83 LB |
1035 | for (unsigned I = 0, E = BarList.size(); I != E; ++I) |
1036 | if (BarList[I]->isFoo()) { | |
223e47cc LB |
1037 | FoundFoo = true; |
1038 | break; | |
1039 | } | |
1040 | ||
1041 | if (FoundFoo) { | |
1042 | ... | |
1043 | } | |
1044 | ||
1045 | This sort of code is awkward to write, and is almost always a bad sign. Instead | |
1046 | of this sort of loop, we strongly prefer to use a predicate function (which may | |
1047 | be `static`_) that uses `early exits`_ to compute the predicate. We prefer the | |
1048 | code to be structured like this: | |
1049 | ||
1050 | .. code-block:: c++ | |
1051 | ||
970d7e83 | 1052 | /// \returns true if the specified list has an element that is a foo. |
223e47cc | 1053 | static bool containsFoo(const std::vector<Bar*> &List) { |
970d7e83 LB |
1054 | for (unsigned I = 0, E = List.size(); I != E; ++I) |
1055 | if (List[I]->isFoo()) | |
223e47cc LB |
1056 | return true; |
1057 | return false; | |
1058 | } | |
1059 | ... | |
1060 | ||
1061 | if (containsFoo(BarList)) { | |
1062 | ... | |
1063 | } | |
1064 | ||
1065 | There are many reasons for doing this: it reduces indentation and factors out | |
1066 | code which can often be shared by other code that checks for the same predicate. | |
1067 | More importantly, it *forces you to pick a name* for the function, and forces | |
1068 | you to write a comment for it. In this silly example, this doesn't add much | |
1069 | value. However, if the condition is complex, this can make it a lot easier for | |
1070 | the reader to understand the code that queries for this predicate. Instead of | |
1071 | being faced with the in-line details of how we check to see if the BarList | |
1072 | contains a foo, we can trust the function name and continue reading with better | |
1073 | locality. | |
1074 | ||
1075 | The Low-Level Issues | |
1076 | -------------------- | |
1077 | ||
1078 | Name Types, Functions, Variables, and Enumerators Properly | |
1079 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
1080 | ||
1081 | Poorly-chosen names can mislead the reader and cause bugs. We cannot stress | |
1082 | enough how important it is to use *descriptive* names. Pick names that match | |
1083 | the semantics and role of the underlying entities, within reason. Avoid | |
1084 | abbreviations unless they are well known. After picking a good name, make sure | |
1085 | to use consistent capitalization for the name, as inconsistency requires clients | |
1086 | to either memorize the APIs or to look it up to find the exact spelling. | |
1087 | ||
1088 | In general, names should be in camel case (e.g. ``TextFileReader`` and | |
1089 | ``isLValue()``). Different kinds of declarations have different rules: | |
1090 | ||
1091 | * **Type names** (including classes, structs, enums, typedefs, etc) should be | |
1092 | nouns and start with an upper-case letter (e.g. ``TextFileReader``). | |
1093 | ||
1094 | * **Variable names** should be nouns (as they represent state). The name should | |
1095 | be camel case, and start with an upper case letter (e.g. ``Leader`` or | |
1096 | ``Boats``). | |
1097 | ||
1098 | * **Function names** should be verb phrases (as they represent actions), and | |
1099 | command-like function should be imperative. The name should be camel case, | |
1100 | and start with a lower case letter (e.g. ``openFile()`` or ``isFoo()``). | |
1101 | ||
1102 | * **Enum declarations** (e.g. ``enum Foo {...}``) are types, so they should | |
1103 | follow the naming conventions for types. A common use for enums is as a | |
1104 | discriminator for a union, or an indicator of a subclass. When an enum is | |
1105 | used for something like this, it should have a ``Kind`` suffix | |
1106 | (e.g. ``ValueKind``). | |
1107 | ||
1108 | * **Enumerators** (e.g. ``enum { Foo, Bar }``) and **public member variables** | |
1109 | should start with an upper-case letter, just like types. Unless the | |
1110 | enumerators are defined in their own small namespace or inside a class, | |
1111 | enumerators should have a prefix corresponding to the enum declaration name. | |
1112 | For example, ``enum ValueKind { ... };`` may contain enumerators like | |
1113 | ``VK_Argument``, ``VK_BasicBlock``, etc. Enumerators that are just | |
1114 | convenience constants are exempt from the requirement for a prefix. For | |
1115 | instance: | |
1116 | ||
1117 | .. code-block:: c++ | |
1118 | ||
1119 | enum { | |
1120 | MaxSize = 42, | |
1121 | Density = 12 | |
1122 | }; | |
1123 | ||
1124 | As an exception, classes that mimic STL classes can have member names in STL's | |
1125 | style of lower-case words separated by underscores (e.g. ``begin()``, | |
1a4d82fc JJ |
1126 | ``push_back()``, and ``empty()``). Classes that provide multiple |
1127 | iterators should add a singular prefix to ``begin()`` and ``end()`` | |
1128 | (e.g. ``global_begin()`` and ``use_begin()``). | |
223e47cc LB |
1129 | |
1130 | Here are some examples of good and bad names: | |
1131 | ||
1132 | .. code-block:: c++ | |
1133 | ||
1134 | class VehicleMaker { | |
1135 | ... | |
1136 | Factory<Tire> F; // Bad -- abbreviation and non-descriptive. | |
1137 | Factory<Tire> Factory; // Better. | |
1138 | Factory<Tire> TireFactory; // Even better -- if VehicleMaker has more than one | |
1139 | // kind of factories. | |
1140 | }; | |
1141 | ||
1142 | Vehicle MakeVehicle(VehicleType Type) { | |
1143 | VehicleMaker M; // Might be OK if having a short life-span. | |
970d7e83 LB |
1144 | Tire Tmp1 = M.makeTire(); // Bad -- 'Tmp1' provides no information. |
1145 | Light Headlight = M.makeLight("head"); // Good -- descriptive. | |
223e47cc LB |
1146 | ... |
1147 | } | |
1148 | ||
1149 | Assert Liberally | |
1150 | ^^^^^^^^^^^^^^^^ | |
1151 | ||
1152 | Use the "``assert``" macro to its fullest. Check all of your preconditions and | |
1153 | assumptions, you never know when a bug (not necessarily even yours) might be | |
1154 | caught early by an assertion, which reduces debugging time dramatically. The | |
1155 | "``<cassert>``" header file is probably already included by the header files you | |
1156 | are using, so it doesn't cost anything to use it. | |
1157 | ||
1158 | To further assist with debugging, make sure to put some kind of error message in | |
1159 | the assertion statement, which is printed if the assertion is tripped. This | |
1160 | helps the poor debugger make sense of why an assertion is being made and | |
1161 | enforced, and hopefully what to do about it. Here is one complete example: | |
1162 | ||
1163 | .. code-block:: c++ | |
1164 | ||
970d7e83 LB |
1165 | inline Value *getOperand(unsigned I) { |
1166 | assert(I < Operands.size() && "getOperand() out of range!"); | |
1167 | return Operands[I]; | |
223e47cc LB |
1168 | } |
1169 | ||
1170 | Here are more examples: | |
1171 | ||
1172 | .. code-block:: c++ | |
1173 | ||
1a4d82fc | 1174 | assert(Ty->isPointerType() && "Can't allocate a non-pointer type!"); |
223e47cc LB |
1175 | |
1176 | assert((Opcode == Shl || Opcode == Shr) && "ShiftInst Opcode invalid!"); | |
1177 | ||
1178 | assert(idx < getNumSuccessors() && "Successor # out of range!"); | |
1179 | ||
1180 | assert(V1.getType() == V2.getType() && "Constant types must be identical!"); | |
1181 | ||
1182 | assert(isa<PHINode>(Succ->front()) && "Only works on PHId BBs!"); | |
1183 | ||
1184 | You get the idea. | |
1185 | ||
970d7e83 LB |
1186 | In the past, asserts were used to indicate a piece of code that should not be |
1187 | reached. These were typically of the form: | |
223e47cc LB |
1188 | |
1189 | .. code-block:: c++ | |
1190 | ||
970d7e83 | 1191 | assert(0 && "Invalid radix for integer literal"); |
223e47cc | 1192 | |
970d7e83 LB |
1193 | This has a few issues, the main one being that some compilers might not |
1194 | understand the assertion, or warn about a missing return in builds where | |
1195 | assertions are compiled out. | |
1196 | ||
1197 | Today, we have something much better: ``llvm_unreachable``: | |
223e47cc LB |
1198 | |
1199 | .. code-block:: c++ | |
1200 | ||
970d7e83 LB |
1201 | llvm_unreachable("Invalid radix for integer literal"); |
1202 | ||
1203 | When assertions are enabled, this will print the message if it's ever reached | |
1204 | and then exit the program. When assertions are disabled (i.e. in release | |
1205 | builds), ``llvm_unreachable`` becomes a hint to compilers to skip generating | |
1206 | code for this branch. If the compiler does not support this, it will fall back | |
1207 | to the "abort" implementation. | |
223e47cc LB |
1208 | |
1209 | Another issue is that values used only by assertions will produce an "unused | |
1210 | value" warning when assertions are disabled. For example, this code will warn: | |
1211 | ||
1212 | .. code-block:: c++ | |
1213 | ||
1214 | unsigned Size = V.size(); | |
1215 | assert(Size > 42 && "Vector smaller than it should be"); | |
1216 | ||
1217 | bool NewToSet = Myset.insert(Value); | |
1218 | assert(NewToSet && "The value shouldn't be in the set yet"); | |
1219 | ||
1220 | These are two interesting different cases. In the first case, the call to | |
1221 | ``V.size()`` is only useful for the assert, and we don't want it executed when | |
1222 | assertions are disabled. Code like this should move the call into the assert | |
1223 | itself. In the second case, the side effects of the call must happen whether | |
1224 | the assert is enabled or not. In this case, the value should be cast to void to | |
1225 | disable the warning. To be specific, it is preferred to write the code like | |
1226 | this: | |
1227 | ||
1228 | .. code-block:: c++ | |
1229 | ||
1230 | assert(V.size() > 42 && "Vector smaller than it should be"); | |
1231 | ||
1232 | bool NewToSet = Myset.insert(Value); (void)NewToSet; | |
1233 | assert(NewToSet && "The value shouldn't be in the set yet"); | |
1234 | ||
1235 | Do Not Use ``using namespace std`` | |
1236 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
1237 | ||
1238 | In LLVM, we prefer to explicitly prefix all identifiers from the standard | |
1239 | namespace with an "``std::``" prefix, rather than rely on "``using namespace | |
1240 | std;``". | |
1241 | ||
1242 | In header files, adding a ``'using namespace XXX'`` directive pollutes the | |
1243 | namespace of any source file that ``#include``\s the header. This is clearly a | |
1244 | bad thing. | |
1245 | ||
1246 | In implementation files (e.g. ``.cpp`` files), the rule is more of a stylistic | |
1247 | rule, but is still important. Basically, using explicit namespace prefixes | |
1248 | makes the code **clearer**, because it is immediately obvious what facilities | |
1249 | are being used and where they are coming from. And **more portable**, because | |
1250 | namespace clashes cannot occur between LLVM code and other namespaces. The | |
1251 | portability rule is important because different standard library implementations | |
1252 | expose different symbols (potentially ones they shouldn't), and future revisions | |
1253 | to the C++ standard will add more symbols to the ``std`` namespace. As such, we | |
1254 | never use ``'using namespace std;'`` in LLVM. | |
1255 | ||
1256 | The exception to the general rule (i.e. it's not an exception for the ``std`` | |
1257 | namespace) is for implementation files. For example, all of the code in the | |
1258 | LLVM project implements code that lives in the 'llvm' namespace. As such, it is | |
1259 | ok, and actually clearer, for the ``.cpp`` files to have a ``'using namespace | |
1260 | llvm;'`` directive at the top, after the ``#include``\s. This reduces | |
1261 | indentation in the body of the file for source editors that indent based on | |
1262 | braces, and keeps the conceptual context cleaner. The general form of this rule | |
1263 | is that any ``.cpp`` file that implements code in any namespace may use that | |
1264 | namespace (and its parents'), but should not use any others. | |
1265 | ||
1266 | Provide a Virtual Method Anchor for Classes in Headers | |
1267 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
1268 | ||
1269 | If a class is defined in a header file and has a vtable (either it has virtual | |
1270 | methods or it derives from classes with virtual methods), it must always have at | |
1271 | least one out-of-line virtual method in the class. Without this, the compiler | |
1272 | will copy the vtable and RTTI into every ``.o`` file that ``#include``\s the | |
1273 | header, bloating ``.o`` file sizes and increasing link times. | |
1274 | ||
1275 | Don't use default labels in fully covered switches over enumerations | |
1276 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
1277 | ||
1278 | ``-Wswitch`` warns if a switch, without a default label, over an enumeration | |
1279 | does not cover every enumeration value. If you write a default label on a fully | |
1280 | covered switch over an enumeration then the ``-Wswitch`` warning won't fire | |
1281 | when new elements are added to that enumeration. To help avoid adding these | |
1282 | kinds of defaults, Clang has the warning ``-Wcovered-switch-default`` which is | |
1283 | off by default but turned on when building LLVM with a version of Clang that | |
1284 | supports the warning. | |
1285 | ||
1286 | A knock-on effect of this stylistic requirement is that when building LLVM with | |
1287 | GCC you may get warnings related to "control may reach end of non-void function" | |
1288 | if you return from each case of a covered switch-over-enum because GCC assumes | |
1289 | that the enum expression may take any representable value, not just those of | |
1290 | individual enumerators. To suppress this warning, use ``llvm_unreachable`` after | |
1291 | the switch. | |
1292 | ||
1293 | Use ``LLVM_DELETED_FUNCTION`` to mark uncallable methods | |
1294 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
1295 | ||
1296 | Prior to C++11, a common pattern to make a class uncopyable was to declare an | |
1297 | unimplemented copy constructor and copy assignment operator and make them | |
1298 | private. This would give a compiler error for accessing a private method or a | |
1299 | linker error because it wasn't implemented. | |
1300 | ||
1301 | With C++11, we can mark methods that won't be implemented with ``= delete``. | |
1302 | This will trigger a much better error message and tell the compiler that the | |
1303 | method will never be implemented. This enables other checks like | |
1304 | ``-Wunused-private-field`` to run correctly on classes that contain these | |
1305 | methods. | |
1306 | ||
1a4d82fc JJ |
1307 | For compatibility with MSVC, ``LLVM_DELETED_FUNCTION`` should be used which |
1308 | will expand to ``= delete`` on compilers that support it. These methods should | |
1309 | still be declared private. Example of the uncopyable pattern: | |
223e47cc LB |
1310 | |
1311 | .. code-block:: c++ | |
1312 | ||
1313 | class DontCopy { | |
1314 | private: | |
1315 | DontCopy(const DontCopy&) LLVM_DELETED_FUNCTION; | |
1316 | DontCopy &operator =(const DontCopy&) LLVM_DELETED_FUNCTION; | |
1317 | public: | |
1318 | ... | |
1319 | }; | |
1320 | ||
1321 | Don't evaluate ``end()`` every time through a loop | |
1322 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
1323 | ||
1324 | Because C++ doesn't have a standard "``foreach``" loop (though it can be | |
1325 | emulated with macros and may be coming in C++'0x) we end up writing a lot of | |
1326 | loops that manually iterate from begin to end on a variety of containers or | |
1327 | through other data structures. One common mistake is to write a loop in this | |
1328 | style: | |
1329 | ||
1330 | .. code-block:: c++ | |
1331 | ||
1332 | BasicBlock *BB = ... | |
1333 | for (BasicBlock::iterator I = BB->begin(); I != BB->end(); ++I) | |
1334 | ... use I ... | |
1335 | ||
1336 | The problem with this construct is that it evaluates "``BB->end()``" every time | |
1337 | through the loop. Instead of writing the loop like this, we strongly prefer | |
1338 | loops to be written so that they evaluate it once before the loop starts. A | |
1339 | convenient way to do this is like so: | |
1340 | ||
1341 | .. code-block:: c++ | |
1342 | ||
1343 | BasicBlock *BB = ... | |
1344 | for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E; ++I) | |
1345 | ... use I ... | |
1346 | ||
1347 | The observant may quickly point out that these two loops may have different | |
1348 | semantics: if the container (a basic block in this case) is being mutated, then | |
1349 | "``BB->end()``" may change its value every time through the loop and the second | |
1350 | loop may not in fact be correct. If you actually do depend on this behavior, | |
1351 | please write the loop in the first form and add a comment indicating that you | |
1352 | did it intentionally. | |
1353 | ||
1354 | Why do we prefer the second form (when correct)? Writing the loop in the first | |
1355 | form has two problems. First it may be less efficient than evaluating it at the | |
1356 | start of the loop. In this case, the cost is probably minor --- a few extra | |
1357 | loads every time through the loop. However, if the base expression is more | |
1358 | complex, then the cost can rise quickly. I've seen loops where the end | |
970d7e83 | 1359 | expression was actually something like: "``SomeMap[X]->end()``" and map lookups |
223e47cc LB |
1360 | really aren't cheap. By writing it in the second form consistently, you |
1361 | eliminate the issue entirely and don't even have to think about it. | |
1362 | ||
1363 | The second (even bigger) issue is that writing the loop in the first form hints | |
1364 | to the reader that the loop is mutating the container (a fact that a comment | |
1365 | would handily confirm!). If you write the loop in the second form, it is | |
1366 | immediately obvious without even looking at the body of the loop that the | |
1367 | container isn't being modified, which makes it easier to read the code and | |
1368 | understand what it does. | |
1369 | ||
1370 | While the second form of the loop is a few extra keystrokes, we do strongly | |
1371 | prefer it. | |
1372 | ||
1373 | ``#include <iostream>`` is Forbidden | |
1374 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
1375 | ||
1376 | The use of ``#include <iostream>`` in library files is hereby **forbidden**, | |
1377 | because many common implementations transparently inject a `static constructor`_ | |
1378 | into every translation unit that includes it. | |
1379 | ||
1380 | Note that using the other stream headers (``<sstream>`` for example) is not | |
1381 | problematic in this regard --- just ``<iostream>``. However, ``raw_ostream`` | |
1382 | provides various APIs that are better performing for almost every use than | |
1383 | ``std::ostream`` style APIs. | |
1384 | ||
1385 | .. note:: | |
1386 | ||
1387 | New code should always use `raw_ostream`_ for writing, or the | |
1388 | ``llvm::MemoryBuffer`` API for reading files. | |
1389 | ||
1390 | .. _raw_ostream: | |
1391 | ||
1392 | Use ``raw_ostream`` | |
1393 | ^^^^^^^^^^^^^^^^^^^ | |
1394 | ||
1395 | LLVM includes a lightweight, simple, and efficient stream implementation in | |
1396 | ``llvm/Support/raw_ostream.h``, which provides all of the common features of | |
1397 | ``std::ostream``. All new code should use ``raw_ostream`` instead of | |
1398 | ``ostream``. | |
1399 | ||
1400 | Unlike ``std::ostream``, ``raw_ostream`` is not a template and can be forward | |
1401 | declared as ``class raw_ostream``. Public headers should generally not include | |
1402 | the ``raw_ostream`` header, but use forward declarations and constant references | |
1403 | to ``raw_ostream`` instances. | |
1404 | ||
1405 | Avoid ``std::endl`` | |
1406 | ^^^^^^^^^^^^^^^^^^^ | |
1407 | ||
1408 | The ``std::endl`` modifier, when used with ``iostreams`` outputs a newline to | |
1409 | the output stream specified. In addition to doing this, however, it also | |
1410 | flushes the output stream. In other words, these are equivalent: | |
1411 | ||
1412 | .. code-block:: c++ | |
1413 | ||
1414 | std::cout << std::endl; | |
1415 | std::cout << '\n' << std::flush; | |
1416 | ||
1417 | Most of the time, you probably have no reason to flush the output stream, so | |
1418 | it's better to use a literal ``'\n'``. | |
1419 | ||
970d7e83 LB |
1420 | Don't use ``inline`` when defining a function in a class definition |
1421 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
1422 | ||
1423 | A member function defined in a class definition is implicitly inline, so don't | |
1424 | put the ``inline`` keyword in this case. | |
1425 | ||
1426 | Don't: | |
1427 | ||
1428 | .. code-block:: c++ | |
1429 | ||
1430 | class Foo { | |
1431 | public: | |
1432 | inline void bar() { | |
1433 | // ... | |
1434 | } | |
1435 | }; | |
1436 | ||
1437 | Do: | |
1438 | ||
1439 | .. code-block:: c++ | |
1440 | ||
1441 | class Foo { | |
1442 | public: | |
1443 | void bar() { | |
1444 | // ... | |
1445 | } | |
1446 | }; | |
1447 | ||
223e47cc LB |
1448 | Microscopic Details |
1449 | ------------------- | |
1450 | ||
1451 | This section describes preferred low-level formatting guidelines along with | |
1452 | reasoning on why we prefer them. | |
1453 | ||
1454 | Spaces Before Parentheses | |
1455 | ^^^^^^^^^^^^^^^^^^^^^^^^^ | |
1456 | ||
1457 | We prefer to put a space before an open parenthesis only in control flow | |
1458 | statements, but not in normal function call expressions and function-like | |
1459 | macros. For example, this is good: | |
1460 | ||
1461 | .. code-block:: c++ | |
1462 | ||
970d7e83 LB |
1463 | if (X) ... |
1464 | for (I = 0; I != 100; ++I) ... | |
1465 | while (LLVMRocks) ... | |
223e47cc LB |
1466 | |
1467 | somefunc(42); | |
1468 | assert(3 != 4 && "laws of math are failing me"); | |
1469 | ||
970d7e83 | 1470 | A = foo(42, 92) + bar(X); |
223e47cc LB |
1471 | |
1472 | and this is bad: | |
1473 | ||
1474 | .. code-block:: c++ | |
1475 | ||
970d7e83 LB |
1476 | if(X) ... |
1477 | for(I = 0; I != 100; ++I) ... | |
1478 | while(LLVMRocks) ... | |
223e47cc LB |
1479 | |
1480 | somefunc (42); | |
1481 | assert (3 != 4 && "laws of math are failing me"); | |
1482 | ||
970d7e83 | 1483 | A = foo (42, 92) + bar (X); |
223e47cc LB |
1484 | |
1485 | The reason for doing this is not completely arbitrary. This style makes control | |
1486 | flow operators stand out more, and makes expressions flow better. The function | |
1487 | call operator binds very tightly as a postfix operator. Putting a space after a | |
1488 | function name (as in the last example) makes it appear that the code might bind | |
1489 | the arguments of the left-hand-side of a binary operator with the argument list | |
1490 | of a function and the name of the right side. More specifically, it is easy to | |
970d7e83 | 1491 | misread the "``A``" example as: |
223e47cc LB |
1492 | |
1493 | .. code-block:: c++ | |
1494 | ||
970d7e83 | 1495 | A = foo ((42, 92) + bar) (X); |
223e47cc LB |
1496 | |
1497 | when skimming through the code. By avoiding a space in a function, we avoid | |
1498 | this misinterpretation. | |
1499 | ||
1500 | Prefer Preincrement | |
1501 | ^^^^^^^^^^^^^^^^^^^ | |
1502 | ||
1503 | Hard fast rule: Preincrement (``++X``) may be no slower than postincrement | |
1504 | (``X++``) and could very well be a lot faster than it. Use preincrementation | |
1505 | whenever possible. | |
1506 | ||
1507 | The semantics of postincrement include making a copy of the value being | |
1508 | incremented, returning it, and then preincrementing the "work value". For | |
1509 | primitive types, this isn't a big deal. But for iterators, it can be a huge | |
1510 | issue (for example, some iterators contains stack and set objects in them... | |
1511 | copying an iterator could invoke the copy ctor's of these as well). In general, | |
1512 | get in the habit of always using preincrement, and you won't have a problem. | |
1513 | ||
1514 | ||
1515 | Namespace Indentation | |
1516 | ^^^^^^^^^^^^^^^^^^^^^ | |
1517 | ||
1518 | In general, we strive to reduce indentation wherever possible. This is useful | |
1519 | because we want code to `fit into 80 columns`_ without wrapping horribly, but | |
1a4d82fc JJ |
1520 | also because it makes it easier to understand the code. To facilitate this and |
1521 | avoid some insanely deep nesting on occasion, don't indent namespaces. If it | |
1522 | helps readability, feel free to add a comment indicating what namespace is | |
1523 | being closed by a ``}``. For example: | |
223e47cc LB |
1524 | |
1525 | .. code-block:: c++ | |
1526 | ||
1527 | namespace llvm { | |
1528 | namespace knowledge { | |
1529 | ||
970d7e83 | 1530 | /// This class represents things that Smith can have an intimate |
223e47cc LB |
1531 | /// understanding of and contains the data associated with it. |
1532 | class Grokable { | |
1533 | ... | |
1534 | public: | |
1535 | explicit Grokable() { ... } | |
1536 | virtual ~Grokable() = 0; | |
1537 | ||
1538 | ... | |
1539 | ||
1540 | }; | |
1541 | ||
1542 | } // end namespace knowledge | |
1543 | } // end namespace llvm | |
1544 | ||
1a4d82fc JJ |
1545 | |
1546 | Feel free to skip the closing comment when the namespace being closed is | |
1547 | obvious for any reason. For example, the outer-most namespace in a header file | |
1548 | is rarely a source of confusion. But namespaces both anonymous and named in | |
1549 | source files that are being closed half way through the file probably could use | |
1550 | clarification. | |
223e47cc LB |
1551 | |
1552 | .. _static: | |
1553 | ||
1554 | Anonymous Namespaces | |
1555 | ^^^^^^^^^^^^^^^^^^^^ | |
1556 | ||
1557 | After talking about namespaces in general, you may be wondering about anonymous | |
1558 | namespaces in particular. Anonymous namespaces are a great language feature | |
1559 | that tells the C++ compiler that the contents of the namespace are only visible | |
1560 | within the current translation unit, allowing more aggressive optimization and | |
1561 | eliminating the possibility of symbol name collisions. Anonymous namespaces are | |
1562 | to C++ as "static" is to C functions and global variables. While "``static``" | |
1563 | is available in C++, anonymous namespaces are more general: they can make entire | |
1564 | classes private to a file. | |
1565 | ||
1566 | The problem with anonymous namespaces is that they naturally want to encourage | |
1567 | indentation of their body, and they reduce locality of reference: if you see a | |
1568 | random function definition in a C++ file, it is easy to see if it is marked | |
1569 | static, but seeing if it is in an anonymous namespace requires scanning a big | |
1570 | chunk of the file. | |
1571 | ||
1572 | Because of this, we have a simple guideline: make anonymous namespaces as small | |
1573 | as possible, and only use them for class declarations. For example, this is | |
1574 | good: | |
1575 | ||
1576 | .. code-block:: c++ | |
1577 | ||
1578 | namespace { | |
1a4d82fc JJ |
1579 | class StringSort { |
1580 | ... | |
1581 | public: | |
1582 | StringSort(...) | |
1583 | bool operator<(const char *RHS) const; | |
1584 | }; | |
223e47cc LB |
1585 | } // end anonymous namespace |
1586 | ||
1587 | static void runHelper() { | |
1588 | ... | |
1589 | } | |
1590 | ||
1591 | bool StringSort::operator<(const char *RHS) const { | |
1592 | ... | |
1593 | } | |
1594 | ||
1595 | This is bad: | |
1596 | ||
1597 | .. code-block:: c++ | |
1598 | ||
1599 | namespace { | |
1a4d82fc | 1600 | |
223e47cc LB |
1601 | class StringSort { |
1602 | ... | |
1603 | public: | |
1604 | StringSort(...) | |
1605 | bool operator<(const char *RHS) const; | |
1606 | }; | |
1607 | ||
1608 | void runHelper() { | |
1609 | ... | |
1610 | } | |
1611 | ||
1612 | bool StringSort::operator<(const char *RHS) const { | |
1613 | ... | |
1614 | } | |
1615 | ||
1616 | } // end anonymous namespace | |
1617 | ||
1618 | This is bad specifically because if you're looking at "``runHelper``" in the middle | |
1619 | of a large C++ file, that you have no immediate way to tell if it is local to | |
1620 | the file. When it is marked static explicitly, this is immediately obvious. | |
1621 | Also, there is no reason to enclose the definition of "``operator<``" in the | |
1622 | namespace just because it was declared there. | |
1623 | ||
1624 | See Also | |
1625 | ======== | |
1626 | ||
970d7e83 | 1627 | A lot of these comments and recommendations have been culled from other sources. |
223e47cc LB |
1628 | Two particularly important books for our work are: |
1629 | ||
1630 | #. `Effective C++ | |
1631 | <http://www.amazon.com/Effective-Specific-Addison-Wesley-Professional-Computing/dp/0321334876>`_ | |
1632 | by Scott Meyers. Also interesting and useful are "More Effective C++" and | |
1633 | "Effective STL" by the same author. | |
1634 | ||
1635 | #. `Large-Scale C++ Software Design | |
1636 | <http://www.amazon.com/Large-Scale-Software-Design-John-Lakos/dp/0201633620/ref=sr_1_1>`_ | |
1637 | by John Lakos | |
1638 | ||
1639 | If you get some free time, and you haven't read them: do so, you might learn | |
1640 | something. |