Attempt to support building on C++98 compliant compilers (XSPEC code) #892
Conversation
The std::begin/end templates, and the use of lambda expressions requite C++11. Rather than try to come up with a C++98 version of std::transform, I just went back and used the old C-style loop. The contents of the macro is based on a check against __cplusplus, which is not universally defined but is hopefully standard on the platforms we are targeting. Rather than picking a potential version, which could be confusing, I trigger on whether the value is larger than the C++98 standard or not.
Requested by @dtnguyen2.
|
I cancelled the first set of tests as I wanted to get the update requested by @dtnguyen2 in. |
|
@Marie-Terrell I hope this will work for you (and that the Travis tests pass too...). |
Codecov Report
@@ Coverage Diff @@
## master #892 +/- ##
=======================================
Coverage 62.98% 62.98%
=======================================
Files 70 70
Lines 24321 24321
Branches 3510 3510
=======================================
Hits 15319 15319
Misses 7863 7863
Partials 1139 1139 Continue to review full report at Codecov.
|
|
It's not just Marie. I just tried to build the master branch on my work machine with CentOS7 and failed because of the default gcc version installed there. |
|
So more testers then: awesome! |
|
Works on mt machine (i.e. it compiles again). |
|
@dtnguyen2 - just to note I made the change you requested so it can be re-reviewed. |
|
Just ran a quick test on two of our machines and it does seem to fix the build errors. Though, I did not run any checks besides making sure the build completed. |
| [](const double val) -> FloatArrayType { return static_cast<FloatArrayType>(val); }); | ||
| #else | ||
| #define CONVERTARRAY(orig, out, npts) \ | ||
| for (int i = 0; i < npts; i++) { \ |
There was a problem hiding this comment.
when possible, you should avoid using single character variable names- int i, float f, etc... Using indexes such as int ii, float ff improves readability and searching (ii will generate fewer false hits than i).
Also, neglecting optimization and loop unrolling, reversing indexing for simple independent assignments
for (int ii = npts; ii--; ) { }
will generally provide more efficient code than
for (int ii = 0; ii < npts; ii++) { }
when compiled the same way.
There was a problem hiding this comment.
@wmclaugh - all I did here was resurrect the old code (the for loop) that we had before #859 was merged. I didn't want to spend time complicating things with changes to code that we knew already worked (and that we can hopefully remove soon). If someone wants to fix up this PR to make these changes then they are more than welcome to, but I feel I've done enough with this patch as is.
Summary
Support building the XSPEC interface using compilers which default to the C++98 standard.
Details
The std::begin/end templates, and the use of lambda expressions
requite C++11. Rather than try to come up with a C++98 version of
std::transform, I just went back and used the old C-style loop.
The contents of the macro is based on a check against __cplusplus,
which is not universally defined but is hopefully standard on the
platforms we are targeting. Rather than picking a potential version,
which could be confusing, I trigger on whether the value is larger
than the C++98 standard or not (a simplification of the code given
in #859 (comment) )
This should not be needed, as we should be requiring/sending -std=c++11 but there appears to be issues somewhere.
Unfortunately this code is not exercised on the Travis infrastructure (the compilers seem to be new enough that they are defaulting to the C++11 standard).