Commit 6e1b1355 authored by scott snyder's avatar scott snyder
Browse files

** Version 0.4

  - Minor updates: we're now using c++14.  Add note about implicit fallthrough
   warnings with gcc7.  Add rule to use std::abs().
parent c3a10de4
#+MACRO: version 0.2
#+LaTeX_HEADER: \def\rulesversion{0.2}
#+MACRO: version 0.4
#+LaTeX_HEADER: \def\rulesversion{0.4}
#+TITLE: ATLAS C++ coding guidelines, version {{{version}}}
#+AUTHOR: Shaun Roe (CERN), Scott Snyder (BNL), and the former ATLAS Quality Control group
#+EMAIL: Correspondence to snyder@bnl.gov.
......@@ -472,6 +472,20 @@ default:
this must be explicitly stated in a comment. This should,
however, be a rare case.
#+BEGIN_EXAMPLE
switch (case) {
case 1:
doSomething();
/* FALLTHROUGH */
case 2:
doSomethingMore();
break;
...
#+END_EXAMPLE
gcc7 will warn about such constructs unless you use a comment like
in the example above. (C++17 will add a =fallthrough= attribute.)
- *An if-statement which does not fit in one line must have braces
around the conditional statement.* [<<if-bracing>>]
......@@ -881,8 +895,8 @@ public:
};
#+END_EXAMPLE
This syntax is new in C++11. Largely the same effect can be had in C++98
by declaring the deleted methods as private (and not implementing them):
This syntax was new in C++11. In C++98, this was achieved
by declaring the deleted methods as private (and not implementing them).
#+BEGIN_EXAMPLE
// There is only one ATLASExperimentalHall,
......@@ -1186,7 +1200,7 @@ void foo (std::unique_ptr<Object> obj);
...
auto obj = CxxUtils::make_unique<Object>();
auto obj = std::make_unique<Object>();
...
foo (std::move (obj);
#+END_EXAMPLE
......@@ -1205,8 +1219,8 @@ void foo (std::unique_ptr<Object> obj);
There is basically no good case for passing =unique_ptr= as a const reference.
If you need to interoperate with existing code, or if
C++11 features cannot be used, object ownership may be passed by pointer.
If you need to interoperate with existing code, object ownership
may be passed by pointer.
The fact that ownership is transferred should be clearly documented.
_Do not_ pass ownership using references.
......@@ -1269,7 +1283,7 @@ std::vector<int> getVector()
If a function is returning a pointer to something that is allocated off the heap
which the caller is responsible for deleting, then return a =unique_ptr=.
If C++11 cannot be used, or if compatibility with existing code is an issue,
If compatibility with existing code is an issue,
then a plain pointer may be used, but the caller takes ownership should
be clearly documented.
......@@ -1414,29 +1428,20 @@ ostream& operator<<(ostream& out, const String& s);
=new= and registered in StoreGate is under
control of StoreGate and must not be deleted.
In C++11, you are encouraged to use =unique_ptr= for this.
In new code, you should generally use =make_unique= for this.
#+BEGIN_EXAMPLE
#include <memory>
...
DataVector<C>* dv = ...;
std::unique_ptr<C> c = new C("argument");
auto c = std::make_unique<C>("argument");
...
if (test) {
dv->push_back (std::move (c));
}
#+END_EXAMPLE
With C++14, prefer using =make_unique=:
#+BEGIN_EXAMPLE
auto c = std::make_unique<C>("argument");
#+END_EXAMPLE
Until C++14 mode becomes the default, there is an implementation
of =make_unique= in CxxUtils that may be used.
=auto_ptr= was an attempt to do something similar to =unique_ptr= in older
versions of the language. However, it has some serious deficiencies
and should not be used in new code.
......@@ -1587,8 +1592,7 @@ private:
This is just for clarity of code. The compiler will know it is
virtual, but the human reader may not. This, of course, also includes
the destructor, as stated in item [[[virtual-destructor]]].
With C++11, virtual functions in derived classes which override
Virtual functions in derived classes which override
methods from the base class should also be declared with the =override= keyword.
If the signature of the method is changed in the base class, so that
the declaration in the derived class is no longer overriding it,
......@@ -1632,6 +1636,39 @@ public:
Other possible exceptions include very tightly coupled classes and unit tests.
** Notes on the use of library functions.
- *Use =std::abs= to calculate an absolute value.* [<<std-abs>>]
The return type of =std::abs= will conform to the argument type; other variants
of =abs= may not do this.
In particular, beware of this:
#+BEGIN_EXAMPLE
#include <cstdlib>
float foo (float x)
{
return abs(x);
}
#+END_EXAMPLE
which will truncate =x= to an integer. (Clang will warn about this.)
Conversely, in this example:
#+BEGIN_EXAMPLE
#include <cmath>
float int (int x)
{
return fabs(x);
}
#+END_EXAMPLE
the argument will first be converted to a float, then the result converted
back to an integer.
Using =std::abs= uniformly should do the right thing in almost all cases
and avoid such surprises.
** Assertions and error conditions
- *Pre-conditions and post-conditions should be checked for validity.* [<<pre-post-conditions>>]
......@@ -1887,8 +1924,8 @@ enum State {halted, starting, running, paused};
- *Do not use NULL to indicate a null pointer; use the nullptr keyword instead.* [<<nullptr>>]
=nullptr= is new in C++11. If your code must compile with older
versions, use the constant 0 instead.
Older code often used the constant =0=. =NULL= is appropriate
for C, but not C++.
- *Do not use const char** *or built-in arrays ``[]''; use* =std::string= *instead.* [<<use-std-string>>]
......@@ -1982,6 +2019,8 @@ some machines, left-to-right on others. -- end note ]
- *Do not use asm (the assembler macro facility of C++).* [<<no-asm>>]
Many special-purpose instructions are available through the use
of compiler intrinsic functions.
For those rare use cases where an =asm= might be needed, the use
of the =asm= should be encapsulated and made available in a low-level
package (such as CxxUtils).
......@@ -2129,14 +2168,12 @@ float ip_cut = 0.1 * Gaudi::Units::cm;
** Portability
- *All code must comply with the 2011 version of the ISO C++ standard (C++11)*. [<<standard-cxx>>]
- *All code must comply with the 2014 version of the ISO C++ standard (C++14)*. [<<standard-cxx>>]
A draft of the standard which is essentially identical to the final version
may be found at [fn:cxx].
Some code may need to remain compatible with C++98 for a limited time.
At some point, compatibility with C++14 will also be required.
At some point, compatibility with C++17 will also be required.
- *Make non-portable code easy to find and replace.* [<<limit-non-portable-code>>]
......@@ -2570,8 +2607,11 @@ The comment includes the fact that it is the perpendicular distance.
* Changes
** Version 0.3
** Version 0.4
- Minor updates: we're now using c++14. Add note about implicit fallthrough
warnings with gcc7. Add rule to use std::abs().
** Version 0.3
- Add recommendation to avoid bit fields.
......@@ -2593,7 +2633,7 @@ The comment includes the fact that it is the perpendicular distance.
[fn:gof] E. Gamma, R. Helm, R. Johnson, and J. Vlissides, /Design Patterns, Elements of Reusable Object-Oriented Software/, Addison-Wesley.
[fn:sutter02] [[http://www.gotw.ca/publications/mill22.htm][H. Sutter, /C++ Users Journal/, *20* (7), 2002.]]
[fn:krzemienski14] [[http://akrzemi1.wordpress.com/2014/04/24/noexcept-what-for/][A. Krzemie\nacute{}ski, /noexcept --- what for?/, 2014.]]
[fn:cxx] [[http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3337.pdf][ /Standard for the Programming Language C++/, ISO/IEC 14882-2011.]]
[fn:cxx] [[https://github.com/cplusplus/draft/blob/master/papers/n4140.pdf][ /Standard for the Programming Language C++/, n4140.]]
[fn:warnings] [[https://twiki.cern.ch/twiki/bin/view/AtlasComputing/FaqCompileTimeWarnings][FaqCompileTimeWarnings ATLAS wiki page.]]
[fn:doxygen] [[https://twiki.cern.ch/twiki/bin/view/AtlasComputing/DoxygenDocumentation][DoxygenDocumentation ATLAS wiki page.]]
......@@ -2676,10 +2716,20 @@ Doxygen also knows about //!< , I think?
# LocalWords: MyException auxid LaTeX usepackage fancyvrb endnotes
# LocalWords: doSomethingWithFoo getObject ExcNotFound note1 0pt Ok
# LocalWords: newpage begingroup parindent parskip 2ex enotesize Ok
# LocalWords: normalsize theendnotes endgroup eval setq html pst
# LocalWords: normalsize theendnotes endgroup eval setq html pst Ok
# LocalWords: knuth84 meyers1 meyers2 gof b's sutter02 else's href
# LocalWords: krzemienski14 hrefsave nolinkurl Vlissides Sutter lxr
# LocalWords: Krzemie nacute x144 RecustomVerbatimEnvironment rfoot
# LocalWords: endnote makeenmark theenmark notesname enoteformat Ok
# LocalWords: enoteformatsave makeatletter thefnmark makeatother
# LocalWords: lineno linenumbers fancyhdr pagestyle MYHEADER 42ul
# LocalWords: doSomething doSomethingElse caloCellContainer ufoo
# LocalWords: ret b''s convertAndBuffer buf fbuf updateTrack 0pt
# LocalWords: interoperate getVector makeFoo decrement 2ex bwd BNL
# LocalWords: FaqCompileTimeWarnings DoxygenDocumentation mylabel
# LocalWords: rulesversion FALLTHROUGH doSomethingMore gcc7 cstdlib
# LocalWords: fallthrough fabs bitfields multithreaded atan2 n4140
# LocalWords: Zach OpenLab
# Local Variables:
......@@ -2692,10 +2742,3 @@ Doxygen also knows about //!< , I think?
# eval: (defun org-html-target (target contents info) (let ((id (org-export-solidify-link-text (org-element-property :value target)))) (org-html--anchor id id)))
# eval: (setq org-export-with-email t)
# End:
# LocalWords: endnote makeenmark theenmark notesname enoteformat
# LocalWords: enoteformatsave makeatletter thefnmark makeatother
# LocalWords: lineno linenumbers fancyhdr pagestyle MYHEADER 42ul
# LocalWords: doSomething doSomethingElse caloCellContainer ufoo
# LocalWords: ret b''s convertAndBuffer buf fbuf updateTrack 0pt
# LocalWords: interoperate getVector makeFoo decrement 2ex bwd
# LocalWords: FaqCompileTimeWarnings DoxygenDocumentation
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment