eddelbuettel / bh

R package providing Boost Header files
85 stars 32 forks source link

warning: ISO C++ prohibits anonymous structs [-Wpedantic] #56

Closed privefl closed 6 years ago

privefl commented 6 years ago

Some packages on CRAN that use {BH} (e.g. {bigmemory}) get some warnings for the devel version on windows (see these CRAN checks). Some packages get rejected by CRAN because of this.

Found the following significant warnings:
     d:/RCompile/CRANpkg/lib/3.5/BH/include/boost/interprocess/detail/win32_api.hpp:145:9: warning: ISO C++ prohibits anonymous structs [-Wpedantic]
     d:/RCompile/CRANpkg/lib/3.5/BH/include/boost/interprocess/detail/win32_api.hpp:153:9: warning: ISO C++ prohibits anonymous structs [-Wpedantic]
     d:/RCompile/CRANpkg/lib/3.5/BH/include/boost/interprocess/detail/win32_api.hpp:178:13: warning: ISO C++ prohibits anonymous structs [-Wpedantic]
     d:/RCompile/CRANpkg/lib/3.5/BH/include/boost/interprocess/detail/win32_api.hpp:180:7: warning: ISO C++ prohibits anonymous structs [-Wpedantic] 

In a recent post (http://dirk.eddelbuettel.com/blog/2018/02/13/#bh_1.66.0-1), you seem to talk about this problem, but I'm not sure how to fix this. Can you please help us figure out how to fix these warnings?

eddelbuettel commented 6 years ago

If warning-as-error is set, and if pedantic warnings are set, then there is nothing you can do. So the key is to not set those. I sometimes add various W-no-.... options in my ~/.R/Makevars to silence this.

Also, I have no control or influence over CRAN.

And I did not talk about the above, but rather the diffs I had to make -- see the patches dir.

As this is in fact an upstream issue, did you check the Boost repos or maybe StackOverflow?.

privefl commented 6 years ago

I did, yet my understanding of C++ is only moderate. What I find difficult to understand is why package {BH} doesn't have this warning, while packages using {BH} have it.

eddelbuettel commented 6 years ago

Look at how much compilation BH does when it installs -- none.

(This is similar for Rcpp, it ships more code than it uses.)

privefl commented 6 years ago

Understood. So basically, we can't do anything about this warning? Just trying to justify it in CRAN comments?

Thanks

eddelbuettel commented 6 years ago

Boost is a lot of code, and some of it stretches (old) compilers quite a bit -- so there are always a lot warnings. I sometimes upgrade BH to try to get of rid of them but as you see, there is relatively little we can do.

privefl commented 6 years ago

We managed to silent this warning. Please see https://github.com/boostorg/interprocess/issues/59.

Could you please add this change to this package? It would be useful for packages using memory-mapping such as bigmemory to facilitate their submission to CRAN. You would just need to change https://github.com/eddelbuettel/bh/blob/master/inst/include/boost/interprocess/detail/win32_api.hpp#L106-L110 by https://github.com/boostorg/interprocess/blob/develop/include/boost/interprocess/detail/win32_api.hpp#L106-L110

eddelbuettel commented 6 years ago

Well -- BH has hundreds of thousands of lines of code. I am not really prepared to add on-request between-release changes. Can you ensure this will get into the next Boost release 1.69?

But kudos for getting an upstream fix in. That is the right approach.

privefl commented 6 years ago

Seems OK for Boost release 1.69 (see https://github.com/boostorg/interprocess/issues/59#issuecomment-412580456).

eddelbuettel commented 6 years ago

Can we keep this closed It is an issue that is purely upstream, and it has been addressed upstream.

eddelbuettel commented 6 years ago

Thanks Florian.

privefl commented 6 years ago

Sorry for bothering you again with this.

Have you an idea of when a new version of BH will come to CRAN?

Thanks

eddelbuettel commented 6 years ago

I do not.