diff options
| author | Bill Wendling <isanbard@gmail.com> | 2012-06-20 02:57:56 +0000 |
|---|---|---|
| committer | Bill Wendling <isanbard@gmail.com> | 2012-06-20 02:57:56 +0000 |
| commit | 2c8293d348227e0f064c55d39acca62c46bf4fd4 (patch) | |
| tree | fb101b303a725eda63134e1f45e0cd13f395c237 /docs/CodingStandards.html | |
| parent | ca077ec5ea41f4537ae58e141781be622d09c786 (diff) | |
Sphinxify the CodingStandard documentation.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@158786 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'docs/CodingStandards.html')
| -rw-r--r-- | docs/CodingStandards.html | 1568 |
1 files changed, 0 insertions, 1568 deletions
diff --git a/docs/CodingStandards.html b/docs/CodingStandards.html deleted file mode 100644 index f92c20baa2..0000000000 --- a/docs/CodingStandards.html +++ /dev/null @@ -1,1568 +0,0 @@ -<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" - "http://www.w3.org/TR/html4/strict.dtd"> -<html> -<head> - <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> - <link rel="stylesheet" href="_static/llvm.css" type="text/css"> - <title>LLVM Coding Standards</title> -</head> -<body> - -<h1> - LLVM Coding Standards -</h1> - -<ol> - <li><a href="#introduction">Introduction</a></li> - <li><a href="#mechanicalissues">Mechanical Source Issues</a> - <ol> - <li><a href="#sourceformating">Source Code Formatting</a> - <ol> - <li><a href="#scf_commenting">Commenting</a></li> - <li><a href="#scf_commentformat">Comment Formatting</a></li> - <li><a href="#scf_includes"><tt>#include</tt> Style</a></li> - <li><a href="#scf_codewidth">Source Code Width</a></li> - <li><a href="#scf_spacestabs">Use Spaces Instead of Tabs</a></li> - <li><a href="#scf_indentation">Indent Code Consistently</a></li> - </ol></li> - <li><a href="#compilerissues">Compiler Issues</a> - <ol> - <li><a href="#ci_warningerrors">Treat Compiler Warnings Like - Errors</a></li> - <li><a href="#ci_portable_code">Write Portable Code</a></li> - <li><a href="#ci_rtti_exceptions">Do not use RTTI or Exceptions</a></li> - <li><a href="#ci_static_ctors">Do not use Static Constructors</a></li> - <li><a href="#ci_class_struct">Use of <tt>class</tt>/<tt>struct</tt> Keywords</a></li> - </ol></li> - </ol></li> - <li><a href="#styleissues">Style Issues</a> - <ol> - <li><a href="#macro">The High-Level Issues</a> - <ol> - <li><a href="#hl_module">A Public Header File <b>is</b> a - Module</a></li> - <li><a href="#hl_dontinclude"><tt>#include</tt> as Little as Possible</a></li> - <li><a href="#hl_privateheaders">Keep "internal" Headers - Private</a></li> - <li><a href="#hl_earlyexit">Use Early Exits and <tt>continue</tt> to Simplify - Code</a></li> - <li><a href="#hl_else_after_return">Don't use <tt>else</tt> after a - <tt>return</tt></a></li> - <li><a href="#hl_predicateloops">Turn Predicate Loops into Predicate - Functions</a></li> - </ol></li> - <li><a href="#micro">The Low-Level Issues</a> - <ol> - <li><a href="#ll_naming">Name Types, Functions, Variables, and Enumerators Properly</a></li> - <li><a href="#ll_assert">Assert Liberally</a></li> - <li><a href="#ll_ns_std">Do not use '<tt>using namespace std</tt>'</a></li> - <li><a href="#ll_virtual_anch">Provide a virtual method anchor for - classes in headers</a></li> - <li><a href="#ll_end">Don't evaluate <tt>end()</tt> every time through a - loop</a></li> - <li><a href="#ll_iostream"><tt>#include <iostream></tt> is - <em>forbidden</em></a></li> - <li><a href="#ll_raw_ostream">Use <tt>raw_ostream</tt></a></li> - <li><a href="#ll_avoidendl">Avoid <tt>std::endl</tt></a></li> - </ol></li> - - <li><a href="#nano">Microscopic Details</a> - <ol> - <li><a href="#micro_spaceparen">Spaces Before Parentheses</a></li> - <li><a href="#micro_preincrement">Prefer Preincrement</a></li> - <li><a href="#micro_namespaceindent">Namespace Indentation</a></li> - <li><a href="#micro_anonns">Anonymous Namespaces</a></li> - </ol></li> - - - </ol></li> - <li><a href="#seealso">See Also</a></li> -</ol> - -<div class="doc_author"> - <p>Written by <a href="mailto:sabre@nondot.org">Chris Lattner</a></p> -</div> - - -<!-- *********************************************************************** --> -<h2><a name="introduction">Introduction</a></h2> -<!-- *********************************************************************** --> - -<div> - -<p>This document attempts to describe a few coding standards that are being used -in the LLVM source tree. Although no coding standards should be regarded as -absolute requirements to be followed in all instances, coding standards are -particularly important for large-scale code bases that follow a library-based -design (like LLVM).</p> - -<p>This document intentionally does not prescribe fixed standards for religious -issues such as brace placement and space usage. For issues like this, follow -the golden rule:</p> - -<blockquote> - -<p><b><a name="goldenrule">If you are extending, enhancing, or bug fixing -already implemented code, use the style that is already being used so that the -source is uniform and easy to follow.</a></b></p> - -</blockquote> - -<p>Note that some code bases (e.g. libc++) have really good reasons to deviate -from the coding standards. In the case of libc++, this is because the naming -and other conventions are dictated by the C++ standard. If you think there is -a specific good reason to deviate from the standards here, please bring it up -on the LLVMdev mailing list.</p> - -<p>There are some conventions that are not uniformly followed in the code base -(e.g. the naming convention). This is because they are relatively new, and a -lot of code was written before they were put in place. Our long term goal is -for the entire codebase to follow the convention, but we explicitly <em>do -not</em> want patches that do large-scale reformating of existing code. OTOH, -it is reasonable to rename the methods of a class if you're about to change it -in some other way. Just do the reformating as a separate commit from the -functionality change. </p> - -<p>The ultimate goal of these guidelines is the increase readability and -maintainability of our common source base. If you have suggestions for topics to -be included, please mail them to <a -href="mailto:sabre@nondot.org">Chris</a>.</p> - -</div> - -<!-- *********************************************************************** --> -<h2> - <a name="mechanicalissues">Mechanical Source Issues</a> -</h2> -<!-- *********************************************************************** --> - -<div> - -<!-- ======================================================================= --> -<h3> - <a name="sourceformating">Source Code Formatting</a> -</h3> - -<div> - -<!-- _______________________________________________________________________ --> -<h4> - <a name="scf_commenting">Commenting</a> -</h4> - -<div> - -<p>Comments are one critical part of readability and maintainability. Everyone -knows they should comment their code, and so should you. When writing comments, -write them as English prose, which means they should use proper capitalization, -punctuation, etc. Aim to describe what a code is trying to do and why, not -"how" it does it at a micro level. Here are a few critical things to -document:</p> - -<h5>File Headers</h5> - -<div> - -<p>Every source file should have a header on it that describes the basic -purpose of the file. If a file does not have a header, it should not be -checked into the tree. The standard header looks like this:</p> - -<div class="doc_code"> -<pre> -//===-- llvm/Instruction.h - Instruction class definition -------*- C++ -*-===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// -// -// This file contains the declaration of the Instruction class, which is the -// base class for all of the VM instructions. -// -//===----------------------------------------------------------------------===// -</pre> -</div> - -<p>A few things to note about this particular format: The "<tt>-*- C++ --*-</tt>" string on the first line is there to tell Emacs that the source file -is a C++ file, not a C file (Emacs assumes <tt>.h</tt> files are C files by default). -Note that this tag is not necessary in <tt>.cpp</tt> files. The name of the file is also -on the first line, along with a very short description of the purpose of the -file. This is important when printing out code and flipping though lots of -pages.</p> - -<p>The next section in the file is a concise note that defines the license -that the file is released under. This makes it perfectly clear what terms the -source code can be distributed under and should not be modified in any way.</p> - -<p>The main body of the description does not have to be very long in most cases. -Here it's only two lines. If an algorithm is being implemented or something -tricky is going on, a reference to the paper where it is published should be -included, as well as any notes or "gotchas" in the code to watch out for.</p> - -</div> - -<h5>Class overviews</h5> - -<p>Classes are one fundamental part of a good object oriented design. As such, -a class definition should have a comment block that explains what the class is -used for and how it works. Every non-trivial class is expected to have a -doxygen comment block.</p> - - -<h5>Method information</h5> - -<div> - -<p>Methods defined in a class (as well as any global functions) should also be -documented properly. A quick note about what it does and a description of the -borderline behaviour is all that is necessary here (unless something -particularly tricky or insidious is going on). The hope is that people can -figure out how to use your interfaces without reading the code itself.</p> - -<p>Good things to talk about here are what happens when something unexpected -happens: does the method return null? Abort? Format your hard disk?</p> - -</div> - -</div> - -<!-- _______________________________________________________________________ --> -<h4> - <a name="scf_commentformat">Comment Formatting</a> -</h4> - -<div> - -<p>In general, prefer C++ style (<tt>//</tt>) comments. They take less space, -require less typing, don't have nesting problems, etc. There are a few cases -when it is useful to use C style (<tt>/* */</tt>) comments however:</p> - -<ol> - <li>When writing C code: Obviously if you are writing C code, use C style - comments.</li> - <li>When writing a header file that may be <tt>#include</tt>d by a C source - file.</li> - <li>When writing a source file that is used by a tool that only accepts C - style comments.</li> -</ol> - -<p>To comment out a large block of code, use <tt>#if 0</tt> and <tt>#endif</tt>. -These nest properly and are better behaved in general than C style comments.</p> - -</div> - -<!-- _______________________________________________________________________ --> -<h4> - <a name="scf_includes"><tt>#include</tt> Style</a> -</h4> - -<div> - -<p>Immediately after the <a href="#scf_commenting">header file comment</a> (and -include guards if working on a header file), the <a -href="#hl_dontinclude">minimal</a> list of <tt>#include</tt>s required by the -file should be listed. We prefer these <tt>#include</tt>s to be listed in this -order:</p> - -<ol> - <li><a href="#mmheader">Main Module Header</a></li> - <li><a href="#hl_privateheaders">Local/Private Headers</a></li> - <li><tt>llvm/*</tt></li> - <li><tt>llvm/Analysis/*</tt></li> - <li><tt>llvm/Assembly/*</tt></li> - <li><tt>llvm/Bitcode/*</tt></li> - <li><tt>llvm/CodeGen/*</tt></li> - <li>...</li> - <li><tt>Support/*</tt></li> - <li><tt>Config/*</tt></li> - <li>System <tt>#includes</tt></li> -</ol> - -<p>and each category should be sorted by name.</p> - -<p><a name="mmheader">The "Main Module Header"</a> file applies to <tt>.cpp</tt> files -which implement an interface defined by a <tt>.h</tt> file. This <tt>#include</tt> -should always be included <b>first</b> regardless of where it lives on the file -system. By including a header file first in the <tt>.cpp</tt> files that implement the -interfaces, we ensure that the header does not have any hidden dependencies -which are not explicitly #included in the header, but should be. It is also a -form of documentation in the <tt>.cpp</tt> file to indicate where the interfaces it -implements are defined.</p> - -</div> - -<!-- _______________________________________________________________________ --> -<h4> - <a name="scf_codewidth">Source Code Width</a> -</h4> - -<div> - -<p>Write your code to fit within 80 columns of text. This helps those of us who -like to print out code and look at your code in an xterm without resizing -it.</p> - -<p>The longer answer is that there must be some limit to the width of the code -in order to reasonably allow developers to have multiple files side-by-side in -windows on a modest display. If you are going to pick a width limit, it is -somewhat arbitrary but you might as well pick something standard. Going with -90 columns (for example) instead of 80 columns wouldn't add any significant -value and would be detrimental to printing out code. Also many other projects -have standardized on 80 columns, so some people have already configured their -editors for it (vs something else, like 90 columns).</p> - -<p>This is one of many contentious issues in coding standards, but it is not up -for debate.</p> - -</div> - -<!-- _______________________________________________________________________ --> -<h4> - <a name="scf_spacestabs">Use Spaces Instead of Tabs</a> -</h4> - -<div> - -<p>In all cases, prefer spaces to tabs in source files. People have different -preferred indentation levels, and different styles of indentation that they -like; this is fine. What isn't fine is that different editors/viewers expand -tabs out to different tab stops. This can cause your code to look completely -unreadable, and it is not worth dealing with.</p> - -<p>As always, follow the <a href="#goldenrule">Golden Rule</a> above: follow the -style of existing code if you are modifying and extending it. If you like four -spaces of indentation, <b>DO NOT</b> do that in the middle of a chunk of code -with two spaces of indentation. Also, do not reindent a whole source file: it -makes for incredible diffs that are absolutely worthless.</p> - -</div> - -<!-- _______________________________________________________________________ --> -<h4> - <a name="scf_indentation">Indent Code Consistently</a> -</h4> - -<div> - -<p>Okay, in your first year of programming you were told that indentation is -important. If you didn't believe and internalize this then, now is the time. -Just do it.</p> - -</div> - -</div> - -<!-- ======================================================================= --> -<h3> - <a name="compilerissues">Compiler Issues</a> -</h3> - -<div> - -<!-- _______________________________________________________________________ --> -<h4> - <a name="ci_warningerrors">Treat Compiler Warnings Like Errors</a> -</h4> - -<div> - -<p>If your code has compiler warnings in it, something is wrong — you -aren't casting values correctly, your have "questionable" constructs in your -code, or you are doing something legitimately wrong. Compiler warnings can -cover up legitimate errors in output and make dealing with a translation unit -difficult.</p> - -<p>It is not possible to prevent all warnings from all compilers, nor is it -desirable. Instead, pick a standard compiler (like <tt>gcc</tt>) that provides -a good thorough set of warnings, and stick to it. At least in the case of -<tt>gcc</tt>, it is possible to work around any spurious errors by changing the -syntax of the code slightly. For example, a warning that annoys me occurs when -I write code like this:</p> - -<div class="doc_code"> -<pre> -if (V = getValue()) { - ... -} -</pre> -</div> - -<p><tt>gcc</tt> will warn me that I probably want to use the <tt>==</tt> -operator, and that I probably mistyped it. In most cases, I haven't, and I -really don't want the spurious errors. To fix this particular problem, I -rewrite the code like this:</p> - -<div class="doc_code"> -<pre> -if ((V = getValue())) { - ... -} -</pre> -</div> - -<p>which shuts <tt>gcc</tt> up. Any <tt>gcc</tt> warning that annoys you can -be fixed by massaging the code appropriately.</p> - -</div> - -<!-- _______________________________________________________________________ --> -<h4> - <a name="ci_portable_code">Write Portable Code</a> -</h4> - -<div> - -<p>In almost all cases, it is possible and within reason to write completely -portable code. If there are cases where it isn't possible to write portable -code, isolate it behind a well defined (and well documented) interface.</p> - -<p>In practice, this means that you shouldn't assume much about the host -compiler, and Visual Studio tends to be the lowest common denominator. -If advanced features are used, they should only be an implementation detail of -a library which has a simple exposed API, and preferably be buried in -libSystem.</p> - -</div> - -<!-- _______________________________________________________________________ --> -<h4> -<a name="ci_rtti_exceptions">Do not use RTTI or Exceptions</a> -</h4> -<div> - -<p>In an effort to reduce code and executable size, LLVM does not use RTTI -(e.g. <tt>dynamic_cast<></tt>) or exceptions. These two language features -violate the general C++ principle of <i>"you only pay for what you use"</i>, -causing executable bloat even if exceptions are never used in the code base, or -if RTTI is never used for a class. Because of this, we turn them off globally -in the code.</p> - -<p>That said, LLVM does make extensive use of a hand-rolled form of RTTI that -use templates like <a href="ProgrammersManual.html#isa"><tt>isa<></tt>, -<tt>cast<></tt>, and <tt>dyn_cast<></tt></a>. This form of RTTI is -opt-in and can be added to any class. It is also substantially more efficient -than <tt>dynamic_cast<></tt>.</p> - -</div> - -<!-- _______________________________________________________________________ --> -<h4> -<a name="ci_static_ctors">Do not use Static Constructors</a> -</h4> -<div> - -<p>Static constructors and destructors (e.g. global variables whose types have -a constructor or destructor) should not be added to the code base, and should be -removed wherever possible. Besides <a -href="http://yosefk.com/c++fqa/ctors.html#fqa-10.12">well known problems</a> -where the order of initialization is undefined between globals in different -source files, the entire concept of static constructors is at odds with the -common use case of LLVM as a library linked into a larger application.</p> - -<p>Consider the use of LLVM as a JIT linked into another application (perhaps -for <a href="http://llvm.org/Users.html">OpenGL, custom languages</a>, -<a href="http://llvm.org/devmtg/2010-11/Gritz-OpenShadingLang.pdf">shaders in -movies</a>, etc). Due to the design of static constructors, they must be -executed at startup time of the entire application, regardless of whether or -how LLVM is used in that larger application. There are two problems with -this:</p> - -<ol> - <li>The time to run the static constructors impacts startup time of - applications — a critical time for GUI apps, among others.</li> - - <li>The static constructors cause the app to pull many extra pages of memory - off the disk: both the code for the constructor in each <tt>.o</tt> file and - the small amount of data that gets touched. In addition, touched/dirty pages - put more pressure on the VM system on low-memory machines.</li> -</ol> - -<p>We would really like for there to be zero cost for linking in an additional -LLVM target or other library into an application, but static constructors -violate this goal.</p> - -<p>That said, LLVM unfortunately does contain static constructors. It would be -a <a href="http://llvm.org/PR11944">great project</a> for someone to purge all -static constructors from LLVM, and then enable the -<tt>-Wglobal-constructors</tt> warning flag (when building with Clang) to ensure -we do not regress in the future. -</p> - -</div> - -<!-- _______________________________________________________________________ --> -<h4> -<a name="ci_class_struct">Use of <tt>class</tt> and <tt>struct</tt> Keywords</a> -</h4> -<div> - -<p>In C++, the <tt>class</tt> and <tt>struct</tt> keywords can be used almost -interchangeably. The only difference is when they are used to declare a class: -<tt>class</tt> makes all members private by default while <tt>struct</tt> makes -all members public by default.</p> - -<p>Unfortunately, not all compilers follow the rules and some will generate -different symbols based on whether <tt>class</tt> or <tt>struct</tt> was used to -declare the symbol. This can lead to problems at link time.</p> - -<p>So, the rule for LLVM is to always use the <tt>class</tt> keyword, unless -<b>all</b> members are public and the type is a C++ -<a href="http://en.wikipedia.org/wiki/Plain_old_data_structure">POD</a> type, in -which case <tt>struct</tt> is allowed.</p> - -</div> - -</div> - -</div> - -<!-- *********************************************************************** --> -<h2> - <a name="styleissues">Style Issues</a> -</h2> -<!-- *********************************************************************** --> - -<div> - -<!-- ======================================================================= --> -<h3> - <a name="macro">The High-Level Issues</a> -</h3> -<!-- ======================================================================= --> - -<div> - -<!-- _______________________________________________________________________ --> -<h4> - <a name="hl_module">A Public Header File <b>is</b> a Module</a> -</h4> - -<div> - -<p>C++ doesn't do too well in the modularity department. There is no real -encapsulation or data hiding (unless you use expensive protocol classes), but it -is what we have to work with. When you write a public header file (in the LLVM -source tree, they live in the top level "<tt>include</tt>" directory), you are -defining a module of functionality.</p> - -<p>Ideally, modules should be completely independent of each other, and their -header files should only <tt>#include</tt> the absolute minimum number of -headers possible. A module is not just a class, a function, or a -namespace: <a href="http://www.cuj.com/articles/2000/0002/0002c/0002c.htm">it's -a collection of these</a> that defines an interface. This interface may be -several functions, classes, or data structures, but the important issue is how -they work together.</p> - -<p>In general, a module should be implemented by one or more <tt>.cpp</tt> -files. Each of these <tt>.cpp</tt> files should include the header that defines -their interface first. This ensures that all of the dependences of the module -header have been properly added to the module header itself, and are not -implicit. System headers should be included after user headers for a -translation unit.</p> - -</div> - -<!-- _______________________________________________________________________ --> -<h4> - <a name="hl_dontinclude"><tt>#include</tt> as Little as Possible</a> -</h4> - -<div> - -<p><tt>#include</tt> hurts compile time performance. Don't do it unless you -have to, especially in header files.</p> - -<p>But wait! Sometimes you need to have the definition of a class to use it, or -to inherit from it. In these cases go ahead and <tt>#include</tt> that header -file. Be aware however that there are many cases where you don't need to have -the full definition of a class. If you are using a pointer or reference to a -class, you don't need the header file. If you are simply returning a class -instance from a prototyped function or method, you don't need it. In fact, for -most cases, you simply don't need the definition of a class. And not -<tt>#include</tt>'ing speeds up compilation.</p> - -<p>It is easy to try to go too overboard on this recommendation, however. You -<b>must</b> include all of the header files that you are using — you can -include them either directly or indirectly (through another header file). To -make sure that you don't accidentally forget to include a header file in your -module header, make sure to include your module header <b>first</b> in the -implementation file (as mentioned above). This way there won't be any hidden -dependencies that you'll find out about later.</p> - -</div> - -<!-- _______________________________________________________________________ --> -<h4> - <a name="hl_privateheaders">Keep "Internal" Headers Private</a> -</h4> - -<div> - -<p>Many modules have a complex implementation that causes them to use more than -one implementation (<tt>.cpp</tt>) file. It is often tempting to put the -internal communication interface (helper classes, extra functions, etc) in the -public module header file. Don't do this!</p> - -<p>If you really need to do something like this, put a private header file in -the same directory as the source files, and include it locally. This ensures -that your private interface remains private and undisturbed by outsiders.</p> - -<p>Note however, that it's okay to put extra implementation methods in a public -class itself. Just make them private (or protected) and all is well.</p> - -</div> - -<!-- _______________________________________________________________________ --> -<h4> - <a name="hl_earlyexit">Use Early Exits and <tt>continue</tt> to Simplify Code</a> -</h4> - -<div> - -<p>When reading code, keep in mind how much state and how many previous -decisions have to be remembered by the reader to understand a block of code. -Aim to reduce indentation where possible when it doesn't make it more difficult -to understand the code. One great way to do this is by making use of early -exits and the <tt>continue</tt> keyword in long loops. As an example of using -an early exit from a function, consider this "bad" code:</p> - -<div class="doc_code"> -<pre> -Value *DoSomething(Instruction *I) { - if (!isa<TerminatorInst>(I) && - I->hasOneUse() && SomeOtherThing(I)) { - ... some long code .... - } - - return 0; -} -</pre> -</div> - -<p>This code has several problems if the body of the '<tt>if</tt>' is large. -When you're looking at the top of the function, it isn't immediately clear that -this <em>only</em> does interesting things with non-terminator instructions, and -only applies to things with the other predicates. Second, it is relatively -difficult to describe (in comments) why these predicates are important because -the <tt>if</tt> statement makes it difficult to lay out the comments. Third, -when you're deep within the body of the code, it is indented an extra level. -Finally, when reading the top of the function, it isn't clear what the result is -if the predicate isn't true; you have to read to the end of the function to know -that it returns null.</p> - -<p>It is much preferred to format the code like this:</p> - -<div class="doc_code"> -<pre> -Value *DoSomething(Instruction *I) { - // Terminators never need 'something' done to them because ... - if (isa<TerminatorInst>(I)) - return 0; - - // We conservatively avoid transforming instructions with multiple uses - // because goats like cheese. - if (!I->hasOneUse()) - return 0; - - // This is really just here for example. - if (!SomeOtherThing(I)) - return 0; - - ... some long code .... -} -</pre> -</div> - -<p>This fixes these problems. A similar problem frequently happens in <tt>for</tt> -loops. A silly example is something like this:</p> - -<div class="doc_code"> -<pre> - for (BasicBlock::iterator II = BB->begin(), E = BB->end(); II != E; ++II) { - if (BinaryOperator *BO = dyn_cast<BinaryOperator>(II)) { - Value *LHS = BO->getOperand(0); - Value *RHS = BO->getOperand(1); - if (LHS != RHS) { - ... - } - } - } -</pre> -</div> - -<p>When you have very, very small loops, this sort of structure is fine. But if -it exceeds more than 10-15 lines, it becomes difficult for people to read and -understand at a glance. The problem with this sort of code is that it gets very -nested very quickly. Meaning that the reader of the code has to keep a lot of -context in their brain to remember what is going immediately on in the loop, -because they don't know if/when the <tt>if</tt> conditions will have elses etc. -It is strongly preferred to structure the loop like this:</p> - -<div class="doc_code"> -<pre> - for (BasicBlock::iterator II = BB->begin(), E = BB->end(); II != E; ++II) { - BinaryOperator *BO = dyn_cast<BinaryOperator>(II); - if (!BO) continue; - - Value *LHS = BO->getOperand(0); - Value *RHS = BO->getOperand(1); - if (LHS == RHS) continue; - - ... - } -</pre> -</div> - -<p>This has all the benefits of using early exits for functions: it reduces -nesting of the loop, it makes it easier to describe why the conditions are true, -and it makes it obvious to the reader that there is no <tt>else</tt> coming up -that they have to push context into their brain for. If a loop is large, this -can be a big understandability win.</p> - -</div> - -<!-- _______________________________________________________________________ --> -<h4> - <a name="hl_else_after_return">Don't use <tt>else</tt> after a <tt>return</tt></a> -</h4> - -<div> - -<p>For similar reasons above (reduction of indentation and easier reading), -please do not use '<tt>else</tt>' or '<tt>else if</tt>' after something that -interrupts control flow — like <tt>return</tt>, <tt>break</tt>, -<tt>continue</tt>, <tt>goto</tt>, etc. For example, this is <em>bad</em>:</p> - -<div class="doc_code"> -<pre> - case 'J': { - if (Signed) { - Type = Context.getsigjmp_bufType(); - if (Type.isNull()) { - Error = ASTContext::GE_Missing_sigjmp_buf; - return QualType(); - <b>} else { - break; - }</b> - } else { - Type = Context.getjmp_bufType(); - if (Type.isNull()) { - Error = ASTContext::GE_Missing_jmp_buf; - return QualType(); - <b>} else { - break; - }</b> - } - } - } -</pre> -</div> - -<p>It is better to write it like this:</p> - -<div class="doc_code"> -<pre> - case 'J': - if (Signed) { - Type = Context.getsigjmp_bufType(); - if (Type.isNull()) { - Error = ASTContext::GE_Missing_sigjmp_buf; - return QualType(); - } - } else { - Type = Context.getjmp_bufType(); - if (Type.isNull()) { - Error = ASTContext::GE_Missing_jmp_buf; - return QualType(); - } - } - <b>break;</b> -</pre> -</div> - -<p>Or better yet (in this case) as:</p> - -<div class="doc_code"> -<pre> - case 'J': - if (Signed) - Type = Context.getsigjmp_bufType(); - else - Type = Context.getjmp_bufType(); - - if (Type.isNull()) { - Error = Signed ? ASTContext::GE_Missing_sigjmp_buf : - ASTContext::GE_Missing_jmp_buf; - return QualType(); - } - <b>break;</b> -</pre> -</div> - -<p>The idea is to reduce indentation and the amount of code you have to keep -track of when reading the code.</p> - -</div> - -<!-- _______________________________________________________________________ --> -<h4> - <a name="hl_predicateloops">Turn Predicate Loops into Predicate Functions</a> -</h4> - -<div> - -<p>It is very common to write small loops that just compute a boolean value. -There are a number of ways that people commonly write these, but an example of -this sort of thing is:</p> - -<div class="doc_code"> -<pre> - <b>bool FoundFoo = false;</b> - for (unsigned i = 0, e = BarList.size(); i != e; ++i) - if (BarList[i]->isFoo()) { - <b>FoundFoo = true;</b> - break; - } - - <b>if (FoundFoo) {</b> - ... - } -</pre> -</div> - -<p>This sort of code is awkward to write, and is almost always a bad sign. -Instead of this sort of loop, we strongly prefer to use a predicate function -(which may be <a href="#micro_anonns">static</a>) that uses -<a href="#hl_earlyexit">early exits</a> to compute the predicate. We prefer -the code to be structured like this:</p> - -<div class="doc_code"> -<pre> -/// ListContainsFoo - Return true if the specified list has an element that is -/// a foo. -static bool ListContainsFoo(const std::vector<Bar*> &List) { - for (unsigned i = 0, e = List.size(); i != e; ++i) - if (List[i]->isFoo()) - return true; - return false; -} -... - - <b>if (ListContainsFoo(BarList)) {</b> - ... - } -</pre> -</div> - -<p>There are many reasons for doing this: it reduces indentation and factors out -code which can often be shared by other code that checks for the same predicate. -More importantly, it <em>forces you to pick a name</em> for the function, and -forces you to write a comment for it. In this silly example, this doesn't add -much value. However, if the condition is complex, this can make it a lot easier -for the reader to understand the code that queries for this predicate. Instead -of being faced with the in-line details of how we check to see if the BarList -contains a foo, we |