Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove const qualifier so it can be built with FCC #755

Merged
merged 2 commits into from Jun 16, 2017

Conversation

@ikitayama
Copy link
Contributor

@ikitayama ikitayama commented Jun 16, 2017

As of today the HEAD can not be built with FCC on the frontend for K computer. Please review and pull the patch into master.

@heplesser
Copy link
Contributor

@heplesser heplesser commented Jun 16, 2017

@ikitayama Thank you for reporting this issue. The root of the problem is a inconsistency in the C++98 standard. Probably due to an oversight, basic_ofstream::is_open() is not a const method in the C++98 standard (§27.8.1.10), even though the underlying basic_filebuf::is_open() method is const (§27.8.1.3). In C++11 and later, is_open() is const also for basic_ofstream. It appears that the FCC compiler is the only one adhering strictly to the C++98 standard here.

Copy link
Contributor

@heplesser heplesser left a comment

@ikitayama Thank you for your pull request. May I kindly ask you to add a comment explaining the background for this change (see my inline comment).

@@ -538,7 +538,7 @@ class RecordingDevice : public Device
//! Store current values in dictionary
void get( const RecordingDevice&, DictionaryDatum& ) const;
//! Set values from dictionary
void set( const RecordingDevice&, const Buffers_&, const DictionaryDatum& );
void set( const RecordingDevice&, Buffers_&, const DictionaryDatum& );

This comment has been minimized.

@heplesser

heplesser Jun 16, 2017
Contributor

Please add the following comment here:

/* Note: `Buffers_&` cannot be `const` because `basic_ofstream::is_open()`
    is not `const` in C++98  (cf C++ Standard §27.8.1.10).
*/

This comment has been minimized.

@ikitayama

ikitayama Jun 16, 2017
Author Contributor

@heplesser please go ahead and add that on top my patch.

@heplesser heplesser merged commit 372f485 into nest:master Jun 16, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.