RcppCore/RcppParallel

`CXX_STD = CXX11` causes CRAN note on devel

rorynolan opened this issue · 3 comments

rorynolan commented

CXX_STD = CXX11 causes a CRAN note. The current CRAN default is C++17 and it doesn't like you to specify an older version.
The easy fix is to change the docs 11 -> 17 but that too will probably be too old someday. Perhaps the docs could just say that at least C++11 is required for RcppParallel to work but not do anything in Makevars?
Up to you how to tackle this, just wanted to make sure you were aware.

eddelbuettel commented

Duplicate of #194

kevinushey commented

I think there are some places we need to update:

  • The skeleton function, here:

    RcppParallel/R/skeleton.R

    Lines 108 to 130 in 649340b

    # src/Makevars
    message(" >> added src/Makevars")
    cat(
    c(
    'CXX_STD = CXX11',
    '# We also need importFrom(RcppParallel,RcppParallelLibs) in NAMESPACE',
    'PKG_LIBS += $(shell ${R_HOME}/bin/Rscript -e "RcppParallel::RcppParallelLibs()")'
    ),
    file = "src/Makevars",
    sep = "\n"
    )
    # src/Makevars.win
    message(" >> added src/Makevars.win")
    cat(
    c(
    'CXX_STD = CXX11',
    'PKG_CXXFLAGS += -DRCPP_PARALLEL_USE_TBB=1',
    'PKG_LIBS += $(shell "${R_HOME}/bin${R_ARCH_BIN}/Rscript.exe" -e "RcppParallel::RcppParallelLibs()")'
    ),
    file = "src/Makevars.win",
    sep = "\n"
    )
  • The documentation for the GitHub pages site here:

    RcppParallel/index.Rmd

    Lines 80 to 96 in 16876a2

    ```make
    CXX_STD = CXX11
    PKG_CXXFLAGS += -DRCPP_PARALLEL_USE_TBB=1
    PKG_LIBS += $(shell "${R_HOME}/bin${R_ARCH_BIN}/Rscript.exe" \
    -e "RcppParallel::RcppParallelLibs()")
    ```
    Note that the Windows variation (Makevars.win) requires an extra `PKG_CXXFLAGS` entry that enables the use of TBB. This is because TBB is not used by default on Windows (for backward compatibility with a previous version of RcppParallel which lacked support for TBB on Windows).
    TBB uses a subset of C++11 features -- for example, `long long` is not defined in the C++98 standard. Although most compilers make this type available as a compiler extension, they may emit warnings / errors when using these types while compiling in C++98 mode. To ensure that your package builds on CRAN without these warnings, you can enforce compilation using C++11, as done by `CXX_STD = CXX11`.
    After you've added the above to the package you can simply include the main RcppParallel package header in source files that need to use it:
    ```cpp
    #include <RcppParallel.h>
    ```

I'll fix those up.

  • 👍1
eddelbuettel commented

One thing I have been wondering, though, is whether or not we should set a CXX_STD. "Way back when" we had to as the then-default compiler was insisting on C++98. Now we have modern C++ by default, and adding this feels like adding a constraint. Should we just leave it open? (IIRC) R 4.0.0 turned on C++11, R 4.1.0 turned to C++14, R 4.3.0 will turn on C++17. Good enough?

Edit: Doh. And I wrote that before I saw your commit. And you did the Right Thing (TM) ❤️