RcppCore/RcppParallel

tbb.h deprecated features

Enchufa2 opened this issue · 11 comments

Packages compiling against RcppParallel show:

In file included from /usr/local/lib/R/library/RcppParallel/include/RcppParallel/TBB.h:10,
                 from /usr/local/lib/R/library/RcppParallel/include/RcppParallel.h:21,
                 from fittingFunctions.cpp:6:
/usr/include/tbb/tbb.h:21:154: note: '#pragma message: TBB Warning: tbb.h contains deprecated functionality. For details, please see Deprecated Features appendix in the TBB reference manual.'
   21 | #pragma message("TBB Warning: tbb.h contains deprecated functionality. For details, please see Deprecated Features appendix in the TBB reference manual.")
      |                                                                                                                                                          ^

tbb/tbb.h is included here. We should think about moving away from that header.

Possibly related:
CRAN told me to move away from C++11 specification. When I do, win-builder devel gives me

* checking whether package 'detrendr' can be installed ... WARNING
Found the following significant warnings:
  D:/RCompile/CRANpkg/lib/4.3/RcppParallel/include/tbb/concurrent_hash_map.h:343:23: warning: 'template<class _Category, class _Tp, class _Distance, class _Pointer, class _Reference> struct std::iterator' is deprecated [-Wdeprecated-declarations]
  D:/RCompile/CRANpkg/lib/4.3/RcppParallel/include/tbb/internal/_concurrent_queue_impl.h:744:21: warning: 'template<class _Category, class _Tp, class _Distance, class _Pointer, class _Reference> struct std::iterator' is deprecated [-Wdeprecated-declarations]
  D:/RCompile/CRANpkg/lib/4.3/RcppParallel/include/tbb/internal/_concurrent_queue_impl.h:1003:21: warning: 'template<class _Category, class _Tp, class _Distance, class _Pointer, class _Reference> struct std::iterator' is deprecated [-Wdeprecated-declarations]
  D:/RCompile/CRANpkg/lib/4.3/RcppParallel/include/tbb/internal/_concurrent_unordered_impl.h:67:36: warning: 'template<class _Category, class _Tp, class _Distance, class _Pointer, class _Reference> struct std::iterator' is deprecated [-Wdeprecated-declarations]
  D:/RCompile/CRANpkg/lib/4.3/RcppParallel/include/RcppParallel/RMatrix.h:18:24: warning: 'template<class _Category, class _Tp, class _Distance, class _Pointer, class _Reference> struct std::iterator' is deprecated [-Wdeprecated-declarations]

All warnings about deprecation of std::iterator.

CRAN told me to move away from C++11 specification.

On upload? Or did you (as I did notice and note here) just saw the note from r-devel? Sometimes those come and go, and not everything added to r-devel checks gets added to incoming checks. That said, they have been making moves there and where we can we should probably move forward. For package BH I commented out an explicit warning that the next upstream will require C++14 (commented out because CRAN dislikes noisy warning) so yes Boost is also moving forward. Generally the right thing to do.

But as we know RcppParallel is also not an easy package to maintain so thanks again to @kevinushey for doing the hard work here.

The C++11 warning is a current CRAN note for me on devel.
https://cran.r-project.org/web/checks/check_results_detrendr.html

When I remove the C++11 stuff it and do win-builder, I get the std::iterator warning
https://win-builder.r-project.org/vLN5P4A1iKHT/00check.log

And yes, every thanks to Kevin Ushey.

This CRAN test, as far as I know only days old, clearly creates confusion as someone on SO already claimed that 'C++11 is now outlawed at CRAN' which is of course not entirely correct. But as they move the minimum language standard from C++14 (currently) to C++17 they simply flag that maintainers way want to consider moving forward. Which may then, as in your case, trigger new and different warnings (from the compiler and C++ library).

Good to know.
For now, I will just live with the CRAN note, but I suspect poor Kevin will be forced to replace std::iterator eventually.
https://www.fluentcpp.com/2018/05/08/std-iterator-deprecated/

It would be better to open a new issue to track that specifically, because I don't think it has anything to do with the report here.

After all, the compilation error was a gcc issue, present in v13.0.0 but fixed in v13.0.1. First comment edited accordingly. We still should think about moving away from the deprecated header.

Including

CXX_STD = CXX14

in the src/Makevars did not solve this issue...

When I build the main branch from your NNS repo, it compiles fine as C++11 under the default flags I keep (and also without them):

edd@rob:/tmp/rcpp/NNS(NNS-Beta-Version)$ R_LIBS=../lib/ R CMD INSTALL .
* installing to library ‘/tmp/rcpp/lib’
* installing *source* package ‘NNS’ ...
** using staged installation
** libs
ccache g++  -std=gnu++11 -I"/usr/share/R/include" -DNDEBUG  -I'/usr/local/lib/R/site-library/Rcpp/include' -I'/usr/local/lib/R/site-library/RcppParallel/include'    -fpic  -g -O3 -Wall -pipe  -Wno-parentheses -Wno-ignored-attributes -Wno-unused-local-typedefs -Wno-deprecated-declarations -Wno-unused-function  -c RcppExports.cpp -o RcppExports.o
ccache g++  -std=gnu++11 -I"/usr/share/R/include" -DNDEBUG  -I'/usr/local/lib/R/site-library/Rcpp/include' -I'/usr/local/lib/R/site-library/RcppParallel/include'    -fpic  -g -O3 -Wall -pipe  -Wno-parentheses -Wno-ignored-attributes -Wno-unused-local-typedefs -Wno-deprecated-declarations -Wno-unused-function  -c nns_rcpp.cpp -o nns_rcpp.o
ccache g++  -std=gnu++11 -I"/usr/share/R/include" -DNDEBUG  -I'/usr/local/lib/R/site-library/Rcpp/include' -I'/usr/local/lib/R/site-library/RcppParallel/include'    -fpic  -g -O3 -Wall -pipe  -Wno-parentheses -Wno-ignored-attributes -Wno-unused-local-typedefs -Wno-deprecated-declarations -Wno-unused-function  -c partial_moments.cpp -o partial_moments.o
ccache g++  -std=gnu++11 -I"/usr/share/R/include" -DNDEBUG  -I'/usr/local/lib/R/site-library/Rcpp/include' -I'/usr/local/lib/R/site-library/RcppParallel/include'    -fpic  -g -O3 -Wall -pipe  -Wno-parentheses -Wno-ignored-attributes -Wno-unused-local-typedefs -Wno-deprecated-declarations -Wno-unused-function  -c partial_moments_rcpp.cpp -o partial_moments_rcpp.o
ccache g++ -std=gnu++11 -Wl,-S -shared -L/usr/lib/R/lib -Wl,-Bsymbolic-functions -flto=auto -ffat-lto-objects -flto=auto -Wl,-z,relro -o NNS.so RcppExports.o nns_rcpp.o partial_moments.o partial_moments_rcpp.o -L/usr/lib/R/lib -lR
installing to /tmp/rcpp/lib/00LOCK-NNS/00new/NNS/libs
** R
** byte-compile and prepare package for lazy loading
** help
*** installing help indices
** building package indices
** installing vignettes
** testing if installed package can be loaded from temporary location
** testing if installed package can be loaded from final location
** testing if installed package keeps a record of temporary installation path
* DONE (NNS)
edd@rob:/tmp/rcpp/NNS(NNS-Beta-Version)$ 

Thank you Dirk, yes, but CRAN is rejecting this src/Makevars configuration (CXX_STD = CXX14), as well as the following:

CXX_STD = CXX17 and removal (in src/Makevars and ensuring no mention in DESCRIPTION) altogether.

It was a Windows NOTE, as the Debian build was clean...

I wonder if in this case you could request that they let you keep CXX_STD=CXX11 until RcppParallel has moved to a new TBB library? (Also, as I recall, on Windows it is yet again a different matter as RcppParallel switches to tinythread (or at least it used to)). All this is a little complicated so you could also consider to bite the bullet and for a release or two to simply skip using RcppParallel until this is sorted out. Different options to consider...