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

Compile Error - "const" vs "constexpr" #232

Closed
1 of 4 tasks
MaxWright opened this issue Jun 24, 2018 · 3 comments
Closed
1 of 4 tasks

Compile Error - "const" vs "constexpr" #232

MaxWright opened this issue Jun 24, 2018 · 3 comments
Labels
Milestone

Comments

@MaxWright
Copy link
Contributor

MaxWright commented Jun 24, 2018

What kind of issue is this?

  • Bug report
  • Feature request
  • Question not answered in documentation
  • Cleanup needed

What is affected by this?

Unable to compile the simulator as is using updated gcc packages due to syntax errors.

System Information
Ubuntu 18.04 LTS
GCC 7.3.0
GNU Make 4.1

How do we replicate the issue/how would it work?

Clone a fresh version and run make with a newest gcc compiler on a new machine. Five occurrences of const variables have to be changed to constexpr in order for the code to compile.

In AllSynapses.h
Change

static const BGFLOAT SYNAPSE_STRENGTH_ADJUSTMENT = 1.0e-8;

-to-

static constexpr BGFLOAT SYNAPSE_STRENGTH_ADJUSTMENT = 1.0e-8;

In AllZHNeurons.h
Change

        /**
         *  Default value of Aconst.
         */
        static const BGFLOAT DEFAULT_a = 0.0035;

        /**
         *  Default value of Bconst.
         */
        static const BGFLOAT DEFAULT_b = 0.2;

        /**
         *  Default value of Cconst.
         */
        static const BGFLOAT DEFAULT_c = -50;

        /**
         *  Default value of Dconst.
         */
        static const BGFLOAT DEFAULT_d = 2;

-to-

        /**
         *  Default value of Aconst.
         */
        static constexpr BGFLOAT DEFAULT_a = 0.0035;

        /**
         *  Default value of Bconst.
         */
        static constexpr BGFLOAT DEFAULT_b = 0.2;

        /**
         *  Default value of Cconst.
         */
        static constexpr BGFLOAT DEFAULT_c = -50;

        /**
         *  Default value of Dconst.
         */
        static constexpr BGFLOAT DEFAULT_d = 2;

Expected behavior (i.e. solution or outline of what it would look like)

Running make or make growth succsesfully compiles.

Other Comments

In the research I did, it seems to be a difference in standards between c++98 and c++11. There is a fix in stack overflow involving macros by detecting the GNU compiler version. I tried to force make to compile at an older version of the GCC and to the c++98 standard to no avail. In the end I changed the syntax and it compiled succsesfully.

error: 'constexpr' needed for in-class initialzation of static data member 'const float AllSynapses::SYNAPSE_STRENGTH_ADJUSTMENT' of non-integral type [-fpermissive]
        static const BGFLOAT SYNAPSE_STRENGTH_ADJUSTMENT = 1.0e-8;
@MaxWright
Copy link
Contributor Author

Makefile Flag Test

-std=c++98
Does compile growth with a good number of warnings.
Did not test make growth_cuda
This is a 20 year old standard... I don't know if I would calll this a fix.

-std=c++11 & -std=c++11 -pedantic
Fail, same error

@stiber
Copy link
Contributor

stiber commented Jul 2, 2018

I think that the age of the C++ 1998 standard isn't really a problem. Actually, as far as I can think of right now, this is just about the only situation in which there was a subsequent change that breaks perfectly good earlier code. Most other changes are either "nice additions" or "extensions that people have differences of opinion on" (a good example of the latter being the auto specifier in variable declarations, which some people feel is good and others feel allows programmers to ignore issues associated with variable types that can bite them later).

I'm not 100% up to speed on constexpr vs. const, but it appears to me that constexpr is intended for much more complex situations than we are involved with. In this particular case, I think we need to understand what's going on first and then decide how we want to change our code.

The warnings have been there for a while; they are in the backlog, associated with improving code quality.

@stiber stiber added this to the Summer 2018 milestone Jul 6, 2018
@stiber
Copy link
Contributor

stiber commented Jul 11, 2018

OK, I am going to "fix" this by adding -std=c++98 to CXXFLAGS in the Makefile. I will also create a new issue to go through the code and bring it into compliance with C++11 as a general policy (I'll refer to this issue in that one).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants