Skip to content

adds macro for making bounds checks a no-op #2423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SteveBronder merged 14 commits into from
Mar 19, 2021
Merged

adds macro for making bounds checks a no-op #2423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
4f92e24
adds macro for making bounds checks a no-op
SteveBronder Mar 12, 2021
9181261
Changes from NDEBUG to STAN_NDEBUG
SteveBronder Mar 13, 2021
6448dff
update docs
SteveBronder Mar 13, 2021
77f6847
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Mar 13, 2021
c97e425
fix include logic for STAN_NO_RANGE_AND_SIZE_CHECK and move compiler …
SteveBronder Mar 13, 2021
d1c23b7
fix include logic for STAN_NO_RANGE_AND_SIZE_CHECK and move compiler …
SteveBronder Mar 13, 2021
be701cb
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Mar 13, 2021
3e5fdba
update macro docs
SteveBronder Mar 13, 2021
a85324c
better logic for defining stan cold path macro
SteveBronder Mar 13, 2021
b9fe223
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Mar 13, 2021
43f4d9a
remove size checks, add tests and github ci for the no range checks
SteveBronder Mar 18, 2021
8179e87
use runtest not make in github ci
SteveBronder Mar 18, 2021
513b5fb
Merge commit 'd2b3af9d2fcc919700e1c8a28a0d5e7293eacc31' into HEAD
yashikno Mar 18, 2021
2f2fe23
[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…
stan-buildbot Mar 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 23 additions & 6 deletions .github/workflows/header_checks.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ jobs:
token: ${{ secrets.GITHUB_TOKEN }}
workflow: c-cpp.yml
if: "!startsWith(github.ref, 'refs/tags/') && github.ref != 'refs/heads/master' && github.ref != 'refs/heads/develop'"

- uses: actions/checkout@v2
- uses: actions/setup-python@v2
with:
python-version: '2.x'

- name: Print g++/mingw32-make version and path
run: |
g++ --version
Expand All @@ -41,7 +41,7 @@ jobs:
- name: Build Math libs
shell: powershell
run: mingw32-make -f make/standalone math-libs

- name: Run header tests
shell: powershell
run: make -j2 test-headers
Expand All @@ -56,10 +56,27 @@ jobs:
token: ${{ secrets.GITHUB_TOKEN }}
workflow: c-cpp.yml
if: "!startsWith(github.ref, 'refs/tags/') && github.ref != 'refs/heads/master' && github.ref != 'refs/heads/develop'"

- uses: actions/checkout@v2

- name: Run header tests
run: |
echo "STAN_OPENCL=true" > make/local
make -j2 test-headers
make -j2 test-headers
no_range_checks:
name: NoRange
runs-on: ubuntu-latest

steps:
- uses: n1hility/cancel-previous-runs@v2
with:
token: ${{ secrets.GITHUB_TOKEN }}
workflow: c-cpp.yml
if: "!startsWith(github.ref, 'refs/tags/') && github.ref != 'refs/heads/master' && github.ref != 'refs/heads/develop'"

- uses: actions/checkout@v2

- name: Run header tests
run: |
echo "CXXFLAGS+=-DSTAN_NO_RANGE_CHECKS" > make/local
./runTests.py -j2 ./test/unit/math/prim/err/
3 changes: 3 additions & 0 deletions stan/math/prim/err/check_range.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ namespace math {
*/
inline void check_range(const char* function, const char* name, int max,
int index, int nested_level, const char* error_msg) {
STAN_NO_RANGE_CHECKS_RETURN;
if (!((index >= stan::error_index::value)
&& (index < max + stan::error_index::value))) {
[&]() STAN_COLD_PATH {
Expand All @@ -47,6 +48,7 @@ inline void check_range(const char* function, const char* name, int max,
*/
inline void check_range(const char* function, const char* name, int max,
int index, const char* error_msg) {
STAN_NO_RANGE_CHECKS_RETURN;
if (!((index >= stan::error_index::value)
&& (index < max + stan::error_index::value))) {
[&]() STAN_COLD_PATH { out_of_range(function, max, index, error_msg); }();
Expand All @@ -65,6 +67,7 @@ inline void check_range(const char* function, const char* name, int max,
*/
inline void check_range(const char* function, const char* name, int max,
int index) {
STAN_NO_RANGE_CHECKS_RETURN;
if (!((index >= stan::error_index::value)
&& (index < max + stan::error_index::value))) {
[&]() STAN_COLD_PATH { out_of_range(function, max, index); }();
Expand Down
1 change: 1 addition & 0 deletions stan/math/prim/err/check_row_index.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ namespace math {
template <typename T_y, typename = require_eigen_t<T_y>>
inline void check_row_index(const char* function, const char* name,
const T_y& y, size_t i) {
STAN_NO_RANGE_CHECKS_RETURN;
if (!(i >= stan::error_index::value
&& i < static_cast<size_t>(y.rows()) + stan::error_index::value)) {
[&]() STAN_COLD_PATH {
Expand Down
1 change: 1 addition & 0 deletions stan/math/prim/err/check_std_vector_index.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ namespace math {
template <typename T>
inline void check_std_vector_index(const char* function, const char* name,
const std::vector<T>& y, int i) {
STAN_NO_RANGE_CHECKS_RETURN;
if (!(i >= static_cast<int>(stan::error_index::value)
&& i < static_cast<int>(y.size() + stan::error_index::value))) {
[&]() STAN_COLD_PATH {
Expand Down
1 change: 1 addition & 0 deletions stan/math/prim/err/check_vector.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ template <typename Mat,
is_prim_or_rev_kernel_expression<Mat>>* = nullptr>
inline void check_vector(const char* function, const char* name, const Mat& x) {
if (!(x.rows() == 1 || x.cols() == 1)) {
STAN_NO_RANGE_CHECKS_RETURN;
[&]() STAN_COLD_PATH {
std::ostringstream msg;
msg << ") has " << x.rows() << " rows and " << x.cols()
Expand Down
1 change: 1 addition & 0 deletions stan/math/prim/err/check_vector_index.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ template <typename T,
is_prim_or_rev_kernel_expression<T>>* = nullptr>
inline void check_vector_index(const char* function, const char* name,
const T& y, size_t i) {
STAN_NO_RANGE_CHECKS_RETURN;
if (!(i >= stan::error_index::value
&& i < static_cast<size_t>(y.size()) + stan::error_index::value)) {
[&]() STAN_COLD_PATH {
Expand Down
2 changes: 1 addition & 1 deletion stan/math/prim/meta.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,11 @@
* `require` type traits to detect types used with OpenCL.
*/

#include <stan/math/prim/meta/compiler_attributes.hpp>
#include <stan/math/prim/meta/ad_promotable.hpp>
#include <stan/math/prim/meta/append_return_type.hpp>
#include <stan/math/prim/meta/base_type.hpp>
#include <stan/math/prim/meta/child_type.hpp>
#include <stan/math/prim/meta/compiler_attributes.hpp>
#include <stan/math/prim/meta/contains_fvar.hpp>
#include <stan/math/prim/meta/contains_std_vector.hpp>
#include <stan/math/prim/meta/error_index.hpp>
Expand Down
41 changes: 38 additions & 3 deletions stan/math/prim/meta/compiler_attributes.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,48 @@
#define STAN_MATH_PRIM_META_COMPILER_ATTRIBUTES_HPP

#ifdef __GNUC__
#ifndef likely
Copy link
Member

@bbbales2 bbbales2 Mar 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it that old gccs didn't have likely and new ones do?

Copy link
Collaborator Author

@SteveBronder SteveBronder Mar 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another package could have it

#define likely(x) __builtin_expect(!!(x), 1)
#endif
#ifndef unlikely
#define unlikely(x) __builtin_expect(!!(x), 0)
#endif
#ifdef __has_attribute
#if __has_attribute(noinline) && __has_attribute(cold)
#ifndef STAN_COLD_PATH
/**
* Functions tagged with this attribute are not inlined and moved
* to a cold branch to tell the CPU to not attempt to pre-fetch
* the associated function.
*/
#define STAN_COLD_PATH __attribute__((noinline, cold))
#else
#define likely(x) (x)
#define unlikely(x) (x)
#endif
#endif
#endif
#endif
#ifndef STAN_COLD_PATH
#define STAN_COLD_PATH
#endif
#ifndef likely
#define likely(x) x
#endif
#ifndef unlikely
#define unlikely(x) x
#endif

/**
* Turns all range and size checks into no-ops
*/
#ifndef STAN_NO_RANGE_CHECKS_RETURN
/**
* If defined, will turn off all range and size checks.
*/
#ifdef STAN_NO_RANGE_CHECKS
#define STAN_NO_RANGE_CHECKS_RETURN return
#endif
#ifndef STAN_NO_RANGE_CHECKS_RETURN
#define STAN_NO_RANGE_CHECKS_RETURN
#endif
#endif

#endif
17 changes: 9 additions & 8 deletions test/unit/math/prim/err/check_range_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include <stan/math/prim.hpp>
#include <gtest/gtest.h>
#include <test/unit/util.hpp>
#include <test/unit/math/prim/err/util.hpp>
#include <string>
#include <vector>

Expand All @@ -20,16 +21,16 @@ TEST(ErrorHandlingMatrix, checkRange_6_arg_std_vector) {
"index 12 out of range; "
"expecting index to be between 1 and 4; "
"index position = 4";
EXPECT_THROW_MSG(check_range("function", "x", 4, 12, 4, ""),
std::out_of_range, expected_message);
STAN_EXPECT_THROW_MSG(check_range("function", "x", 4, 12, 4, ""),
std::out_of_range, expected_message);

std::string expected_message_empty_container
= "function: accessing element out of range. "
"index 0 out of range; "
"container is empty and cannot be indexed; "
"index position = 4";
EXPECT_THROW_MSG(check_range("function", "x", 0, 0, 4, ""), std::out_of_range,
expected_message_empty_container);
STAN_EXPECT_THROW_MSG(check_range("function", "x", 0, 0, 4, ""),
std::out_of_range, expected_message_empty_container);
}

TEST(ErrorHandlingMatrix, checkRange_4_arg_std_vector) {
Expand All @@ -47,13 +48,13 @@ TEST(ErrorHandlingMatrix, checkRange_4_arg_std_vector) {
= "function: accessing element out of range. "
"index 12 out of range; "
"expecting index to be between 1 and 4";
EXPECT_THROW_MSG(check_range("function", "x", 4, 12), std::out_of_range,
expected_message);
STAN_EXPECT_THROW_MSG(check_range("function", "x", 4, 12), std::out_of_range,
expected_message);

std::string expected_message_empty_container
= "function: accessing element out of range. "
"index 0 out of range; "
"container is empty and cannot be indexed";
EXPECT_THROW_MSG(check_range("function", "x", 0, 0), std::out_of_range,
expected_message_empty_container);
STAN_EXPECT_THROW_MSG(check_range("function", "x", 0, 0), std::out_of_range,
expected_message_empty_container);
}
21 changes: 13 additions & 8 deletions test/unit/math/prim/err/check_row_index_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include <stan/math/prim.hpp>
#include <gtest/gtest.h>
#include <test/unit/math/prim/err/util.hpp>
#include <limits>

TEST(ErrorHandlingMatrix, checkRowIndexMatrix) {
Expand All @@ -15,12 +16,14 @@ TEST(ErrorHandlingMatrix, checkRowIndexMatrix) {
stan::math::check_row_index("checkRowIndexMatrix", "i", y, i));

y.resize(2, 3);
EXPECT_THROW(stan::math::check_row_index("checkRowIndexMatrix", "i", y, i),
std::out_of_range);
STAN_EXPECT_THROW(
stan::math::check_row_index("checkRowIndexMatrix", "i", y, i),
std::out_of_range);

i = 0;
EXPECT_THROW(stan::math::check_row_index("checkRowIndexMatrix", "i", y, i),
std::out_of_range);
STAN_EXPECT_THROW(
stan::math::check_row_index("checkRowIndexMatrix", "i", y, i),
std::out_of_range);
}

TEST(ErrorHandlingMatrix, checkRowIndexMatrix_nan) {
Expand All @@ -39,10 +42,12 @@ TEST(ErrorHandlingMatrix, checkRowIndexMatrix_nan) {

y.resize(2, 3);
y << nan, nan, nan, nan, nan, nan;
EXPECT_THROW(stan::math::check_row_index("checkRowIndexMatrix", "i", y, i),
std::out_of_range);
STAN_EXPECT_THROW(
stan::math::check_row_index("checkRowIndexMatrix", "i", y, i),
std::out_of_range);

i = 0;
EXPECT_THROW(stan::math::check_row_index("checkRowIndexMatrix", "i", y, i),
std::out_of_range);
STAN_EXPECT_THROW(
stan::math::check_row_index("checkRowIndexMatrix", "i", y, i),
std::out_of_range);
}
25 changes: 13 additions & 12 deletions test/unit/math/prim/err/check_std_vector_index_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include <stan/math/prim.hpp>
#include <gtest/gtest.h>
#include <test/unit/math/prim/err/util.hpp>
#include <limits>
#include <vector>

Expand All @@ -20,14 +21,14 @@ TEST(ErrorHandlingMatrix, checkStdVectorIndexMatrix) {
"checkStdVectorIndexMatrix", "i", y, i));

y.resize(2);
EXPECT_THROW(stan::math::check_std_vector_index("checkStdVectorIndexMatrix",
"i", y, i),
std::out_of_range);
STAN_EXPECT_THROW(stan::math::check_std_vector_index(
"checkStdVectorIndexMatrix", "i", y, i),
std::out_of_range);

i = 0;
EXPECT_THROW(stan::math::check_std_vector_index("checkStdVectorIndexMatrix",
"i", y, i),
std::out_of_range);
STAN_EXPECT_THROW(stan::math::check_std_vector_index(
"checkStdVectorIndexMatrix", "i", y, i),
std::out_of_range);
}

TEST(ErrorHandlingMatrix, checkStdVectorIndexMatrix_nan) {
Expand All @@ -48,12 +49,12 @@ TEST(ErrorHandlingMatrix, checkStdVectorIndexMatrix_nan) {
"checkStdVectorIndexMatrix", "i", y, i));

y.resize(2);
EXPECT_THROW(stan::math::check_std_vector_index("checkStdVectorIndexMatrix",
"i", y, i),
std::out_of_range);
STAN_EXPECT_THROW(stan::math::check_std_vector_index(
"checkStdVectorIndexMatrix", "i", y, i),
std::out_of_range);

i = 0;
EXPECT_THROW(stan::math::check_std_vector_index("checkStdVectorIndexMatrix",
"i", y, i),
std::out_of_range);
STAN_EXPECT_THROW(stan::math::check_std_vector_index(
"checkStdVectorIndexMatrix", "i", y, i),
std::out_of_range);
}
9 changes: 5 additions & 4 deletions test/unit/math/prim/err/check_vector_index_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include <stan/math/prim.hpp>
#include <gtest/gtest.h>
#include <test/unit/math/prim/err/util.hpp>
#include <limits>

TEST(ErrorHandlingMatrix, checkVectorIndexColumnVector) {
Expand All @@ -18,12 +19,12 @@ TEST(ErrorHandlingMatrix, checkVectorIndexColumnVector) {

y.resize(2);
y << nan, nan;
EXPECT_THROW(
STAN_EXPECT_THROW(
stan::math::check_vector_index("checkVectorIndexMatrix", "i", y, i),
std::out_of_range);

i = 0;
EXPECT_THROW(
STAN_EXPECT_THROW(
stan::math::check_vector_index("checkVectorIndexMatrix", "i", y, i),
std::out_of_range);
}
Expand All @@ -44,12 +45,12 @@ TEST(ErrorHandlingMatrix, checkVectorIndexRowVector) {

y.resize(2);
y << nan, nan;
EXPECT_THROW(
STAN_EXPECT_THROW(
stan::math::check_vector_index("checkVectorIndexMatrix", "i", y, i),
std::out_of_range);

i = 0;
EXPECT_THROW(
STAN_EXPECT_THROW(
stan::math::check_vector_index("checkVectorIndexMatrix", "i", y, i),
std::out_of_range);
}
17 changes: 9 additions & 8 deletions test/unit/math/prim/err/check_vector_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
#include <stan/math/prim.hpp>
#include <gtest/gtest.h>
#include <test/unit/math/prim/err/util.hpp>
#include <limits>

TEST(ErrorHandlingMatrix, checkVectorMatrix) {
Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic> x;

x.resize(3, 3);
EXPECT_THROW(stan::math::check_vector("checkVector", "x", x),
std::invalid_argument);
STAN_EXPECT_THROW(stan::math::check_vector("checkVector", "x", x),
std::invalid_argument);
x.resize(0, 0);
EXPECT_THROW(stan::math::check_vector("checkVector", "x", x),
std::invalid_argument);
STAN_EXPECT_THROW(stan::math::check_vector("checkVector", "x", x),
std::invalid_argument);

x.resize(1, 5);
EXPECT_NO_THROW(stan::math::check_vector("checkVector", "x", x));
Expand All @@ -25,11 +26,11 @@ TEST(ErrorHandlingMatrix, checkVectorMatrix_nan) {

x.resize(3, 3);
x << nan, nan, nan, nan, nan, nan, nan, nan, nan;
EXPECT_THROW(stan::math::check_vector("checkVector", "x", x),
std::invalid_argument);
STAN_EXPECT_THROW(stan::math::check_vector("checkVector", "x", x),
std::invalid_argument);
x.resize(0, 0);
EXPECT_THROW(stan::math::check_vector("checkVector", "x", x),
std::invalid_argument);
STAN_EXPECT_THROW(stan::math::check_vector("checkVector", "x", x),
std::invalid_argument);

x.resize(1, 5);
x << nan, nan, nan, nan, nan;
Expand Down
Loading