-
Notifications
You must be signed in to change notification settings - Fork 21
code style guide
[TOC]
Much of this guide quotes or paraphrases the Python style guide (PEP 8).
Code is read much more often than it is written. The guidelines provided here are intended to improve the readability of code and make it consistent across SLATE, as an important part of SLATE's sustainability.
A style guide is about consistency. Consistency with this style guide is important. Consistency within a project is more important. Consistency within one module or function is the most important.
However, know when to be inconsistent -- sometimes style guide recommendations just aren't applicable. When in doubt, use your best judgment. Look at other examples and decide what looks best. And don't hesitate to ask!
In particular: do not break backwards compatibility just to comply with this style guide!
Some other good reasons to ignore a particular guideline:
-
When applying the guideline would make the code less readable, even for someone who is used to reading code that follows this style guide.
-
To be consistent with surrounding code that also breaks it (maybe for historic reasons) -- although this is also an opportunity to clean up someone else's mess (in true extreme programming style).
-
Because the code in question predates the introduction of the guideline and there is no other reason to be modifying that code.
Recommended order for sections in a function:
-
using
statements - trace block
- constants (with
// Constants
comment) - options (with
// Options
comment) - code
Example:
template <typename scalar_t>
void foo(
Matrix<scalar_t>& A,
Options const& opts )
{
using real_t = blas::real_type<scalar_t>;
using blas::real;
trace::Block trace_block( __func__ );
// Constants
const scalar_t zero = 0.0;
const real_t r_zero = 0.0;
// Options
int64_t lookahead = get_option<int64_t>( opts, Option::Lookahead, 1 );
... code ...
}
-
Enum values are UpperCamelCase:
enum class MatrixType
-
Classes are UpperCamelCase:
class SymmetricMatrix
-
Class member functions are lowerCamelCase:
Matrix::tileGet()
- Some discussion to make these snake_case.
-
Global functions are snake_case:
gesv_mixed()
- Previously these were lowerCamelCase, but that is awkward so we're moving away from it.
-
Local variables are snake_case:
my_awesome_variable
-
Local constants are snake_case:
max_life
-
Global constants (at file level) are UpperCamelCase:
const int HostNum = -1;
-
Namespaces are lowercase:
namespace slate {...}
Namespaces should have short one word names, not two words that would need lowerCamelCase or snake_case. -
Private variables have underscore suffix:
int mt_, nt_;
Note that leading underscores are reserved by C++:_mt
is illegal.
Question: Should private routines be named differently, to clearly distinguish them? I guess not, since C++ will enforce private/public, and we may want to change a private routine to public.
Note: When using acronyms in CamelCase, capitalize all the
letters of the acronym. Thus HTTPServerError is better than
HttpServerError. Hence fromLAPACK
, not fromLapack
.
Headers that an application needs (i.e., that would be included via slate.hh) go in include/slate. Strictly internal headers go in src.
SLATE library (-lslate
) source files go in src. Underneath src, directories
group similar files. Ideally, directories would be based on namespaces, like
src/internal
for the slate::internal
namespace. Currently some directories
differ from that practice.
Never have 2 files
with the same name in different directories, since this causes an issue building
a static library. For instance, do not have src/gemm.cc
and
src/internal/gemm.cc
, which would make two different gemm.o
files to add to
libslate.a
. In this case, repeat the namespace in the filename:
src/gemm.cc // slate::gemm
src/internal/internal_gemm.cc // slate::internal::gemm
Follow common mathematical conventions, often following BLAS and LAPACK names, including:
-
For matrices, use capital
A
,B
,C
, etc. -
For vectors, use lowercase
x
,y
,z
, etc. -
For vector of eigenvalues, use
Lambda
. For vector of singular values, useSigma
. -
For scalars, use lowercase Greek
alpha
,beta
, etc. -
For number of block rows and cols, use
mt
andnt
, respectively. -
For number of individual rows and cols, use
m
andn
, respectively. -
For block row and col loop counters, use
i
andj
, respectively. This differs from PLASMA, which usedm
andn
. -
For 3rd loop counter, use
k
. -
For individual row and col loop counters, use
ii
andjj
, respectively. -
For tile block size, use
mb
(rows) andnb
(cols). -
Use
ib
for inner blocking. -
For truncated tile size, use
ib
andjb
, as in LAPACK. E.g.,ib = min( nb, m - i )
andjb = min( nb, n - j );
. This creates a conflict with inner blocking, but SLATE doesn't need to refer to the truncated tile size very often. -
For iterators, use
iter
andend
. See section on Loops. Do not useit
as that is a common English word, making it difficult to search for iterators (case in point). -
For device index, use
device
.
for (int device = 0; device < num_devices; ++device)
- For lock guards, use
guard
:
LockGuard guard( &lock );
A common error is to forget the variable name: LockGuard( &lock );
acquires and immediately releases the lock!
Declare constants near the top of the function (see Code Structure above). Use these names:
// scalar (real or complex)
const scalar_t zero = 0.0;
const scalar_t half = 0.5;
const scalar_t one = 1.0;
// real
const real_t r_zero = 0.0;
const real_t r_half = 0.5;
const real_t r_one = 1.0;
// integer
// Integer constants are rarely needed; just use 0 and 1.
// The exception is directly calling Fortran, which needs a pointer to a constant.
const int64_t izero = 0;
const int64_t ione = 1;
// machine constants
// eps (2.22e-16) is as defined by C++, C, Python, Fortran 90, and lamch("p"),
// but differs from LAPACK's lamch("e"), which is unit roundoff (1.11e-16).
const real_t safe_min = std::numeric_limits<real_t>::min();
const real_t eps = std::numeric_limits<real_t>::epsilon();
const real_t sml_num = safe_min / eps;
const real_t big_num = 1 / sml_num;
const real_t sqrt_sml = sqrt( sml_num );
const real_t sqrt_big = sqrt( big_num );
Negative values can be expressed using those names, such as -one
, instead of
negative_one
, neg_one
, or worse, mone
.
Yes:
gemm( -one, A, B, zero, C );
Older, deprecated style:
gemm( scalar_t(-1.0), A, B, scalar_t(0.0), C );
For clarity, instead of passing an ambiguous constant like 0 or 1 for an argument, name the constant:
Yes:
const int priority_0 = 0;
const int priority_1 = 1;
internal::gemm( alpha, A, B, beta, C, priority_1 );
No:
internal::gemm( alpha, A, B, beta, C, 1 );
Never use the characters 'l' (lowercase letter el), 'O' (uppercase letter oh), or 'I' (uppercase letter eye) as single character variable names.
Rationale: In some fonts, these characters are indistinguishable from the numerals one and zero. When tempted to use 'l', use 'L' instead.
SLATE largely follows K&R and Stroustrup style.
There is an Astyle script in development to (semi-)automate formatting. See notes on its use.
Use 4 spaces per indentation level
(class, enum, function, if, while, switch, case, etc.).
However, namespaces do not add an indentation level.
Also, public
, protected
, and private
keywords are not indented.
Never use tabs. (Makefiles of course must use tabs for recipes.)
For functions, place the opening brace on a line by itself. This visually separates the parameters from the code inside the function.
Yes:
void longFunctionName(
param1,
param2)
{
code();
}
No:
void longFunctionName(
param1,
param2) {
code();
}
For other blocks, place the opening brace at the end of its statement line ("cuddled"). This saves significant vertical space.
Yes:
namespace slate {
namespace internal {
//------------------------------------------------------------------------------
enum class Values {
A,
B,
};
//==============================================================================
class FooBar {
public:
FooBar( int m, int n )
: number_rows_( m ),
number_cols_( n )
{
code();
}
private:
int num_rows_, num_cols_;
};
//------------------------------------------------------------------------------
void longFunctionName()
{
if (condition) {
code();
}
else if (condition) {
code();
}
else {
code();
}
for (int i = 0; i < n; ++i) {
code();
}
while (condition) {
code();
}
do {
code();
} while (condition);
switch (value) {
case 1:
break;
default:
break;
}
}
} // namespace internal
} // namespace slate
No:
// Don't indent namespaces.
namespace slate {
namespace internal {
class FooBar {
};
}
}
// Don't use more than 4 spaces.
class FooBar {
member;
};
// Don't use less than 4 spaces.
void longFunctionName()
{
if (condition) {
for (...) {
}
}
}
// Don't skip indenting cases inside switch.
switch (value) {
case 1:
code();
break;
default:
code();
break;
}
// Don't cuddle else.
if (condition) {
code();
} else if (condition) {
code();
} else {
code();
}
// Don't uncuddle braces (with some exceptions below).
if (condition)
{
code();
}
else
{
code();
}
The closing brace usually goes on a line by itself. The exception is an empty block, such as an empty constructor or empty loop.
class Foo {
Foo()
{}
};
while (progress.at(sweep-1).load() < depend) {}
Don't make an empty loop using just a semi-colon. Make it obvious that it is empty.
No: while (progress.at(sweep-1).load() < depend);
Braces may be used, but are not required, on short one line if
, for
, and
while
blocks. However, if a single statement spans multiple lines, braces
should be added. If one block in an if-then-else
has braces, put braces on all
its blocks.
Yes:
// Braces not required.
if (condition)
one-line;
else
one-line;
// Braces may be used.
if (condition) {
one-line;
}
else {
one-line;
}
// Braces required for a single multi-line statement.
if (condition) {
longFunctionName(
args,
args);
}
// `else` block requires braces, so use braces for both blocks.
if (condition) {
one-line;
}
else {
code();
code();
}
No:
// Braces required for a multi-line statement.
if (condition)
longFunctionName(
args,
args);
// `else` block requires braces, so use braces for both blocks.
if (condition)
one-line;
else {
code();
code();
}
However, do not duplicate the opening brace if using an #ifdef
. Doing so often
confuses text editors, since they see an unbalanced opening brace. In this
(hopefully rare) case, put the brace on a line by itself (Allman style):
Yes:
#ifdef MACRO
while (condition)
#else
while (other-condition)
#endif
{
code();
}
No:
#ifdef MACRO
while (condition) {
#else
while (other-condition) {
#endif
code();
}
OpenMP pragmas must also have the brace after the pragma:
#pragma omp task
{
code();
}
Put one space between if
, for
, while
, do
, switch
and its opening
parenthesis or brace.
Put one space before an opening brace, except for a function.
No spaces inside the parenthesis.
// ,-----------,----- spaces
// | ,......., | .... no spaces
// V x x v
Yes: if (condition) {
No: if(condition){
No: if( condition ){
// ,-----------------------,----- spaces
// | ,..................., | .... no spaces
// V x x v
Yes: for (int i = 0; i < n; ++i) {
No: for(int i = 0; i < n; ++i){
No: for( int i = 0; i < n; ++i ){
// ,-----------,----- spaces
// | ,......., | .... no spaces
// V x x v
Yes: while (condition) {
No: while(condition){
No: while( condition ){
// ,------------------ space
// V
Yes: do {
code();
} while (condition);
// ^ ^ x x
// | | '.......'.... no spaces
// '-----'-------------- space
No: do{
code();
}while(condition);
- Put one space immediately inside parentheses, brackets or braces.
Yes: spam( ham[ 1 ], { eggs: 2 } )
No: spam(ham[1], {eggs: 2})
This is a change from past style; update code as it is edited.
Motivation: This is not the usual with C/C++ code, but it is the convention in LAPACK code, and makes code easier to read. Note that English has the opposite convention (putting one space before the parenthesis), like so.
long_function_name(long_first_argument(long_inner_argument))
vs.
long_function_name( long_first_argument( long_inner_argument ) )
- Immediately before a comma, semicolon, or colon:
Yes: for (int i = 0; i < n; ++i)
No: for (int i = 0 ; i < n ; ++i)
Yes: function( a, b, c );
No: function( a , b , c );
- Immediately before the open parenthesis that starts the argument list of a function call:
Yes: spam(1)
No: spam (1)
- Immediately before the open brace that starts an indexing:
Yes: mymap["key"] = mylist[index]
No: mymap ["key"] = mylist [index]
-
Excessive space around an assignment (or other) operator to align it with another. Small amounts of extra space (say, ≤ 4 spaces) to align items to improve readability and vertical editing are acceptable; use discretion.
Yes:
x = 1;
y = 2;
long_variable = 3;
// One extra space to align = signs.
m = 1;
n = 2;
nb = 40;
No:
x = 1;
y = 2;
extra_long_variable = 3;
-
Always surround these binary and ternary operators with a single space on either side, and a single space after unary
!
:- assignment (
=
) - augmented assignment (
+=
,-=
, etc.) - comparison operators (
==
,<
,>
,!=
,<=
,>=
) - boolean operators (
&&
,||
,!
) - ternary operator (
? :
)
Examples:
- assignment (
Yes: i = j;
No: i=j;
Yes: if (i < n)
No: if (i<n)
Yes: if (i == j && diag == Unit)
No: if (i==j&&diag==Unit)
Yes: if (! row_major)
No: if (!row_major)
Yes: condition ? value-if-true : value-if-false
No: condition?value-if-true:value-if-false
Question: for default arguments, space or no space?
function( argument=default );
function( argument = default );
template <typename type=default>
template <typename type = default>
-
If operators with different priorities are used, consider adding whitespace around the lowest priority operators. Use your own judgment; however, never use more than one space, and always have the same amount of whitespace on both sides of a binary operator.
Yes:
x = x*2 - 1
hypot2 = x*x + y*y
c = (a + b)*(a - b)
Rather not:
x = x*2-1
x = x * 2 - 1
hypot2 = x * x + y * y
c = (a + b) * (a - b)
Definitely not:
x = x *2 - 1
x = x*2 -1
For pointer and reference types, align the * and & to the left, attached to the type.
yes: void foo( int* x, int& y );
no: void foo( int *x, int &y );
Don't declare multiple pointers in one line:
yes: int* x;
int* y;
int* z;
no: int* x, y, z; // x is pointer; y and z are int
no: int *x, *y, *z; // all are pointers
Soft limit goal is 80 characters. Hard limit is 85 characters.
Rationale: Limiting the required editor window width makes it possible to have several files open side-by-side, and works well when using code review tools that present the two versions side-by-side. Having a soft limit avoids awkward wrapping just because 1-2 characters exceed the soft limit, and avoids the need to rewrap code that did fit into 80 characters, but was later indented one more level. Don't be pedantic about it, but don't stretch a line to 100 characters, either!
For statements that continue onto multiple lines, align lines either:
- Vertically after open parenthesis, or
- Using hanging indent, with no arguments on the first line. Add an extra level of indentation if needed to clearly distinguish it as a continuation line. Question: add 2 levels?
Yes:
// 1. Align with opening parenthesis.
foo = long_function_name( var_one, var_two,
var_three, var_four );
// 2. Hanging indent adds a level (4 spaces).
foo = long_function_name(
var_one, var_two,
var_three, var_four);
// Question: add 2 levels (8 spaces)?
foo = long_function_name(
var_one, var_two,
var_three, var_four);
No:
// Arguments on first line forbidden when not using vertical alignment.
foo = long_function_name( var_one, var_two,
var_three, var_four );
For OpenMP, put slate_omp_default_none
on the #pragma omp task
line,
and use a hanging indent with clauses indented 1 level (4 spaces):
#pragma omp task slate_omp_default_none \
depend( in:col[ k-1 ] ) \
depend( inout:col[ k+lookahead ] ) \
depend( inout:row[ k+1+lookahead ] ) \
firstprivate( tag )
(Previously aligned depend clauses after task
, but that pushes
firstprivate
really far to the right.)
Similarly, parallel for
uses 1 level hanging indent:
#pragma omp parallel for \
num_threads( thread_size ) \
shared( reflectors, lock, progress )
When an if
condition extends across multiple lines, the
if (
creates a natural 4-space indent for the
subsequent lines of the multi-line conditional, which can produce a visual
conflict with the code inside the if
-statement,
which is also indented by 4 spaces.
Question: Should SLATE take a position on this? (PEP 8 does not.)
Acceptable options in this situation include, but are not limited to:
// No extra indentation.
if (this_is_one_thing
&& that_is_another_thing) {
do_something();
}
// Add a blank line.
if (this_is_one_thing
&& that_is_another_thing) {
do_something();
}
// Move opening brace to next line.
if (this_is_one_thing
&& that_is_another_thing)
{
do_something();
}
// Add a comment, which will provide some distinction in editors
// supporting syntax highlighting.
if (this_is_one_thing
&& that_is_another_thing) {
// Since both conditions are true, we can frobnicate.
do_something();
}
// Add some extra indentation on the conditional continuation line.
if (this_is_one_thing
&& that_is_another_thing) {
do_something();
}
(Also see the discussion below of breaking before binary operators.)
In class constructors, the colon for initializing member variables is indented 4 spaces; the actual member variables are thus indented 6 spaces. This is to avoid the visual conflict between parameters (with hanging indent) and member initializers. Initializers are listed one per line.
Yes:
MyClass::MyClass(
param1,
param2)
: member1_(),
member2_()
{}
No:
MyClass::MyClass(
param1,
param2)
: member1_(), member2_()
{}
MyClass::MyClass(
param1,
param2)
: member1_(),
member2_()
{}
MyClass::MyClass(
param1,
param2):
member1_(),
member2_()
{}
The closing brace/bracket/parenthesis on multi-line constructs may be lined up under the first character of the line that starts the multi-line construct, as in:
int my_list[] = {
1, 2, 3,
4, 5, 6
};
Consistent with math publishing convention, break continued lines before binary and ternary operators. Where applicable, align operator under assignment = sign. Short ternary operators may be on one line.
Yes: easy to match operators with operands
income = gross_wages
+ taxable_interest
+ (dividends - qualified_dividends)
- ira_deduction
- student_loan_interest;
No: operators sit far away from their operands
income = gross_wages +
taxable_interest +
(dividends - qualified_dividends) -
ira_deduction -
student_loan_interest
Yes: Lines beginning with &&
or ||
signal the condition
continues. However, this sometimes ruins alignment of the
long_conditions
--I'm conflicted.
if (some_long_condition
&& another_long_condition
&& final_long_condition) {
code();
}
No:
if (some_long_condition &&
another_long_condition &&
final_long_condition) {
code();
}
Yes: ?
and :
should always be on the same line as their value.
// ternary operator
result = condition ? value-if-true : value-if-false;
result = long-condition
? long-value-if-true
: long-value-if-false;
No:
result = condition ?
value-if-true :
value-if-false;
Rationale: See PEP008. This is the convention followed in mathematics and advocated by Knuth. It highlights the operators, makes them all line up, and is clearer what is being operated on.
This is a change from PLASMA and SLATE's original style sheet, which placed boolean operators (&& and ||) at the end of lines.
Question: what about lining up clauses for readability:
// new style
if (A.op() == Op::NoTrans
|| A.op() == Op::Trans)
vs.
// new style + extra spaces
if ( A.op() == Op::NoTrans
|| A.op() == Op::Trans)
vs.
// old style
if (A.op() == Op::NoTrans ||
A.op() == Op::Trans)
Question: should ternary operators be enclosed in parenthesis?
result = (condition ? value-if-true : value-if-false);
result = (condition
? value-if-true
: value-if-false);
Multiple statements on the same line are generally discouraged.
Yes:
if (foo == "blah")
do_blah_thing();
do_one();
do_two();
do_three();
Rather not:
if (foo == "blah") do_blah_thing();
do_one(); do_two(); do_three();
Definitely not:
if (foo == "blah") do_blah_thing();
else do_non_blah_thing();
try { something(); }
catch (Exception const& ex) { cleanup(); }
do_one(); do_two(); do_three( long, argument,
list, like, this );
if (foo == "blah") { one(); two(); three(); }
Surround function definitions and classes by a single blank line. Blank lines may be omitted between a bunch of related one-liners (although probably not if properly documented with Doxygen).
Use blank lines in functions, sparingly, to indicate logical sections. Think of this like paragraphs in a paper.
Yes:
//==============================================================================
class Foo {
public:
Foo()
{}
Foo( param )
{
code();
}
// No blanks needed between prototypes, only between definitions.
void member1();
void member2();
void member3();
// May skip blank lines for one-liners.
int m() const { return m_; }
int n() const { return n_; }
int mt() const { return mt_; }
int nt() const { return nt_; }
};
//------------------------------------------------------------------------------
void function1()
{
code();
}
//------------------------------------------------------------------------------
void function2()
{
code();
}
Never include trailing whitespace.
Rationale: Changes in trailing whitespace needlessly complicate diff and merges. Some editors can be set to automatically delete trailing whitespace on save.
- In BBEdit, Preferences > Text Files > Strip trailing whitespace.
- In Vim, set in ~/.vimrc:
autocmd BufWritePre * :%s/\s\+$//e
See https://vim.fandom.com/wiki/Remove_unwanted_spaces
Files should end with exactly one newline.
Rationale: Omitting the newline complicates diff, merges, grep, and some editors will complain. Having multiple newlines causes needless changes, and breaks some compilers (PGI).
Trailing commas, on the last value, are okay and encouraged in enum lists (since C++11). Including a trailing comma helps to avoid merge conflicts, where 2 different changes both add a new item and thus both add a comma to the previous line.
Yes:
enum class Foo {
A,
B,
C,
};
The same rationale is used for line continuation (\
) in Makefile lists.
A comment as the line after the list is recommended; otherwise a blank
line after the list is required by make.
src = \
file1.cc \
file2.cc \
# End. Add alphabetically.
next = ...
Comments that contradict the code are worse than no comments. Always make a priority of keeping the comments up-to-date when the code changes!
Use C++ style //
comments.
For Doxygen, use ///
comments.
For explanatory comments, always put one space after //
or ///
.
For commenting out code, a space is not required.
Comment your code heavily, so a reader later can understand what you did, but do not state the obvious.
If a comment is a sentence, start with a capital letter (unless it is an identifier -- never alter the case of identifiers!), and end with a period. Use one space between sentences. Consider using backticks `...` around identifiers.
If a comment is not a sentence, start with a small letter and do not end with a period.
Write all comments in English.
Yes:
//----------------------------------------
/// Solves a complex problem.
/// Leave blank `///` line before each @param and before function.
///
/// @param[in] n
/// Array size.
///
/// @param[out] result
/// Output data.
///
/// @param[in,out] data
/// Input and output data. Doxygen uses "in,out", not "inout".
///
void function( int n, int* result, int* data )
{
// Do some work.
code();
// block size
int nb = 32;
// No space required to comment out code.
//int mb = 64;
}
//----------------------------------------
/// Small functions, e.g., inside classes, can have compact documentation.
/// @param[in] i: array index. Use colon after variable name.
value_type at( int64_t i ) { return data_[ i ]; }
No:
//Solves a complex problem. # Missing initial space.
// solves a complex problem. # Missing initial capital.
// Solves a complex problem # Missing final period.
Comment the function definition, not the function prototype. Duplicated comments diverge.
Start each file with the copyright boilerplate.
Use A^H
and A^T
rather than Matlab A'
notation. The explicit
conjugate-transpose or transpose is more explicit, easier to see, and will
format nicely in Doxygen.
In Doxygen, use Latex-style markup: $...$
and \[...\]
for equations.
Block comments generally apply to some (or all) code that follows
them, and are indented to the same level as that code. Each line of a
block comment starts with //
or ///
, and a single space (unless it is
indented text inside the comment).
Paragraphs inside a block comment are separated by a line containing only //
or ///
. Do not indent paragraphs.
Yes:
/// Computes an LU factorization of a general m-by-n matrix $A$
/// using partial pivoting with row interchanges.
///
/// The factorization has the form
/// \[
/// A = P L U
/// \]
/// where $P$ is a permutation matrix, $L$ is lower triangular with unit
/// diagonal elements (lower trapezoidal if m > n), and $U$ is upper
/// triangular (upper trapezoidal if m < n).
An inline comment is a comment on the same line as a statement.
Use inline comments sparingly. They make it harder to fit within 80 characters,
and can lead to lines changing when only the comment changed, not the code.
Inline comments should be separated by at least two spaces from the
statement. They should start with //
and a single space.
If they state the obvious, inline comments are unnecessary and in fact distracting. Don't do this:
x = x + 1; // Increment x.
But sometimes, this is useful:
x = x + 1; // Compensate for border.
Inline comments using Doxygen's ///<
can be useful to document enum values
and member variables:
int64_t mt_; ///< Number of block rows in this view.
int64_t nt_; ///< Number of block cols in this view.
In Makefiles, do not use inline comments. They cause problems because they add trailing whitespace to variables.
Yes:
# comment
src = foo.cc
No. Here the string $(src) = "foo.cc "
, including the 2 spaces before the
comment:
src = foo.cc # comment
Use horizontal rule lines consisting of dashes or equal signs to separate
major sections of code. Note these use //
, not Doxygen's ///
, which would
add extra rule lines to Doxygen's output.
Before a class, use an 80 character line of //
and equals:
//==============================================================================
/// Class documentation.
///
class Foo {
};
Before a function definition or enum, use an 80 character line of //
and
dashes:
//------------------------------------------------------------------------------
/// Function documentation.
///
void function() {
}
Inside a function, major sections can be marked by lines such as these, which have 40, 20, and 10 dashes, respectively:
//----------------------------------------
// Section.
//--------------------
// Sub-section.
//----------
// Sub-sub-section.
Rationale: Instead of fixed length rules, as above, some prefer making the rule line fit the length of the comment text. However, this requires changing the rule line whenever the comment changes, and prevents a hierarchy of sections.
Testers use 3 boilerplate comments to quickly identify where the actual routine calls are, amidst all the matrix setup:
//==================================================
// Run SLATE test.
//==================================================
//==================================================
// Check results.
//==================================================
//==================================================
// Run ScaLAPACK reference routine.
//==================================================
Declare loop variables in the loop itself.
for (int i = 0; i < n; ++i) {
...
}
auto end = container.end();
for (auto iter = container.begin(); iter != end; ++iter) {
int value = *iter;
}
Prefer pre-increment to post-increment.
Yes: for (int i = 0; i < n; ++i)
No: for (int i = 0; i < n; i++)
Rationale:
For int, these are essentially equivalent, but for iterators, i++
creates an
extra temporary variable to return, thus being inherently less inefficient,
while ++i
doesn't create a temporary.
Question: preferred spacing for range loop colon?
Where applicable, use a C++11 range loop (note spaces before and after : operator).
for (int value : container) {
...
}
For an infinite loop, use while (true)
.
Do not use for ( ; ; )
, which is a weird C++ idiom.
while (true) {
...
if (condition)
break;
...
}
Catch exceptions as const reference, and name it ex
:
try {
...
}
catch (Exception const& ex) {
...
}
Additionally, for all try/except clauses, limit the try
clause
to the absolute minimum amount of code necessary. This
avoids masking bugs.
Stuff to write up.
Generally SLATE requires C++11, plus tuple fix { ... } that may require C++17.
Self-contained headers
#define guards
C libraries have extern guards
Order of includes:
- SLATE headers
- standard C++ headers
- standard C headers
- other library headers
Rationale: This ensures that SLATE headers are including everything they need, and not accidentally getting it from a previously included C++ header. It is the order advocated by Google [verify].
All code is in slate
namespace
Avoid global variables
Data members private, unless static const
Use auto
for long type names
Rule of 3 or 5 Either need all 3: destructor, copy constructor, assignment, or none of them. Generally need all 3 when memory is being allocated with new. For C++11, rule of 5: destructor, copy constructor, move constructor, assignment, move assignment. There are some variations with the swap routine, copy-and-swap idiom.
Do not use volatile
without reading
Item 4: Use std::atomic
for concurrency, volatile
for special memory
in Scott Meyer's book Effective Modern C++. You probably want to use atomic
.
Avoid dead code. In general, do not leave dead code commented out. Doing so raises the question of what that code is doing there. Instead, simply delete it. The original code is maintained in hg/git if ever needed again. This also reduces the number of commits and work to cleanup codes later on.
The only time to leave dead code in is if it is a feature that isn't yet implemented, or isn't fully working. For instance, pivoting to the left in LU during the factorization. Then the experimental code can be left commented out, with a comment explaining why it is commented out.
Don't compare boolean values to true or false using ==.
Yes: if (row_major)
No: if (row_major == true)
Yes: if (! row_major)
No: if (row_major == false)
Many style guides, and some auto formatting tools like clang-format, insist that
preprocessor directives #define
, #if
, #else
, #endif
occur without any
indentation. There is little sense in this. Without indent, the preprocessor
directives disrupt the visual flow of the code, making it difficult to tell what
is inside a loop or if
block. The code is much easier to read if the
preprocessor directives are indented along with the code, and add their own
level of indentation. Thankfully, SLATE has fewer #ifdef
than previous
libraries such as PLASMA and MAGMA, due to templating.
Yes:
#ifndef BLAS_FORTRAN_NAME
#if defined( FORTRAN_UPPER )
#define BLAS_FORTRAN_NAME( lower, UPPER ) UPPER
#elif defined( FORTRAN_LOWER )
#define BLAS_FORTRAN_NAME( lower, UPPER ) lower
#else
#define BLAS_FORTRAN_NAME( lower, UPPER ) lower ## _
#endif
#endif
if (k > 1) {
#pragma omp task
{
#if CONDITION
slate::internal::gemmW( ... );
#else
slate::internal::gemmA( ... );
#endif
}
}
Rather not: If the code inside the #if
is multiple lines, it is hard to find
where the #if
ends:
if (k > 1) {
#pragma omp task
{
#if CONDITION
slate::internal::gemmW( ... );
#else
slate::internal::gemmA( ... );
#endif
}
}
No:
#ifndef BLAS_FORTRAN_NAME
#if defined( FORTRAN_UPPER )
#define BLAS_FORTRAN_NAME( lower, UPPER ) UPPER
#elif defined( FORTRAN_LOWER )
#define BLAS_FORTRAN_NAME( lower, UPPER ) lower
#else
#define BLAS_FORTRAN_NAME( lower, UPPER ) lower ## _
#endif
#endif
if (k > 1) {
#pragma omp task
{
#if CONDITION
slate::internal::gemmW( ... );
#else
slate::internal::gemmA( ... );
#endif
}
}
Use Latex-style in comments E.g., use ^T and ^H instead of Matlab-style ' (which is ambiguous and easy to miss). Don't use *, just use juxtaposition for multiplication. However, no need to backslash everything -- it's not actually typesetting Latex.
w = Av
A = A - tau v w^H - conj( tau ) w v^H