Problem 1548 - Invalid code in G4INCLInverseInterpolationTable.{hh,cc}, happens to compile on Mac and Linux
Summary: Invalid code in G4INCLInverseInterpolationTable.{hh,cc}, happens to compile o...
Status: RESOLVED FIXED
Alias: None
Product: Geant4
Classification: Unclassified
Component: processes/hadronic/models/incl (show other problems)
Version: 9.6
Hardware: All All
: P5 minor
Assignee: Davide Mancusi
URL:
Depends on:
Blocks:
 
Reported: 2013-12-19 22:14 CET by Tom Roberts
Modified: 2014-01-06 11:56 CET (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this problem.
Description Tom Roberts 2013-12-19 22:14:42 CET
G4INCLInverseInterpolationTable.hh has this invalid code on line 85:
 friend G4bool operator<(const InterpolationNode &lhs, const G4double rhs) {

This is wrong in several ways: "friend" is just plain wrong on a function definition inside a class declaration, it has an extra argument (lhs), it's not a const function, and std::lower_bound() won't use this function (as was apparently intended). Delete this entire function, as the correct operator< is defined earlier in the file.

G4INCLInverseInterpolationTable.cc has this invalid code on line 120:
    std::lower_bound(nodes.begin(), nodes.end(), x);
This function requires an InterpolationNode as the 3rd argument, not a double. change it to:
    std::lower_bound(nodes.begin(), nodes.end(), InterpolationNode(x,0.0,0.0));
A better resolution would probably be to add a new constructor InterpolationNode(G4double), and use it instead.

It's remarkable that these two errors conspire to compile on Linux and Mac (without even a warning on the Mac). They don't compile on Windows.

The above changes get it to compile on Windows and Mac; I have not yet compiled it on Linux, and have not tested it (I have no reason to believe it is incorrect or won't work).

This is Geant4.9.6.p02.

Perhaps you should add a Windows machine to your test suite.
Comment 1 Pekka Kaitaniemi 2013-12-29 03:31:40 CET
Thanks for the bug report! As I'm no longer a member of the Geant4 collaboration I'm reassigning the bug to the current INCL maintainer Davide Mancusi.
Comment 2 Davide Mancusi 2014-01-06 11:56:24 CET
Thank you for your report. The issues you mentioned have already been fixed in Geant4 v10.0. Therefore, I'm closing this bug report.

Let me anyway make some specific comments. First, you are totally right about the definition of operator<. What was meant, of course, was to declare an external friend function, to be defined elsewhere. This is of no consequence, however, and I believe it compiles smoothly on all the platforms where this was tested.

Concerning the use of lower_bound, the C++ standard does *not* mandate that the third argument should be of the same type as what you get when you dereference the iterator. This used to be the case in C++98, but C++03 amended this (see http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-defects.html#270). Nevertheless, I am aware that VC9 chokes on this code snippet. Since VC10 does not, I think it's a bug in the VC9 standard library. The code in Geant4 v10.0 was anyway modified to be similar to your suggestion, so that it compiles with VC9 and VC10.

For your information, we have at least five different Windows compilers in out test suite.