Adds compiler warning message (closes #308) #515
Conversation
Modifies /Rcpp/platform/compiler.h to trigger a pragma message indicating if gcc <= 4.6.2
|
We will get that @jjallaire What is the informal census on proportion of systems out there still running old RHEL/CentOS? How many of these are likely to be using Rcpp (maybe via tidyverse) ? |
|
I'm thinking that the only way to do this is going to be via Though, it may be a bit more appropriate to alert the user at the time of compilation that the compiler is problematic. What if the alert just has only one line? #pragma message ("WARNING: GCC Compiler Version <= 4.6.2")I say this because sometimes it is just the sysadmin or an inexperience user who might not catch the warning in the sea of compile messages if it is only on package install. |
|
May be converging:
|
|
@eddelbuettel Bad url for the RcppArmadillo line. I think you are referring to: ## If it is g++, we have GXX set so let's examine it
if test "${GXX}" = yes; then
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether g++ version is sufficient" >&5
$as_echo_n "checking whether g++ version is sufficient... " >&6; }
gxx_version=$(${CXX} -v 2>&1 | awk '/^.*g.. version/ {print $3}')
case ${gxx_version} in
1.*|2.*|3.*|4.0.*|4.1.*|4.2.*|4.3.*|4.4.*|4.5.*)
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
$as_echo "no" >&6; }
{ $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: Only g++ version 4.6 or greater can be used with RcppArmadillo." >&5
$as_echo "$as_me: WARNING: Only g++ version 4.6 or greater can be used with RcppArmadillo." >&2;}
as_fn_error $? "Please use a different compiler." "$LINENO" 5
;;
4.6.*|4.7.*|4.8.*|4.9.*|5.*|6.*)
gxx_newer_than_45="-fpermissive"
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: (${gxx_version}) yes" >&5
$as_echo "(${gxx_version}) yes" >&6; }
;;
esac
fihttps://github.com/RcppCore/RcppArmadillo/blob/master/configure#L2700-L2719 That is a heck of a lot of code that is powering the |
|
@coatless My bad. Updated the URL. Still had the region highlighted in another tab. In general we always look at |
|
To confirm, the plan is to write a #pragma message ("WARNING: GCC Compiler Version <= 4.6.2")Sound about right? If so, I'll update this PR on Friday. |
|
Yes, let's refine. I see two things. First:
|
|
On a CentOS 6.8 VM: |
|
Maybe we can guess what people are using according to release dates (https://access.redhat.com/articles/3078). Default compilers on different version are list below: |
|
Good list. We should essentially just tell anybody who is on RHEL 6 or older to bag it and go home. Sadly, we can't quite ... |
|
Okay, per a little bit of discovery...
Discussion of the behavior of
In: https://lists.gnu.org/archive/html/autoconf/2014-09/msg00027.html As a result, the fake define throws off the compiler specific detects in To view this in practice, note that within the checking whether g++ version is sufficient... checking LAPACK_LIBS... fallback LAPACK from R 3.3.0 or later used
configure: creating ./config.status
config.status: creating inst/include/RcppArmadilloLapack.hOne way to solve this is to mimic llvm's AC_PROG_CC(clang llvm-gcc gcc)
AC_PROG_CXX(clang++ llvm-g++ g++)
AC_PROG_CPP
dnl If CXX is Clang, check that it can find and parse C++ standard library
dnl headers.
if test "$CXX" = "clang++" ; thenTo avoid this problem cropping up in the Also, to ease worries about noise, I've added in a |
…t down to a single `if` statement that triggers error if improperly defined. Added a better trigger for poor GCC version support (tries to detect a fake GCC flag and accommodate it).
…lower than a given threshold. Note: The GCC check is misleading as it will pick up other compilers that "show" GCC defines. Thus, `clang` was picked up causing the absence of a `gcc` version.
| #pragma message ("WARNING: if something does not work, you should upgrade your compiler!") | ||
| #if GCC_VERSION <= 40602 && !defined(RCPP_GCC_COMPILER_WARNING_SILENT) | ||
| // Ensure not fake GCC flag set by clang or intel. | ||
| #if !(defined(__clang__) || defined(__INTEL_COMPILER)) |
There was a problem hiding this comment.
I guess these do not hurt. I would have hope the earlier #ifdef __GNUC__ would be enough. Maybe not.
There was a problem hiding this comment.
The reason why I added in the detect genuine GCC is due to the warning being triggered on macOS due to GCC headers being identified as 4.2.1
There was a problem hiding this comment.
Are we saying that anyone running RHEL 6 is going to see this warning when
compiling Rcpp? That I think is far too aggressive as this describes the
vast majority of Linux systems deployed in larger organizations. Worse yet
is that the end users who see this message during compilation have no way
to update the compiler (that's handled by the sysadmin on a multi-user
system, and compiler upgrades are considered extremely heavyweight/risky so
are basically not done).
On Sat, Jul 30, 2016 at 10:13 PM, James J Balamuta <notifications@github.com
wrote:
In inst/include/Rcpp/platform/compiler.h
#515 (comment):@@ -57,10 +44,11 @@
#define IS_GCC_460_OR_LATER
#endif
// Inform users of dated compiler (e.g. <= RHEL6 )
- #if GCC_VERSION <= 40602
#pragma message ("WARNING: GCC Compiler Version <= 4.6.2")#pragma message ("WARNING: The compiler being used is both OUTDATED and INCOMPLETE in terms of C++ standards;")#pragma message ("WARNING: if something does not work, you should upgrade your compiler!")- #if GCC_VERSION <= 40602 && !defined(RCPP_GCC_COMPILER_WARNING_SILENT)
// Ensure not fake GCC flag set by clang or intel.#if !(defined(**clang**) || defined(__INTEL_COMPILER))The reason why I added in the detect genuine GCC is due to the warning
being triggered on macOS due to GCC headers being identified as 4.2.1—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/RcppCore/Rcpp/pull/515/files/c5f7b40770c14f8efb12f2f5e77e7aa5d17e0d0d..6b292afd4f11c5f9f95abd830f5cb383a194c3de#r72896929,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGXx8hmEGS19KROXcv6n0iq4lmyLblbks5qbASvgaJpZM4JTRmr
.
There was a problem hiding this comment.
Yes that is the current discussion but I held off on merging as I wanted to hear from you.
|
This now has too. See discussion in #528. Can merge manually if you want me to. |
…message Fixes merge conflicts in: - ChangeLog - inst/NEWS.Rd
|
Let me call out to @jjallaire one last time ... JJ: any view as to with all the RHEL systems out there we are too aggressive? A one-liner message seems fair, particularly with a |
| @@ -56,6 +43,13 @@ | |||
| #if GCC_VERSION >= 40600 | |||
| #define IS_GCC_460_OR_LATER | |||
| #endif | |||
| // Inform users of dated compiler (e.g. <= RHEL6 ) | |||
There was a problem hiding this comment.
Per my other comment, I don't think we can do this. We have 700+ packages on CRAN all of whom will be subjected to this warning (and all of whom will have to field enquiries about the warning from users and sysadmins worried about it's implications). If we believe that GCC <= 4.6.2 truly warrants a warning message then we need to change our code so it doesn't.
There was a problem hiding this comment.
(I should add that RcppArmadillo now actually fails when this happens, but then again Conrad left me no choice. It has 246 rev.deps and mayhem may well ensue ...)
There was a problem hiding this comment.
Right, your hand was forced there.
I should note that I am not at all happy about the state of affairs with
RHEL 6 and we are looking at creating a new binary distribution of R for
RHEL 6 that uses clang. Until then though I don't think we can cry wolf
when being compiled on RHEL 6, it's just too mainstream a platform.
On Tue, Aug 2, 2016 at 4:50 PM, Dirk Eddelbuettel notifications@github.com
wrote:
In inst/include/Rcpp/platform/compiler.h
#515 (comment):@@ -56,6 +43,13 @@
#if GCC_VERSION >= 40600
#define IS_GCC_460_OR_LATER
#endif
- // Inform users of dated compiler (e.g. <= RHEL6 )
(I should add that RcppArmadillo now actually fails when this happens, but
then again Conrad left me no choice. It has 246 rev.deps and mayhem may
well ensue ...)—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/RcppCore/Rcpp/pull/515/files/4bc57824b83491162a38e18fc3ca68d5e5ec3c41#r73234667,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGXx3FxbIiLaRmRSUxXk1fKSR09SYkFks5qb62agaJpZM4JTRmr
.
There was a problem hiding this comment.
Throw it out the window... The window... We'll throw it out the window... Err, I'm removing it.
Do we still want to proceed in throwing a warning via: configure.ac? @eddelbuettel @jjallaire
There was a problem hiding this comment.
I think we could, and should -- but if @jjallaire vetos it I will not object.
|
I don't think we should throw a warning of any kind right now because we are in fact fully compatible with GCC 4.6 (and if we aren't compatible we should be). Again, I would like us to get beyond this by having a RHEL 6 compatible R distribution with a more modern compiler but we aren't there yet. |
|
Thanks @jjallaire ! @eddelbuettel verdict decreed. Please close out issue #308. |
Modifies /Rcpp/platform/compiler.h to trigger a pragma message
indicating if gcc <= 4.6.2 to satisfy #308
Part of #506