Minimum compilation standard C++11 or C++14 needed with g++-11

[eddelbuettel]

g++ version 11 defaults to C++17 as a compilation standard. And C++17 introduces std::data() which clashes with

rcppsmc/inst/include/LinReg.h

Lines 33 to 39 in ff4d2c6

	class rad_obs
	{
	public:
	arma::vec y, x;
	};
	rad_obs data;

and similarly in LinReg_LA.h and LinReg_LA_adapt.h. The simplest fix is to just (re- ?) introduce CXX_STD=CXX11 (or equally CXX14) in src/Makevars and src/Makevars.win.

I can this in a simple commit assuming that is ok with everybody. We could also rename our data to data_ or something else but that may be more involved. Thoughts?

[ilyaZar]

Just made a local branch and renamed all instances of data to dataLinReg and checked with an updated g++11 that it works on my platform.

It's not that involved, though, but actually the files LinReg.h and all three .cpp pendants are affected by the change.

I am ready to make the PR if we decide to change the qualifier data; no particular opinion on that, but maybe it's better to get rid of the name clash sooner than later.

Also sorry for being absent, I will come back to this project asap, there were some other urgent things and the weeks went by in a flash...

[eddelbuettel]

No strong feelings either way. The PR is more work but now that you have done it we may as well use it. Though dataLinReg is a slightly clumsy now...

The real way to fix it, of course, is to not use using namespace std; so that our data cannot get confused. But that is even more work I am not suggesting you need to do that. "One day" we all should...

[adamjohansen]

Personally, I weakly favour changing a variable name over imposing otherwise unnecessary restrictions on compiler versions given the potential for future headaches. dataLinReg is a shade clumsy, I agree.

Thanks, @ilyaZar, for having the energy to actually do something. No need to apologise for being busy, I'm sure we've all had similar issues and I know I haven't managed to do anything related to this project recently -- it would certainly be good get things tidied up if we can find some time between us.

[eddelbuettel]

(Just a nit on wording: we do not "impose restrictions" on compilers; we defend ourselves against our own sloppyness by asking newer compilers to use C++11 or C++14. Which may all be my fault as we IIRC once had CXX_STD=CXX11 which I removed when that became the minimum. But such markers can be accessed from below (asking older compilers to jump ahead to C++11) and above (newer ones to 'only' use C++11). The fact is that our using namespave std; is a smell, and C++17 finally bites us on that _in combination with naming a variable data.)

But it's all a mountain/molehill problem. Any fix will do.

[adamjohansen]

Fair point. Still, asking for a specific compiler standard to address a detail like this didn't seem to be an optimal solution. I agree that using namespace std is really what we do /wrong/, but as you say fixing that probably isn't quite so quick.

[LeahPrice]

Thanks Dirk for bringing this up and Ilya for making the change. I'm happy with any fix!

[eddelbuettel]

Valid points, @conradsnicta, but parts of me still thinks that beyond the (current) "band aid" of adjusting / constraining the C++ dialect we should ultimately do the deeper work of compiling with any standard and drop the 'not-really-recommended-in-a-library' practice of using namespace std;. To be continued...

[eddelbuettel]

Turns out that by simply removing using namespace std; from those three files we can build with g++-11.

[adamjohansen]

Nice. Well spotted.
I'm happy to defer to those who know more than me about the right way to deal with versions. There may be good reasons to prefer a particular standard specification (my naive preference for just building okay with as broad a range as possible is largely based on antediluvian experiences with the difference between standards and implementations which are probably totally irrelevant in the modern world) and I don't feel strongly one way or the other, it just doesn't seem like the right thing to do to avoid a namespace conflict that can be easily fixed by more direct means.

[ilyaZar]

Thanks @eddelbuettel ! Just did the same fix as you but enforced C++ 17 via Makevars setting CXX_STD=CXX17 (just in case our preference is for not setting the compiler flag to C++11 or 14)

Output:

> Executing task: R CMD INSTALL --preclean --no-multiarch --with-keep.source . <

* installing to library ‘/home/iz/Dropbox/libraries/R/4.0’
* installing *source* package ‘RcppSMC’ ...
** using staged installation
** libs
g++ -std=gnu++17 -I"/usr/share/R/include" -DNDEBUG -I../inst/include -I'/home/iz/Dropbox/libraries/R/4.0/Rcpp/include' -I'/home/iz/Dropbox/libraries/R/4.0/RcppArmadillo/include'    -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-i2PIHO/r-base-4.1.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c LinReg.cpp -o LinReg.o
g++ -std=gnu++17 -I"/usr/share/R/include" -DNDEBUG -I../inst/include -I'/home/iz/Dropbox/libraries/R/4.0/Rcpp/include' -I'/home/iz/Dropbox/libraries/R/4.0/RcppArmadillo/include'    -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-i2PIHO/r-base-4.1.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c LinReg_LA.cpp -o LinReg_LA.o
g++ -std=gnu++17 -I"/usr/share/R/include" -DNDEBUG -I../inst/include -I'/home/iz/Dropbox/libraries/R/4.0/Rcpp/include' -I'/home/iz/Dropbox/libraries/R/4.0/RcppArmadillo/include'    -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-i2PIHO/r-base-4.1.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c LinReg_LA_adapt.cpp -o LinReg_LA_adapt.o
g++ -std=gnu++17 -I"/usr/share/R/include" -DNDEBUG -I../inst/include -I'/home/iz/Dropbox/libraries/R/4.0/Rcpp/include' -I'/home/iz/Dropbox/libraries/R/4.0/RcppArmadillo/include'    -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-i2PIHO/r-base-4.1.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c RcppExports.cpp -o RcppExports.o
g++ -std=gnu++17 -I"/usr/share/R/include" -DNDEBUG -I../inst/include -I'/home/iz/Dropbox/libraries/R/4.0/Rcpp/include' -I'/home/iz/Dropbox/libraries/R/4.0/RcppArmadillo/include'    -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-i2PIHO/r-base-4.1.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c blockpfgaussianopt.cpp -o blockpfgaussianopt.o
g++ -std=gnu++17 -I"/usr/share/R/include" -DNDEBUG -I../inst/include -I'/home/iz/Dropbox/libraries/R/4.0/Rcpp/include' -I'/home/iz/Dropbox/libraries/R/4.0/RcppArmadillo/include'    -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-i2PIHO/r-base-4.1.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c nonLinPMMH.cpp -o nonLinPMMH.o
g++ -std=gnu++17 -I"/usr/share/R/include" -DNDEBUG -I../inst/include -I'/home/iz/Dropbox/libraries/R/4.0/Rcpp/include' -I'/home/iz/Dropbox/libraries/R/4.0/RcppArmadillo/include'    -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-i2PIHO/r-base-4.1.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c pflineart.cpp -o pflineart.o
g++ -std=gnu++17 -I"/usr/share/R/include" -DNDEBUG -I../inst/include -I'/home/iz/Dropbox/libraries/R/4.0/Rcpp/include' -I'/home/iz/Dropbox/libraries/R/4.0/RcppArmadillo/include'    -fpic  -g -O2 -fdebug-prefix-map=/build/r-base-i2PIHO/r-base-4.1.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -c pfnonlinbs.cpp -o pfnonlinbs.o
g++ -std=gnu++17 -shared -L/usr/lib/R/lib -Wl,-Bsymbolic-functions -Wl,-z,relro -o RcppSMC.so LinReg.o LinReg_LA.o LinReg_LA_adapt.o RcppExports.o blockpfgaussianopt.o nonLinPMMH.o pflineart.o pfnonlinbs.o -llapack -lblas -lgfortran -lm -lquadmath -L/usr/lib/R/lib -lR
installing to /home/iz/Dropbox/libraries/R/4.0/00LOCK-rcppsmc/00new/RcppSMC/libs
** R
** data
*** moving datasets to lazyload DB
** inst
** byte-compile and prepare package for lazy loading
** help
*** installing help indices
** building package indices
** testing if installed package can be loaded from temporary location
** checking absolute paths in shared objects and dynamic libraries
** testing if installed package can be loaded from final location
** testing if installed package keeps a record of temporary installation path
* DONE (RcppSMC)

Terminal will be reused by tasks, press any key to close it.

[eddelbuettel]

our preference is for not setting the compiler flag

There are different ways to skin this cat, but my weak overall preference is to not constrain and not set a standard unless we have to. Setting CXX_STD=CXX17 and using g++-11 is a null-op as is setting CXX_14 with g++-10 and so on -- these are their defaults. Over all these years working with the compiler defaults has generally been a good experience. This isssue here (where we have data object clashing with std::data) is rare and generally avoided by using explicit namespaces as we all learned years ago with abs() vs fabs() vs std::abs() (where only the latter was properly overloaded).