-.. _coding_standards:
-
=====================
LLVM Coding Standards
=====================
particularly important for large-scale code bases that follow a library-based
design (like LLVM).
-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:
+While this document may provide guidance for some mechanical formatting issues,
+whitespace, or other "microscopic details", these are not fixed standards.
+Always follow the golden rule:
.. _Golden Rule:
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.
+it up on the LLVM-dev mailing list.
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
change it in some other way. Just do the reformating as a separate commit from
the functionality change.
-The ultimate goal of these guidelines is the increase readability and
+The ultimate goal of these guidelines is to increase the readability and
maintainability of our common source base. If you have suggestions for topics to
be included, please mail them to `Chris <mailto:sabre@nondot.org>`_.
+Languages, Libraries, and Standards
+===================================
+
+Most source code in LLVM and other LLVM projects using these coding standards
+is C++ code. There are some places where C code is used either due to
+environment restrictions, historical restrictions, or due to third-party source
+code imported into the tree. Generally, our preference is for standards
+conforming, modern, and portable C++ code as the implementation language of
+choice.
+
+C++ Standard Versions
+---------------------
+
+LLVM, Clang, and LLD are currently written using C++11 conforming code,
+although we restrict ourselves to features which are available in the major
+toolchains supported as host compilers. The LLDB project is even more
+aggressive in the set of host compilers supported and thus uses still more
+features. Regardless of the supported features, code is expected to (when
+reasonable) be standard, portable, and modern C++11 code. We avoid unnecessary
+vendor-specific extensions, etc.
+
+C++ Standard Library
+--------------------
+
+Use the C++ standard library facilities whenever they are available for
+a particular task. LLVM and related projects emphasize and rely on the standard
+library facilities for as much as possible. Common support libraries providing
+functionality missing from the standard library for which there are standard
+interfaces or active work on adding standard interfaces will often be
+implemented in the LLVM namespace following the expected standard interface.
+
+There are some exceptions such as the standard I/O streams library which are
+avoided. Also, there is much more detailed information on these subjects in the
+:doc:`ProgrammersManual`.
+
+Supported C++11 Language and Library Features
+---------------------------------------------
+
+While LLVM, Clang, and LLD use C++11, not all features are available in all of
+the toolchains which we support. The set of features supported for use in LLVM
+is the intersection of those supported in MSVC 2013, GCC 4.7, and Clang 3.1.
+The ultimate definition of this set is what build bots with those respective
+toolchains accept. Don't argue with the build bots. However, we have some
+guidance below to help you know what to expect.
+
+Each toolchain provides a good reference for what it accepts:
+
+* Clang: http://clang.llvm.org/cxx_status.html
+* GCC: http://gcc.gnu.org/projects/cxx0x.html
+* MSVC: http://msdn.microsoft.com/en-us/library/hh567368.aspx
+
+In most cases, the MSVC list will be the dominating factor. Here is a summary
+of the features that are expected to work. Features not on this list are
+unlikely to be supported by our host compilers.
+
+* Rvalue references: N2118_
+
+ * But *not* Rvalue references for ``*this`` or member qualifiers (N2439_)
+
+* Static assert: N1720_
+* ``auto`` type deduction: N1984_, N1737_
+* Trailing return types: N2541_
+* Lambdas: N2927_
+
+ * But *not* lambdas with default arguments.
+
+* ``decltype``: N2343_
+* Nested closing right angle brackets: N1757_
+* Extern templates: N1987_
+* ``nullptr``: N2431_
+* Strongly-typed and forward declarable enums: N2347_, N2764_
+* Local and unnamed types as template arguments: N2657_
+* Range-based for-loop: N2930_
+
+ * But ``{}`` are required around inner ``do {} while()`` loops. As a result,
+ ``{}`` are required around function-like macros inside range-based for
+ loops.
+
+* ``override`` and ``final``: N2928_, N3206_, N3272_
+* Atomic operations and the C++11 memory model: N2429_
+* Variadic templates: N2242_
+* Explicit conversion operators: N2437_
+* Defaulted and deleted functions: N2346_
+
+ * But not defaulted move constructors or move assignment operators, MSVC 2013
+ cannot synthesize them.
+* Initializer lists: N2627_
+* Delegating constructors: N1986_
+* Default member initializers (non-static data member initializers): N2756_
+
+ * Only use these for scalar members that would otherwise be left
+ uninitialized. Non-scalar members generally have appropriate default
+ constructors, and MSVC 2013 has problems when braced initializer lists are
+ involved.
+
+.. _N2118: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2118.html
+.. _N2439: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2439.htm
+.. _N1720: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2004/n1720.html
+.. _N1984: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n1984.pdf
+.. _N1737: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2004/n1737.pdf
+.. _N2541: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2541.htm
+.. _N2927: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2009/n2927.pdf
+.. _N2343: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2343.pdf
+.. _N1757: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2005/n1757.html
+.. _N1987: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n1987.htm
+.. _N2431: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2431.pdf
+.. _N2347: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2347.pdf
+.. _N2764: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2764.pdf
+.. _N2657: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2657.htm
+.. _N2930: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2009/n2930.html
+.. _N2928: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2009/n2928.htm
+.. _N3206: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2010/n3206.htm
+.. _N3272: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2011/n3272.htm
+.. _N2429: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2429.htm
+.. _N2242: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2242.pdf
+.. _N2437: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2437.pdf
+.. _N2346: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2346.htm
+.. _N2627: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2672.htm
+.. _N1986: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n1986.pdf
+.. _N2756: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2756.htm
+
+The supported features in the C++11 standard libraries are less well tracked,
+but also much greater. Most of the standard libraries implement most of C++11's
+library. The most likely lowest common denominator is Linux support. For
+libc++, the support is just poorly tested and undocumented but expected to be
+largely complete. YMMV. For libstdc++, the support is documented in detail in
+`the libstdc++ manual`_. There are some very minor missing facilities that are
+unlikely to be common problems, and there are a few larger gaps that are worth
+being aware of:
+
+* Not all of the type traits are implemented
+* No regular expression library.
+* While most of the atomics library is well implemented, the fences are
+ missing. Fortunately, they are rarely needed.
+* The locale support is incomplete.
+* ``std::equal()`` (and other algorithms) incorrectly assert in MSVC when given
+ ``nullptr`` as an iterator.
+
+Other than these areas you should assume the standard library is available and
+working as expected until some build bot tells you otherwise. If you're in an
+uncertain area of one of the above points, but you cannot test on a Linux
+system, your best approach is to minimize your use of these features, and watch
+the Linux build bots to find out if your usage triggered a bug. For example, if
+you hit a type trait which doesn't work we can then add support to LLVM's
+traits header to emulate it.
+
+.. _the libstdc++ manual:
+ http://gcc.gnu.org/onlinedocs/gcc-4.7.3/libstdc++/manual/manual/status.html#status.iso.2011
+
+Other Languages
+---------------
+
+Any code written in the Go programming language is not subject to the
+formatting rules below. Instead, we adopt the formatting rules enforced by
+the `gofmt`_ tool.
+
+Go code should strive to be idiomatic. Two good sets of guidelines for what
+this means are `Effective Go`_ and `Go Code Review Comments`_.
+
+.. _gofmt:
+ https://golang.org/cmd/gofmt/
+
+.. _Effective Go:
+ https://golang.org/doc/effective_go.html
+
+.. _Go Code Review Comments:
+ https://code.google.com/p/go-wiki/wiki/CodeReviewComments
+
Mechanical Source Issues
========================
// 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.
- //
+ ///
+ /// \file
+ /// This file contains the declaration of the Instruction class, which is the
+ /// base class for all of the VM instructions.
+ ///
//===----------------------------------------------------------------------===//
A few things to note about this particular format: The "``-*- C++ -*-``" string
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.
-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.
+The main body is a ``doxygen`` comment (identified by the ``///`` comment
+marker instead of the usual ``//``) describing the purpose of the file. The
+first sentence or a passage beginning with ``\brief`` is used as an abstract.
+Any additional information should be separated by a blank line. 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.
Class overviews
"""""""""""""""
Comment Formatting
^^^^^^^^^^^^^^^^^^
-In general, prefer C++ style (``//``) comments. They take less space, require
+In general, prefer C++ style comments (``//`` for normal comments, ``///`` for
+``doxygen`` documentation 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 (``/* */``) comments however:
To comment out a large block of code, use ``#if 0`` and ``#endif``. These nest
properly and are better behaved in general than C style comments.
+Doxygen Use in Documentation Comments
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Use the ``\file`` command to turn the standard file header into a file-level
+comment.
+
+Include descriptive paragraphs for all public interfaces (public classes,
+member and non-member functions). Don't just restate the information that can
+be inferred from the API name. The first sentence or a paragraph beginning
+with ``\brief`` is used as an abstract. Put detailed discussion into separate
+paragraphs.
+
+To refer to parameter names inside a paragraph, use the ``\p name`` command.
+Don't use the ``\arg name`` command since it starts a new paragraph that
+contains documentation for the parameter.
+
+Wrap non-inline code examples in ``\code ... \endcode``.
+
+To document a function parameter, start a new paragraph with the
+``\param name`` command. If the parameter is used as an out or an in/out
+parameter, use the ``\param [out] name`` or ``\param [in,out] name`` command,
+respectively.
+
+To describe function return value, start a new paragraph with the ``\returns``
+command.
+
+A minimal documentation comment:
+
+.. code-block:: c++
+
+ /// Sets the xyzzy property to \p Baz.
+ void setXyzzy(bool Baz);
+
+A documentation comment that uses all Doxygen features in a preferred way:
+
+.. code-block:: c++
+
+ /// \brief Does foo and bar.
+ ///
+ /// Does not do foo the usual way if \p Baz is true.
+ ///
+ /// Typical usage:
+ /// \code
+ /// fooBar(false, "quux", Res);
+ /// \endcode
+ ///
+ /// \param Quux kind of foo to do.
+ /// \param [out] Result filled with bar sequence on foo success.
+ ///
+ /// \returns true on success.
+ bool fooBar(bool Baz, StringRef Quux, std::vector<int> &Result);
+
+Don't duplicate the documentation comment in the header file and in the
+implementation file. Put the documentation comments for public APIs into the
+header file. Documentation comments for private APIs can go to the
+implementation file. In any case, implementation files can include additional
+comments (not necessarily in Doxygen markup) to explain implementation details
+as needed.
+
+Don't duplicate function or class name at the beginning of the comment.
+For humans it is obvious which function or class is being documented;
+automatic documentation processing tools are smart enough to bind the comment
+to the correct declaration.
+
+Wrong:
+
+.. code-block:: c++
+
+ // In Something.h:
+
+ /// Something - An abstraction for some complicated thing.
+ class Something {
+ public:
+ /// fooBar - Does foo and bar.
+ void fooBar();
+ };
+
+ // In Something.cpp:
+
+ /// fooBar - Does foo and bar.
+ void Something::fooBar() { ... }
+
+Correct:
+
+.. code-block:: c++
+
+ // In Something.h:
+
+ /// An abstraction for some complicated thing.
+ class Something {
+ public:
+ /// Does foo and bar.
+ void fooBar();
+ };
+
+ // In Something.cpp:
+
+ // Builds a B-tree in order to do foo. See paper by...
+ void Something::fooBar() { ... }
+
+It is not required to use additional Doxygen features, but sometimes it might
+be a good idea to do so.
+
+Consider:
+
+* adding comments to any narrow namespace containing a collection of
+ related functions or types;
+
+* using top-level groups to organize a collection of related functions at
+ namespace scope where the grouping is smaller than the namespace;
+
+* using member groups and additional comments attached to member
+ groups to organize within a class.
+
+For example:
+
+.. code-block:: c++
+
+ class Something {
+ /// \name Functions that do Foo.
+ /// @{
+ void fooBar();
+ void fooBaz();
+ /// @}
+ ...
+ };
+
``#include`` Style
^^^^^^^^^^^^^^^^^^
#. Main Module Header
#. Local/Private Headers
-#. ``llvm/*``
-#. ``llvm/Analysis/*``
-#. ``llvm/Assembly/*``
-#. ``llvm/Bitcode/*``
-#. ``llvm/CodeGen/*``
-#. ...
-#. ``llvm/Support/*``
-#. ``llvm/Config/*``
+#. ``llvm/...``
#. System ``#include``\s
-and each category should be sorted by name.
+and each category should be sorted lexicographically by the full path.
The `Main Module Header`_ file applies to ``.cpp`` files which implement an
interface defined by a ``.h`` file. This ``#include`` should always be included
^^^^^^^^^^^^^^^^^^^^^^^^
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.
+important. If you didn't believe and internalize this then, now is the time.
+Just do it. With the introduction of C++11, there are some new formatting
+challenges that merit some suggestions to help have consistent, maintainable,
+and tool-friendly formatting and indentation.
-Compiler Issues
----------------
+Format Lambdas Like Blocks Of Code
+""""""""""""""""""""""""""""""""""
+
+When formatting a multi-line lambda, format it like a block of code, that's
+what it is. If there is only one multi-line lambda in a statement, and there
+are no expressions lexically after it in the statement, drop the indent to the
+standard two space indent for a block of code, as if it were an if-block opened
+by the preceding part of the statement:
+
+.. code-block:: c++
+
+ std::sort(foo.begin(), foo.end(), [&](Foo a, Foo b) -> bool {
+ if (a.blah < b.blah)
+ return true;
+ if (a.baz < b.baz)
+ return true;
+ return a.bam < b.bam;
+ });
+
+To take best advantage of this formatting, if you are designing an API which
+accepts a continuation or single callable argument (be it a functor, or
+a ``std::function``), it should be the last argument if at all possible.
+
+If there are multiple multi-line lambdas in a statement, or there is anything
+interesting after the lambda in the statement, indent the block two spaces from
+the indent of the ``[]``:
+
+.. code-block:: c++
+
+ dyn_switch(V->stripPointerCasts(),
+ [] (PHINode *PN) {
+ // process phis...
+ },
+ [] (SelectInst *SI) {
+ // process selects...
+ },
+ [] (LoadInst *LI) {
+ // process loads...
+ },
+ [] (AllocaInst *AI) {
+ // process allocas...
+ });
+
+Braced Initializer Lists
+""""""""""""""""""""""""
+
+With C++11, there are significantly more uses of braced lists to perform
+initialization. These allow you to easily construct aggregate temporaries in
+expressions among other niceness. They now have a natural way of ending up
+nested within each other and within function calls in order to build up
+aggregates (such as option structs) from local variables. To make matters
+worse, we also have many more uses of braces in an expression context that are
+*not* performing initialization.
+
+The historically common formatting of braced initialization of aggregate
+variables does not mix cleanly with deep nesting, general expression contexts,
+function arguments, and lambdas. We suggest new code use a simple rule for
+formatting braced initialization lists: act as-if the braces were parentheses
+in a function call. The formatting rules exactly match those already well
+understood for formatting nested function calls. Examples:
+
+.. code-block:: c++
+
+ foo({a, b, c}, {1, 2, 3});
+
+ llvm::Constant *Mask[] = {
+ llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 0),
+ llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 1),
+ llvm::ConstantInt::get(llvm::Type::getInt32Ty(getLLVMContext()), 2)};
+
+This formatting scheme also makes it particularly easy to get predictable,
+consistent, and automatic formatting with tools like `Clang Format`_.
+
+.. _Clang Format: http://clang.llvm.org/docs/ClangFormat.html
+
+Language and Compiler Issues
+----------------------------
Treat Compiler Warnings Like Errors
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
code.
That said, LLVM does make extensive use of a hand-rolled form of RTTI that use
-templates like `isa<>, cast<>, and dyn_cast<> <ProgrammersManual.html#isa>`_.
-This form of RTTI is opt-in and can be added to any class. It is also
+templates like :ref:`isa\<>, cast\<>, and dyn_cast\<> <isa>`.
+This form of RTTI is opt-in and can be
+:doc:`added to any class <HowToSetUpLLVMStyleRTTI>`. It is also
substantially more efficient than ``dynamic_cast<>``.
.. _static constructor:
Unfortunately, not all compilers follow the rules and some will generate
different symbols based on whether ``class`` or ``struct`` was used to declare
-the symbol. This can lead to problems at link time.
+the symbol (e.g., MSVC). This can lead to problems at link time.
+
+* All declarations and definitions of a given ``class`` or ``struct`` must use
+ the same keyword. For example:
+
+.. code-block:: c++
-So, the rule for LLVM is to always use the ``class`` keyword, unless **all**
-members are public and the type is a C++ `POD
-<http://en.wikipedia.org/wiki/Plain_old_data_structure>`_ type, in which case
-``struct`` is allowed.
+ class Foo;
+
+ // Breaks mangling in MSVC.
+ struct Foo { int Data; };
+
+* As a rule of thumb, ``struct`` should be kept to structures where *all*
+ members are declared public.
+
+.. code-block:: c++
+
+ // Foo feels like a class... this is strange.
+ struct Foo {
+ private:
+ int Data;
+ public:
+ Foo() : Data(0) { }
+ int getData() const { return Data; }
+ void setData(int D) { Data = D; }
+ };
+
+ // Bar isn't POD, but it does look like a struct.
+ struct Bar {
+ int Data;
+ Bar() : Data(0) { }
+ };
+
+Do not use Braced Initializer Lists to Call a Constructor
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+In C++11 there is a "generalized initialization syntax" which allows calling
+constructors using braced initializer lists. Do not use these to call
+constructors with any interesting logic or if you care that you're calling some
+*particular* constructor. Those should look like function calls using
+parentheses rather than like aggregate initialization. Similarly, if you need
+to explicitly name the type and call its constructor to create a temporary,
+don't use a braced initializer list. Instead, use a braced initializer list
+(without any type for temporaries) when doing aggregate initialization or
+something notionally equivalent. Examples:
+
+.. code-block:: c++
+
+ class Foo {
+ public:
+ // Construct a Foo by reading data from the disk in the whizbang format, ...
+ Foo(std::string filename);
+
+ // Construct a Foo by looking up the Nth element of some global data ...
+ Foo(int N);
+
+ // ...
+ };
+
+ // The Foo constructor call is very deliberate, no braces.
+ std::fill(foo.begin(), foo.end(), Foo("name"));
+
+ // The pair is just being constructed like an aggregate, use braces.
+ bar_map.insert({my_key, my_value});
+
+If you use a braced initializer list when initializing a variable, use an equals before the open curly brace:
+
+.. code-block:: c++
+
+ int data[] = {0, 1, 2, 3};
+
+Use ``auto`` Type Deduction to Make Code More Readable
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Some are advocating a policy of "almost always ``auto``" in C++11, however LLVM
+uses a more moderate stance. Use ``auto`` if and only if it makes the code more
+readable or easier to maintain. Don't "almost always" use ``auto``, but do use
+``auto`` with initializers like ``cast<Foo>(...)`` or other places where the
+type is already obvious from the context. Another time when ``auto`` works well
+for these purposes is when the type would have been abstracted away anyways,
+often behind a container's typedef such as ``std::vector<T>::iterator``.
+
+Beware unnecessary copies with ``auto``
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The convenience of ``auto`` makes it easy to forget that its default behavior
+is a copy. Particularly in range-based ``for`` loops, careless copies are
+expensive.
+
+As a rule of thumb, use ``auto &`` unless you need to copy the result, and use
+``auto *`` when copying pointers.
+
+.. code-block:: c++
+
+ // Typically there's no reason to copy.
+ for (const auto &Val : Container) { observe(Val); }
+ for (auto &Val : Container) { Val.change(); }
+
+ // Remove the reference if you really want a new copy.
+ for (auto Val : Container) { Val.change(); saveSomewhere(Val); }
+
+ // Copy pointers, but make it clear that they're pointers.
+ for (const auto *Ptr : Container) { observe(*Ptr); }
+ for (auto *Ptr : Container) { Ptr->change(); }
Style Issues
============
.. code-block:: c++
- Value *DoSomething(Instruction *I) {
+ Value *doSomething(Instruction *I) {
if (!isa<TerminatorInst>(I) &&
- I->hasOneUse() && SomeOtherThing(I)) {
+ I->hasOneUse() && doOtherThing(I)) {
... some long code ....
}
.. code-block:: c++
- Value *DoSomething(Instruction *I) {
+ Value *doSomething(Instruction *I) {
// Terminators never need 'something' done to them because ...
if (isa<TerminatorInst>(I))
return 0;
return 0;
// This is really just here for example.
- if (!SomeOtherThing(I))
+ if (!doOtherThing(I))
return 0;
... some long code ....
.. code-block:: c++
bool FoundFoo = false;
- for (unsigned i = 0, e = BarList.size(); i != e; ++i)
- if (BarList[i]->isFoo()) {
+ for (unsigned I = 0, E = BarList.size(); I != E; ++I)
+ if (BarList[I]->isFoo()) {
FoundFoo = true;
break;
}
.. code-block:: c++
- /// 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())
+ /// \returns true if the specified list has an element that is a foo.
+ static bool containsFoo(const std::vector<Bar*> &List) {
+ for (unsigned I = 0, E = List.size(); I != E; ++I)
+ if (List[I]->isFoo())
return true;
return false;
}
...
- if (ListContainsFoo(BarList)) {
+ if (containsFoo(BarList)) {
...
}
As an exception, classes that mimic STL classes can have member names in STL's
style of lower-case words separated by underscores (e.g. ``begin()``,
-``push_back()``, and ``empty()``).
+``push_back()``, and ``empty()``). Classes that provide multiple
+iterators should add a singular prefix to ``begin()`` and ``end()``
+(e.g. ``global_begin()`` and ``use_begin()``).
Here are some examples of good and bad names:
Vehicle MakeVehicle(VehicleType Type) {
VehicleMaker M; // Might be OK if having a short life-span.
- Tire tmp1 = M.makeTire(); // Bad -- 'tmp1' provides no information.
- Light headlight = M.makeLight("head"); // Good -- descriptive.
+ Tire Tmp1 = M.makeTire(); // Bad -- 'Tmp1' provides no information.
+ Light Headlight = M.makeLight("head"); // Good -- descriptive.
...
}
.. code-block:: c++
- inline Value *getOperand(unsigned i) {
- assert(i < Operands.size() && "getOperand() out of range!");
- return Operands[i];
+ inline Value *getOperand(unsigned I) {
+ assert(I < Operands.size() && "getOperand() out of range!");
+ return Operands[I];
}
Here are more examples:
.. code-block:: c++
- assert(Ty->isPointerType() && "Can't allocate a non pointer type!");
+ assert(Ty->isPointerType() && "Can't allocate a non-pointer type!");
assert((Opcode == Shl || Opcode == Shr) && "ShiftInst Opcode invalid!");
You get the idea.
-Please be aware that, when adding assert statements, not all compilers are aware
-of the semantics of the assert. In some places, asserts are used to indicate a
-piece of code that should not be reached. These are typically of the form:
+In the past, asserts were used to indicate a piece of code that should not be
+reached. These were typically of the form:
.. code-block:: c++
- assert(0 && "Some helpful error message");
+ assert(0 && "Invalid radix for integer literal");
+
+This has a few issues, the main one being that some compilers might not
+understand the assertion, or warn about a missing return in builds where
+assertions are compiled out.
-When used in a function that returns a value, they should be followed with a
-return statement and a comment indicating that this line is never reached. This
-will prevent a compiler which is unable to deduce that the assert statement
-never returns from generating a warning.
+Today, we have something much better: ``llvm_unreachable``:
.. code-block:: c++
- assert(0 && "Some helpful error message");
- return 0;
+ llvm_unreachable("Invalid radix for integer literal");
+
+When assertions are enabled, this will print the message if it's ever reached
+and then exit the program. When assertions are disabled (i.e. in release
+builds), ``llvm_unreachable`` becomes a hint to compilers to skip generating
+code for this branch. If the compiler does not support this, it will fall back
+to the "abort" implementation.
Another issue is that values used only by assertions will produce an "unused
value" warning when assertions are disabled. For example, this code will warn:
will copy the vtable and RTTI into every ``.o`` file that ``#include``\s the
header, bloating ``.o`` file sizes and increasing link times.
-Use ``LLVM_DELETED_FUNCTION`` to mark uncallable methods
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-Prior to C++11, a common pattern to make a class uncopyable was to declare an
-unimplemented copy constructor and copy assignment operator and make them
-private. This would give a compiler error for accessing a private method or a
-linker error because it wasn't implemented.
-
-With C++11, we can mark methods that won't be implemented with ``= delete``.
-This will trigger a much better error message and tell the compiler that the
-method will never be implemented. This enables other checks like
-``-Wunused-private-field`` to run correctly on classes that contain these
-methods.
+Don't use default labels in fully covered switches over enumerations
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-To maintain compatibility with C++03, ``LLVM_DELETED_FUNCTION`` should be used
-which will expand to ``= delete`` if the compiler supports it. These methods
-should still be declared private. Example of the uncopyable pattern:
+``-Wswitch`` warns if a switch, without a default label, over an enumeration
+does not cover every enumeration value. If you write a default label on a fully
+covered switch over an enumeration then the ``-Wswitch`` warning won't fire
+when new elements are added to that enumeration. To help avoid adding these
+kinds of defaults, Clang has the warning ``-Wcovered-switch-default`` which is
+off by default but turned on when building LLVM with a version of Clang that
+supports the warning.
-.. code-block:: c++
-
- class DontCopy {
- private:
- DontCopy(const DontCopy&) LLVM_DELETED_FUNCTION;
- DontCopy &operator =(const DontCopy&) LLVM_DELETED_FUNCTION;
- public:
- ...
- };
+A knock-on effect of this stylistic requirement is that when building LLVM with
+GCC you may get warnings related to "control may reach end of non-void function"
+if you return from each case of a covered switch-over-enum because GCC assumes
+that the enum expression may take any representable value, not just those of
+individual enumerators. To suppress this warning, use ``llvm_unreachable`` after
+the switch.
Don't evaluate ``end()`` every time through a loop
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
start of the loop. In this case, the cost is probably minor --- a few extra
loads every time through the loop. However, if the base expression is more
complex, then the cost can rise quickly. I've seen loops where the end
-expression was actually something like: "``SomeMap[x]->end()``" and map lookups
+expression was actually something like: "``SomeMap[X]->end()``" and map lookups
really aren't cheap. By writing it in the second form consistently, you
eliminate the issue entirely and don't even have to think about it.
Most of the time, you probably have no reason to flush the output stream, so
it's better to use a literal ``'\n'``.
+Don't use ``inline`` when defining a function in a class definition
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+A member function defined in a class definition is implicitly inline, so don't
+put the ``inline`` keyword in this case.
+
+Don't:
+
+.. code-block:: c++
+
+ class Foo {
+ public:
+ inline void bar() {
+ // ...
+ }
+ };
+
+Do:
+
+.. code-block:: c++
+
+ class Foo {
+ public:
+ void bar() {
+ // ...
+ }
+ };
+
Microscopic Details
-------------------
.. code-block:: c++
- if (x) ...
- for (i = 0; i != 100; ++i) ...
- while (llvm_rocks) ...
+ if (X) ...
+ for (I = 0; I != 100; ++I) ...
+ while (LLVMRocks) ...
somefunc(42);
assert(3 != 4 && "laws of math are failing me");
- a = foo(42, 92) + bar(x);
+ A = foo(42, 92) + bar(X);
and this is bad:
.. code-block:: c++
- if(x) ...
- for(i = 0; i != 100; ++i) ...
- while(llvm_rocks) ...
+ if(X) ...
+ for(I = 0; I != 100; ++I) ...
+ while(LLVMRocks) ...
somefunc (42);
assert (3 != 4 && "laws of math are failing me");
- a = foo (42, 92) + bar (x);
+ A = foo (42, 92) + bar (X);
The reason for doing this is not completely arbitrary. This style makes control
flow operators stand out more, and makes expressions flow better. The function
function name (as in the last example) makes it appear that the code might bind
the arguments of the left-hand-side of a binary operator with the argument list
of a function and the name of the right side. More specifically, it is easy to
-misread the "``a``" example as:
+misread the "``A``" example as:
.. code-block:: c++
- a = foo ((42, 92) + bar) (x);
+ A = foo ((42, 92) + bar) (X);
when skimming through the code. By avoiding a space in a function, we avoid
this misinterpretation.
In general, we strive to reduce indentation wherever possible. This is useful
because we want code to `fit into 80 columns`_ without wrapping horribly, but
-also because it makes it easier to understand the code. Namespaces are a funny
-thing: they are often large, and we often desire to put lots of stuff into them
-(so they can be large). Other times they are tiny, because they just hold an
-enum or something similar. In order to balance this, we use different
-approaches for small versus large namespaces.
-
-If a namespace definition is small and *easily* fits on a screen (say, less than
-35 lines of code), then you should indent its body. Here's an example:
-
-.. code-block:: c++
-
- namespace llvm {
- namespace X86 {
- /// RelocationType - An enum for the x86 relocation codes. Note that
- /// the terminology here doesn't follow x86 convention - word means
- /// 32-bit and dword means 64-bit.
- enum RelocationType {
- /// reloc_pcrel_word - PC relative relocation, add the relocated value to
- /// the value already in memory, after we adjust it for where the PC is.
- reloc_pcrel_word = 0,
-
- /// reloc_picrel_word - PIC base relative relocation, add the relocated
- /// value to the value already in memory, after we adjust it for where the
- /// PIC base is.
- reloc_picrel_word = 1,
-
- /// reloc_absolute_word, reloc_absolute_dword - Absolute relocation, just
- /// add the relocated value to the value already in memory.
- reloc_absolute_word = 2,
- reloc_absolute_dword = 3
- };
- }
- }
-
-Since the body is small, indenting adds value because it makes it very clear
-where the namespace starts and ends, and it is easy to take the whole thing in
-in one "gulp" when reading the code. If the blob of code in the namespace is
-larger (as it typically is in a header in the ``llvm`` or ``clang`` namespaces),
-do not indent the code, and add a comment indicating what namespace is being
-closed. For example:
+also because it makes it easier to understand the code. To facilitate this and
+avoid some insanely deep nesting on occasion, don't indent namespaces. If it
+helps readability, feel free to add a comment indicating what namespace is
+being closed by a ``}``. For example:
.. code-block:: c++
namespace llvm {
namespace knowledge {
- /// Grokable - This class represents things that Smith can have an intimate
+ /// This class represents things that Smith can have an intimate
/// understanding of and contains the data associated with it.
class Grokable {
...
} // end namespace knowledge
} // end namespace llvm
-Because the class is large, we don't expect that the reader can easily
-understand the entire concept in a glance, and the end of the file (where the
-namespaces end) may be a long ways away from the place they open. As such,
-indenting the contents of the namespace doesn't add any value, and detracts from
-the readability of the class. In these cases it is best to *not* indent the
-contents of the namespace.
+
+Feel free to skip the closing comment when the namespace being closed is
+obvious for any reason. For example, the outer-most namespace in a header file
+is rarely a source of confusion. But namespaces both anonymous and named in
+source files that are being closed half way through the file probably could use
+clarification.
.. _static:
.. code-block:: c++
namespace {
- class StringSort {
- ...
- public:
- StringSort(...)
- bool operator<(const char *RHS) const;
- };
+ class StringSort {
+ ...
+ public:
+ StringSort(...)
+ bool operator<(const char *RHS) const;
+ };
} // end anonymous namespace
- static void Helper() {
+ static void runHelper() {
...
}
.. code-block:: c++
namespace {
+
class StringSort {
...
public:
bool operator<(const char *RHS) const;
};
- void Helper() {
+ void runHelper() {
...
}
} // end anonymous namespace
-This is bad specifically because if you're looking at "``Helper``" in the middle
+This is bad specifically because if you're looking at "``runHelper``" in the middle
of a large C++ file, that you have no immediate way to tell if it is local to
the file. When it is marked static explicitly, this is immediately obvious.
Also, there is no reason to enclose the definition of "``operator<``" in the
See Also
========
-A lot of these comments and recommendations have been culled for other sources.
+A lot of these comments and recommendations have been culled from other sources.
Two particularly important books for our work are:
#. `Effective C++
projects / oota-llvm.git / blobdiff