From 2797ff412a012b00fed9acbd9cbfa508df6c540d Mon Sep 17 00:00:00 2001 From: Benjamin Kietzman Date: Thu, 6 Aug 2020 00:12:56 -0400 Subject: [PATCH 1/7] add a workflow to check clang-format --- .github/workflows/format.yaml | 30 +++++++++++++++++++++++++ Makefile | 8 +++++++ inst/include/cpp11.hpp | 4 ++-- inst/include/cpp11/as.hpp | 9 ++++---- inst/include/cpp11/attribute_proxy.hpp | 5 +++-- inst/include/cpp11/data_frame.hpp | 12 +++++----- inst/include/cpp11/doubles.hpp | 7 +++--- inst/include/cpp11/environment.hpp | 3 ++- inst/include/cpp11/external_pointer.hpp | 7 +++--- inst/include/cpp11/function.hpp | 8 ++++--- inst/include/cpp11/integers.hpp | 7 +++--- inst/include/cpp11/list.hpp | 3 ++- inst/include/cpp11/list_of.hpp | 3 ++- inst/include/cpp11/logicals.hpp | 7 +++--- inst/include/cpp11/matrix.hpp | 3 ++- inst/include/cpp11/named_arg.hpp | 8 ++++--- inst/include/cpp11/protect.hpp | 12 +++++----- inst/include/cpp11/r_string.hpp | 5 +++-- inst/include/cpp11/r_vector.hpp | 24 +++++++++++--------- inst/include/cpp11/raws.hpp | 9 ++++---- inst/include/cpp11/sexp.hpp | 6 +++-- inst/include/cpp11/strings.hpp | 5 +++-- 22 files changed, 122 insertions(+), 63 deletions(-) create mode 100644 .github/workflows/format.yaml diff --git a/.github/workflows/format.yaml b/.github/workflows/format.yaml new file mode 100644 index 00000000..d4f43a02 --- /dev/null +++ b/.github/workflows/format.yaml @@ -0,0 +1,30 @@ +on: + push: + branches: master + pull_request: + branches: + - master + +name: format_check + +jobs: + format_check: + runs-on: ubuntu-18.04 + steps: + - uses: actions/checkout@v2 + + - name: Install ClangFormat + run: apt install -y clang-format-10 + + - name: Run ClangFormat + run: make format clang_format=clang-format-10 + + - name: Check for a non-empty diff + run: | + if [ "$(git diff --name-only)" != "" ] + then + echo "Please run clang-format to make the following changes:" + git diff + exit 1 + fi + diff --git a/Makefile b/Makefile index a368217f..6e08c904 100644 --- a/Makefile +++ b/Makefile @@ -10,3 +10,11 @@ test: all clean: @Rscript -e 'devtools::clean_dll()' + +clang_format=`which clang-format` +format: $(shell find . -name *.hpp) +ifeq ($(findstring version 10,$(shell ${clang_format} --version 2>/dev/null)),) + @echo "clang-format 10 is required" +else + @${clang_format} -i $? +endif diff --git a/inst/include/cpp11.hpp b/inst/include/cpp11.hpp index 1cd736a7..86cd3104 100644 --- a/inst/include/cpp11.hpp +++ b/inst/include/cpp11.hpp @@ -16,8 +16,8 @@ #include "cpp11/matrix.hpp" #include "cpp11/named_arg.hpp" #include "cpp11/protect.hpp" +#include "cpp11/r_string.hpp" +#include "cpp11/r_vector.hpp" #include "cpp11/raws.hpp" #include "cpp11/sexp.hpp" -#include "cpp11/r_string.hpp" #include "cpp11/strings.hpp" -#include "cpp11/r_vector.hpp" diff --git a/inst/include/cpp11/as.hpp b/inst/include/cpp11/as.hpp index 01251cc4..c6dd5b59 100644 --- a/inst/include/cpp11/as.hpp +++ b/inst/include/cpp11/as.hpp @@ -1,9 +1,10 @@ #pragma once -#include // for modf -#include // for initializer_list -#include // for string, basic_string -#include // for decay, enable_if, is_same, is_convertible +#include // for modf +#include // for initializer_list +#include // for string, basic_string +#include // for decay, enable_if, is_same, is_convertible + #include "cpp11/R.hpp" // for SEXP, SEXPREC, Rf_xlength, R_xlen_t #include "cpp11/protect.hpp" // for stop, protect, safe, protect::function diff --git a/inst/include/cpp11/attribute_proxy.hpp b/inst/include/cpp11/attribute_proxy.hpp index 7bb2bc69..64e7436b 100644 --- a/inst/include/cpp11/attribute_proxy.hpp +++ b/inst/include/cpp11/attribute_proxy.hpp @@ -1,7 +1,8 @@ #pragma once -#include // for initializer_list -#include // for string, basic_string +#include // for initializer_list +#include // for string, basic_string + #include "cpp11/R.hpp" // for SEXP, SEXPREC, Rf_install, PROTECT, Rf_... #include "cpp11/as.hpp" // for as_sexp #include "cpp11/protect.hpp" // for protect, safe, protect::function diff --git a/inst/include/cpp11/data_frame.hpp b/inst/include/cpp11/data_frame.hpp index c88e0b8f..f1caad51 100644 --- a/inst/include/cpp11/data_frame.hpp +++ b/inst/include/cpp11/data_frame.hpp @@ -1,17 +1,17 @@ #pragma once -#include // for abs -#include // for initializer_list -#include // for string, basic_string -#include // for move +#include // for abs +#include +#include // for initializer_list +#include // for string, basic_string +#include // for move + #include "R_ext/Arith.h" // for NA_INTEGER #include "cpp11/R.hpp" // for Rf_xlength, SEXP, SEXPREC, INTEGER #include "cpp11/attribute_proxy.hpp" // for attribute_proxy #include "cpp11/list.hpp" // for list, r_vector<>::r_vector, r_v... #include "cpp11/r_vector.hpp" // for r_vector -#include - namespace cpp11 { class named_arg; diff --git a/inst/include/cpp11/doubles.hpp b/inst/include/cpp11/doubles.hpp index cd1ce9e8..88a812cb 100644 --- a/inst/include/cpp11/doubles.hpp +++ b/inst/include/cpp11/doubles.hpp @@ -1,8 +1,9 @@ #pragma once -#include // for min -#include // for array -#include // for initializer_list +#include // for min +#include // for array +#include // for initializer_list + #include "R_ext/Arith.h" // for ISNA #include "cpp11/R.hpp" // for SEXP, SEXPREC, Rf_allocVector, REAL #include "cpp11/as.hpp" // for as_sexp diff --git a/inst/include/cpp11/environment.hpp b/inst/include/cpp11/environment.hpp index 6256c997..6e285595 100644 --- a/inst/include/cpp11/environment.hpp +++ b/inst/include/cpp11/environment.hpp @@ -1,6 +1,7 @@ #pragma once -#include // for string, basic_string +#include // for string, basic_string + #include "Rversion.h" // for R_VERSION, R_Version #include "cpp11/R.hpp" // for SEXP, SEXPREC, Rf_install, Rf_findVarIn... #include "cpp11/as.hpp" // for as_sexp diff --git a/inst/include/cpp11/external_pointer.hpp b/inst/include/cpp11/external_pointer.hpp index 47d8fa62..ca082fa6 100644 --- a/inst/include/cpp11/external_pointer.hpp +++ b/inst/include/cpp11/external_pointer.hpp @@ -1,8 +1,9 @@ #pragma once -#include // for nullptr_t, NULL -#include // for bad_weak_ptr -#include // for add_lvalue_reference +#include // for nullptr_t, NULL +#include // for bad_weak_ptr +#include // for add_lvalue_reference + #include "cpp11/R.hpp" // for SEXP, SEXPREC, TYPEOF, R_NilValue, R_C... #include "cpp11/protect.hpp" // for protect, safe, protect::function #include "cpp11/r_vector.hpp" // for type_error diff --git a/inst/include/cpp11/function.hpp b/inst/include/cpp11/function.hpp index 3bfa426d..24086b98 100644 --- a/inst/include/cpp11/function.hpp +++ b/inst/include/cpp11/function.hpp @@ -1,8 +1,10 @@ #pragma once -#include // for strcmp -#include // for string, basic_string -#include // for forward +#include // for strcmp + +#include // for string, basic_string +#include // for forward + #include "cpp11/R.hpp" // for SEXP, SEXPREC, CDR, Rf_install, SETCAR #include "cpp11/as.hpp" // for as_sexp #include "cpp11/named_arg.hpp" // for named_arg diff --git a/inst/include/cpp11/integers.hpp b/inst/include/cpp11/integers.hpp index 5389b79a..d757e350 100644 --- a/inst/include/cpp11/integers.hpp +++ b/inst/include/cpp11/integers.hpp @@ -1,8 +1,9 @@ #pragma once -#include // for min -#include // for array -#include // for initializer_list +#include // for min +#include // for array +#include // for initializer_list + #include "R_ext/Arith.h" // for NA_INTEGER #include "cpp11/R.hpp" // for SEXP, SEXPREC, Rf_allocVector #include "cpp11/as.hpp" // for as_sexp diff --git a/inst/include/cpp11/list.hpp b/inst/include/cpp11/list.hpp index fa54440d..0f1352ea 100644 --- a/inst/include/cpp11/list.hpp +++ b/inst/include/cpp11/list.hpp @@ -1,6 +1,7 @@ #pragma once -#include // for initializer_list +#include // for initializer_list + #include "cpp11/R.hpp" // for SEXP, SEXPREC, SET_VECTOR_ELT #include "cpp11/attribute_proxy.hpp" // for attribute_proxy #include "cpp11/named_arg.hpp" // for named_arg diff --git a/inst/include/cpp11/list_of.hpp b/inst/include/cpp11/list_of.hpp index ce0f9478..14780986 100644 --- a/inst/include/cpp11/list_of.hpp +++ b/inst/include/cpp11/list_of.hpp @@ -1,6 +1,7 @@ #pragma once -#include // for string, basic_string +#include // for string, basic_string + #include "cpp11/R.hpp" // for R_xlen_t, SEXP, SEXPREC, LONG_VECTOR_SUPPORT #include "cpp11/list.hpp" // for list diff --git a/inst/include/cpp11/logicals.hpp b/inst/include/cpp11/logicals.hpp index ebf98690..91c72038 100644 --- a/inst/include/cpp11/logicals.hpp +++ b/inst/include/cpp11/logicals.hpp @@ -1,8 +1,9 @@ #pragma once -#include // for min -#include // for array -#include // for initializer_list +#include // for min +#include // for array +#include // for initializer_list + #include "cpp11/R.hpp" // for Rboolean, SEXP, SEXPREC, Rf_all... #include "cpp11/attribute_proxy.hpp" // for attribute_proxy #include "cpp11/named_arg.hpp" // for named_arg diff --git a/inst/include/cpp11/matrix.hpp b/inst/include/cpp11/matrix.hpp index a6ee596e..dc264751 100644 --- a/inst/include/cpp11/matrix.hpp +++ b/inst/include/cpp11/matrix.hpp @@ -1,6 +1,7 @@ #pragma once -#include // for string +#include // for string + #include "cpp11/R.hpp" // for SEXP, SEXPREC, R_xlen_t, Rboolean, INT... #include "cpp11/r_string.hpp" // for r_string #include "cpp11/r_vector.hpp" // for r_vector diff --git a/inst/include/cpp11/named_arg.hpp b/inst/include/cpp11/named_arg.hpp index 26cc2c4a..fadbcc20 100644 --- a/inst/include/cpp11/named_arg.hpp +++ b/inst/include/cpp11/named_arg.hpp @@ -1,9 +1,11 @@ #pragma once -#include // for size_t +#include // for size_t + #include // for initializer_list -#include "cpp11/R.hpp" // for SEXP, SEXPREC, literals -#include "cpp11/as.hpp" // for as_sexp + +#include "cpp11/R.hpp" // for SEXP, SEXPREC, literals +#include "cpp11/as.hpp" // for as_sexp namespace cpp11 { class named_arg { diff --git a/inst/include/cpp11/protect.hpp b/inst/include/cpp11/protect.hpp index 9b0d22cc..a4611dde 100644 --- a/inst/include/cpp11/protect.hpp +++ b/inst/include/cpp11/protect.hpp @@ -1,16 +1,16 @@ #pragma once -#include "cpp11/R.hpp" // for SEXP, SEXPREC, CDR, R_NilValue, CAR, R_Pres... +#include // for longjmp, setjmp, jmp_buf +#include // for exception +#include // for std::runtime_error +#include // for string, basic_string +#include // for tuple, make_tuple -#include // for longjmp, setjmp, jmp_buf -#include // for exception -#include // for std::runtime_error -#include // for string, basic_string -#include // for tuple, make_tuple #include "R_ext/Error.h" // for Rf_error, Rf_warning #include "R_ext/Print.h" // for REprintf #include "R_ext/Utils.h" // for R_CheckUserInterrupt #include "Rversion.h" // for R_VERSION, R_Version +#include "cpp11/R.hpp" // for SEXP, SEXPREC, CDR, R_NilValue, CAR, R_Pres... #if defined(R_VERSION) && R_VERSION >= R_Version(3, 5, 0) #define HAS_UNWIND_PROTECT diff --git a/inst/include/cpp11/r_string.hpp b/inst/include/cpp11/r_string.hpp index 7ac45457..3c7cc4a4 100644 --- a/inst/include/cpp11/r_string.hpp +++ b/inst/include/cpp11/r_string.hpp @@ -1,7 +1,8 @@ #pragma once -#include // for string, basic_string, operator== -#include // for is_convertible, enable_if +#include // for string, basic_string, operator== +#include // for is_convertible, enable_if + #include "R_ext/Memory.h" // for vmaxget, vmaxset #include "cpp11/R.hpp" // for SEXP, SEXPREC, Rf_mkCharCE, Rf_translat... #include "cpp11/as.hpp" // for as_sexp diff --git a/inst/include/cpp11/r_vector.hpp b/inst/include/cpp11/r_vector.hpp index 878dbd64..b74149ed 100644 --- a/inst/include/cpp11/r_vector.hpp +++ b/inst/include/cpp11/r_vector.hpp @@ -1,16 +1,18 @@ #pragma once -#include // for ptrdiff_t, size_t -#include // for max -#include // for array -#include // for snprintf -#include // for exception -#include // for initializer_list -#include // for forward_iterator_tag, random_ac... -#include // for out_of_range -#include // for string, basic_string -#include // for decay, is_same, enable_if, is_c... -#include // for declval +#include // for ptrdiff_t, size_t + +#include // for max +#include // for array +#include // for snprintf +#include // for exception +#include // for initializer_list +#include // for forward_iterator_tag, random_ac... +#include // for out_of_range +#include // for string, basic_string +#include // for decay, is_same, enable_if, is_c... +#include // for declval + #include "cpp11/R.hpp" // for R_xlen_t, SEXP, SEXPREC, Rf_xle... #include "cpp11/attribute_proxy.hpp" // for attribute_proxy #include "cpp11/protect.hpp" // for protect_sexp, release_protect diff --git a/inst/include/cpp11/raws.hpp b/inst/include/cpp11/raws.hpp index 5adc617f..8e6fe1f9 100644 --- a/inst/include/cpp11/raws.hpp +++ b/inst/include/cpp11/raws.hpp @@ -1,9 +1,10 @@ #pragma once -#include // for min -#include // for array -#include // for uint8_t -#include // for initializer_list +#include // for min +#include // for array +#include // for uint8_t +#include // for initializer_list + #include "cpp11/R.hpp" // for RAW, SEXP, SEXPREC, Rf_allocVector #include "cpp11/attribute_proxy.hpp" // for attribute_proxy #include "cpp11/named_arg.hpp" // for named_arg diff --git a/inst/include/cpp11/sexp.hpp b/inst/include/cpp11/sexp.hpp index 44e62bdf..73d59bd8 100644 --- a/inst/include/cpp11/sexp.hpp +++ b/inst/include/cpp11/sexp.hpp @@ -1,7 +1,9 @@ #pragma once -#include // for size_t -#include // for string, basic_string +#include // for size_t + +#include // for string, basic_string + #include "cpp11/R.hpp" // for SEXP, SEXPREC, REAL_ELT, R_NilV... #include "cpp11/attribute_proxy.hpp" // for attribute_proxy #include "cpp11/protect.hpp" // for protect_sexp, release_protect diff --git a/inst/include/cpp11/strings.hpp b/inst/include/cpp11/strings.hpp index 6c08e0a8..7bcb602a 100644 --- a/inst/include/cpp11/strings.hpp +++ b/inst/include/cpp11/strings.hpp @@ -1,7 +1,8 @@ #pragma once -#include // for initializer_list -#include // for string, basic_string +#include // for initializer_list +#include // for string, basic_string + #include "cpp11/R.hpp" // for SEXP, TYPEOF, SEXPREC, SET_STRI... #include "cpp11/as.hpp" // for as_sexp #include "cpp11/attribute_proxy.hpp" // for attribute_proxy From 05b916032cb2067e75379619f1db4bd0cd4dc13d Mon Sep 17 00:00:00 2001 From: Benjamin Kietzman Date: Thu, 6 Aug 2020 08:45:10 -0400 Subject: [PATCH 2/7] sudo apt install clang-format-10 --- .github/workflows/format.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/format.yaml b/.github/workflows/format.yaml index d4f43a02..6603bdf9 100644 --- a/.github/workflows/format.yaml +++ b/.github/workflows/format.yaml @@ -14,7 +14,7 @@ jobs: - uses: actions/checkout@v2 - name: Install ClangFormat - run: apt install -y clang-format-10 + run: sudo apt install -y clang-format-10 - name: Run ClangFormat run: make format clang_format=clang-format-10 From bf3ee545a7b60110442d58cc83baefa60dc850d0 Mon Sep 17 00:00:00 2001 From: Benjamin Kietzman Date: Thu, 6 Aug 2020 08:59:15 -0400 Subject: [PATCH 3/7] try forcing #include order --- inst/include/cpp11/protect.hpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/inst/include/cpp11/protect.hpp b/inst/include/cpp11/protect.hpp index a4611dde..fe5f8aa4 100644 --- a/inst/include/cpp11/protect.hpp +++ b/inst/include/cpp11/protect.hpp @@ -6,11 +6,13 @@ #include // for string, basic_string #include // for tuple, make_tuple +// NB: cpp11/R.hpp must precede R_ext includes so our definition of Rboolean is used +#include "cpp11/R.hpp" // for SEXP, SEXPREC, CDR, R_NilValue, CAR, R_Pres... + #include "R_ext/Error.h" // for Rf_error, Rf_warning #include "R_ext/Print.h" // for REprintf #include "R_ext/Utils.h" // for R_CheckUserInterrupt #include "Rversion.h" // for R_VERSION, R_Version -#include "cpp11/R.hpp" // for SEXP, SEXPREC, CDR, R_NilValue, CAR, R_Pres... #if defined(R_VERSION) && R_VERSION >= R_Version(3, 5, 0) #define HAS_UNWIND_PROTECT From 89bf286105683768ecf656d97a8cf47509131b44 Mon Sep 17 00:00:00 2001 From: Benjamin Kietzman Date: Thu, 6 Aug 2020 09:01:14 -0400 Subject: [PATCH 4/7] use apt-get to install instead of apt --- .github/workflows/format.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/format.yaml b/.github/workflows/format.yaml index 6603bdf9..1443d9ea 100644 --- a/.github/workflows/format.yaml +++ b/.github/workflows/format.yaml @@ -14,7 +14,7 @@ jobs: - uses: actions/checkout@v2 - name: Install ClangFormat - run: sudo apt install -y clang-format-10 + run: sudo apt-get install -y clang-format-10 - name: Run ClangFormat run: make format clang_format=clang-format-10 From b7c70718c08ad9369fda4531138a670fb7ca2d3f Mon Sep 17 00:00:00 2001 From: Benjamin Kietzman Date: Thu, 6 Aug 2020 09:09:27 -0400 Subject: [PATCH 5/7] Only sort *within* a block of #include --- .clang-format | 1 + 1 file changed, 1 insertion(+) diff --git a/.clang-format b/.clang-format index f5978616..93359353 100644 --- a/.clang-format +++ b/.clang-format @@ -1,3 +1,4 @@ BasedOnStyle: Google DerivePointerAlignment: false ColumnLimit: 90 +IncludeBlocks: Preserve From 61410d95b77230e6ebaf19b569cca112a20c97e5 Mon Sep 17 00:00:00 2001 From: Benjamin Kietzman Date: Thu, 6 Aug 2020 10:03:18 -0400 Subject: [PATCH 6/7] also format **/*.cpp --- Makefile | 2 +- cpp11test/src/test-list.cpp | 4 +--- cpp11test/src/test-list_of.cpp | 6 +++--- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 6e08c904..7b365a97 100644 --- a/Makefile +++ b/Makefile @@ -12,7 +12,7 @@ clean: @Rscript -e 'devtools::clean_dll()' clang_format=`which clang-format` -format: $(shell find . -name *.hpp) +format: $(shell find . -name *.hpp) $(shell find . -name *.cpp) ifeq ($(findstring version 10,$(shell ${clang_format} --version 2>/dev/null)),) @echo "clang-format 10 is required" else diff --git a/cpp11test/src/test-list.cpp b/cpp11test/src/test-list.cpp index d7b4c4dd..d2e909ae 100644 --- a/cpp11test/src/test-list.cpp +++ b/cpp11test/src/test-list.cpp @@ -59,9 +59,7 @@ context("list-C++") { } test_that("list::iterator uses VECTOR_ELT") { - cpp11::writable::list x({ - cpp11::writable::integers({1, 2}) - }); + cpp11::writable::list x({cpp11::writable::integers({1, 2})}); cpp11::integers first(*x.begin()); expect_true(first[0] == 1); expect_true(first[1] == 2); diff --git a/cpp11test/src/test-list_of.cpp b/cpp11test/src/test-list_of.cpp index ed5a588c..979a16e7 100644 --- a/cpp11test/src/test-list_of.cpp +++ b/cpp11test/src/test-list_of.cpp @@ -1,15 +1,15 @@ #include -#include "cpp11/strings.hpp" #include "cpp11/doubles.hpp" #include "cpp11/list.hpp" #include "cpp11/list_of.hpp" +#include "cpp11/strings.hpp" context("list_of-C++") { test_that("list_of works") { using namespace cpp11::literals; cpp11::writable::list x({"x"_nm = cpp11::writable::doubles({1., 2., 3.}), - "y"_nm = cpp11::writable::doubles({4., 5., 6.})}); + "y"_nm = cpp11::writable::doubles({4., 5., 6.})}); cpp11::list_of res(x); @@ -24,7 +24,7 @@ context("list_of-C++") { using namespace cpp11::literals; cpp11::writable::list x({"x"_nm = cpp11::writable::doubles({1., 2., 3.}), - "y"_nm = cpp11::writable::doubles({4., 5., 6.})}); + "y"_nm = cpp11::writable::doubles({4., 5., 6.})}); cpp11::writable::list_of res(x); From 0a6da948afb08f6bf6e3d95c134423d65e9aa190 Mon Sep 17 00:00:00 2001 From: Benjamin Kietzman Date: Thu, 6 Aug 2020 12:23:37 -0400 Subject: [PATCH 7/7] use cleaner diff check --- .github/workflows/format.yaml | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/.github/workflows/format.yaml b/.github/workflows/format.yaml index 1443d9ea..a9670588 100644 --- a/.github/workflows/format.yaml +++ b/.github/workflows/format.yaml @@ -20,11 +20,5 @@ jobs: run: make format clang_format=clang-format-10 - name: Check for a non-empty diff - run: | - if [ "$(git diff --name-only)" != "" ] - then - echo "Please run clang-format to make the following changes:" - git diff - exit 1 - fi + run: git diff-files -U --exit-code