-
-
Notifications
You must be signed in to change notification settings - Fork 367
Coding Style and Idioms
We generally follow the Google Style Guide except for the exceptions listed in the following sections. We use a few tools to automate style and formatting checking, but they don't completely capture our formatting and style guidelines.
cpplint.py
is a Python (2.7) script that checks for these guidelines. Here is a make target that will check the source code for any instances that do not conform to this standard:
> make cpplint
If your default python version is python 3, the cpplint.py script fails silently. Once you install python 2.x, you can add this to your make/local
file to use python 2.x to run the python script:
RUN_CPPLINT = python2 stan/lib/cpplint_4.45/cpplint.py
The make target includes Stan-specific options and limits the tests to the src/stan directory.
clang-format is a tool that will automatically format C++ code according to formatting rules in the .clang-format
file. Our file is just a few changes from the vanilla Google Style guide. clang-format
doesn't deal with many of our exceptions to the Style guide, below, as they are difficult to automate or not strictly formatting related.
First, install clang-format
. If you're on a mac and using homebrew, please remove any clang-formats you have installed and then install clang-format@2017-11-14 with this command:
brew install https://raw.githubusercontent.com/Homebrew/homebrew-core/5d29e8a6bfbe2e7d8f9525645bb0c00879d0984a/Formula/clang-format.rb
(if that doesn't work, try brew install clang-format
or brew upgrade clang-format
and get whatever your computer thinks is the latest)
If you're on Ubuntu Linux, you can use if you're on Ubuntu you can use the LLVM PPA to install clang-format-5.0 and then use update-alternatives to set it as the default. Find the relevant 5.0 PPA for your version of Ubuntu here: https://apt.llvm.org/ and follow their PPA install instructions, then:
sudo apt-get install clang-format-5.0
sudo update-alternatives --install /usr/bin/clang-format clang-format /usr/bin/clang-format-5.0 100
For other distributions, try installing from source: https://github.com/llvm-mirror/clang/commits/release_50
Emacs - there's a plugin/script called google-c-style
. You can find it here or just install from MELPA.
Spacemacs - you can add google-c-style
to dotspacemacs-additional-packages
.
Vim- check out https://github.com/google/vim-codefmt
There is a pre-commit hook in hooks/pre-commit, and if you run bash hooks/install_hooks.sh
it will be installed for you. Going forward, it will run clang-format -i
(which implicitly uses the .clang-format
file in the repo) on any changed files to format them. If you need to disable this for some reason (though your tests will fail if you haven't formatted things correctly) you can use git commit --no-verify
.
To see which rules are different, you can run this command:
clang-format --style=google -dump-config > google
diff google .clang-format
And here's the current output:
15,16c15,16
< AllowShortIfStatementsOnASingleLine: true
< AllowShortLoopsOnASingleLine: true
---
> AllowShortIfStatementsOnASingleLine: false
> AllowShortLoopsOnASingleLine: false
36c36
< BreakBeforeBinaryOperators: None
---
> BreakBeforeBinaryOperators: All
86c86
< SortIncludes: true
---
> SortIncludes: false
You can see the rule definitions here.
Include what you use testing.
Easy installation requires Homebrew.
Steps:
brew tap jasonmp85/iwyu
brew install iwyu
More details: https://github.com/jasonmp85/homebrew-iwyu
Once iwyu has been installed:
make -k test/test-models/stanc CC=iwyu &> iwyu.out
Inspect iwyu.out
.
More info:
-
-k
flag continues running make in the presence of errors. iwyu always returns an error code, so omitting that will stop at the first compilation. -
CC=iwyu
just replaces the compiler with iwyu. iwyu doesn't support the full set of compiler flags, so it'll put up warnings about some of our compiler flags. -
&> iwyu.out
just grabs the output in both the stdout and stderr and puts it into iwyu.out.
We want to institute some new policies on code, documentation, and testing quality.
-
Every
stan::math
function is part of our API and needs doxygen documentation indicating- what its arguments are and whether they are input or output or both (
@param[in]
,@param[out]
, or@param[in,out]
tags) - what it's template parameters are (
@tparam
tag) - any constraints on the values of the arguments and an indication of what happens (typically exceptions) if the constraints are violated
- which exceptions are thrown
- what the return value is (
@return
tag)
- what its arguments are and whether they are input or output or both (
-
Reused code needs to be broken out into reusable functions.
-
Every
stan::math
function needs tests evaluating all the of constraints and conditions indicated by (1) as well as indicating that the right value is being computed -
Variables should have informative names (within reason).
-
Keep dependencies clean. In particular, we want to avoid
- anything can depend on traits (but it should only depend on double traits if you use no agrad)
- mcmc can depend on anything other than io (?)
- memory shouldn't depend on anything (?)
-
io
should only depend on the data types it needs and test functions (matrix and math tests only?) -
lang
shouldn't depend on anything else
-
Absolutely minimal includes (include what you use).
-
Tests should be parallel in paths to the source files (
src/stan/**
andtest/unit/**
). -
Use the nested namespace
internal
for hidden implementation details for developers. Theinternal
code has similar quality requirements but doesn't have the same completeness requirements for docs and testing that API functions have.
We know there are many bugs in our code. With good test coverage, when a bug is found, these tests allow us to demonstrate:
- That the bug exists,
- that the bug has been fixed,
- and, that the fix has won't break something else.
While we can't prove that the fix hasn't introduced new, unknown bugs, we can prove that this fix won't cause something else to break.
To show that the bug exists, write a test that tickles the bug. Before going on to fix the bug, run the test, verify that it causes failure, and add and commit the test to the Stan repo.
Now fix the bug. To verify that the bug has been fixed, re-run the test and verify that it no longer fails.
Verify that the fix hasn't broken existing code. To do this, we re-run existing tests. Before submitting a pull request, we expect the developer to run those tests which directly exercise the code. At a minimum, if you fix a bug in stan/foo/bar.hpp
, then you should run all tests for src/stan/foo
.
When a pull request is submitted, the entire Stan test suite will be run on our Continuous Integration server.
Stan tests code live in the directory src/test
in the Stan repository.
Stan currently uses the Unix make
utility, specifically GNU Make 3.8, to build and test Stan. See Testing Stan using GNU Make and Python for details. You can run make help
to see some of the options.
We'll try to describe some of the implementation details of the tests for Stan.
-
Test naming convention
All tests are named
*_test.cpp
and are located in subdirectories ofsrc/test
.The test executable for
src/test/*_test.cpp
will be created attest/*
(add.exe
for Windows). The rule is to strip the leadingsrc/
and the_test.cpp
from the filename. -
How to run a specific test
The
make
target is test executable. For example, if the test is located atsrc/test/foo/bar_test.cpp
, the command to build the test is> make test/foo/bar
.
- How to add new tests
- How the autogenerated tests work
- What tests to add for a new file?
- To what extent should methods be tested?
- Examples