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

Let's switch to C++11 #317

Closed
JohannesLorenz opened this issue Feb 15, 2014 · 32 comments
Closed

Let's switch to C++11 #317

JohannesLorenz opened this issue Feb 15, 2014 · 32 comments

Comments

@JohannesLorenz
Copy link
Contributor

It's coming anyways. The new ZynAddSubFX branch has a lot of C++11 code. And it brings a lot of advantages, even outside template programming. Just thinking about enum classes, default values for class members, auto keyword and the new for loop syntax.

I tried to compile LMMS with -std=c++11, and I got a few errors indeed. But I guess I could fix them all very easily.

I'd go ahead and prepare it all, if you allow. What do you think?

@lukas-w
Copy link
Member

lukas-w commented Feb 15, 2014

👍 +1, I'm not the one who may decide this though.

@diizy
Copy link
Contributor

diizy commented Feb 15, 2014

I thought we already had...

Yeah, I'm for this, no point in staying with old standards, technology marches on and all that.

Unless there's something I don't know that would cause problems, in which case someone's bound to point those out soon...

@pgiblock
Copy link
Contributor

On 2/15/14, 1:35 PM, Vesa V wrote:

I thought we already had...

Yeah, I'm for this, no point in staying with old standards, technology marches on and all that.

Unless there's something I don't know that would cause problems, in which case someone's bound to point those out soon...


Reply to this email directly or view it on GitHub:
#317 (comment)

Well, there is the fact that all of C++11 is still not widely
implemented and Qt doesn't use it. I don't see too much motivation
since many of the system abstractions are already offered in Qt.

Is there a feature that would be greatly simplified if implemented in C++11?

@JohannesLorenz
Copy link
Contributor Author

Well, there is the fact that all of C++11 is still not widely
implemented and Qt doesn't use it. I don't see too much motivation
since many of the system abstractions are already offered in Qt.

Is there a feature that would be greatly simplified if implemented in C++11?

Though Qt does much of our work, I guess that some useful features are useful
especially for us:

  • enum classes
  • constructor
  • range based for loops
  • type interference (auto etc)
  • static assertions
  • object construction improvement [1]

On the other side, you say you have no motivation, but what could the
disadvantages be? I can only think of these:

  • The time to port LMMS - it's very short (I'd like to do that)
  • We must check that old g++'es don't use the c++11 flag (since they don't
    know it) - CMake should be able to check the g++ version.

[1] http://en.wikipedia.org/wiki/C%2B%2B11#Object_construction_improvement

@tobydox
Copy link
Member

tobydox commented Feb 15, 2014

Yeah, C++11 is fine and the only possible show stopper is compatibility. It's not about adding the stdc++ flag or not because omitting it will still result in failing compilation with older versions of GCC. Nevertheless I think it would be no harm to start using widely supported C++11 features. Currently mainly GCC is used for building LMMS so we don't need to take care of VSC++ or similiar. There's a nice list of supported C++11 features in individual GCC releases at

http://gcc.gnu.org/projects/cxx0x.html

For the time being, we should support GCC downto at least 4.6. This means, don't make use of C++11 features supported by GCC 4.7 or 4.8 only. We can advance to 4.7 maybe within later this year. Requiring a newer version at this time should be a no-go in order to support Debian stable (4.7), Ubuntu LTS (GCC 4.6) etc.

@tobydox
Copy link
Member

tobydox commented Feb 15, 2014

@Vea: it's not about getting rid of old technologies but supporting Linux distributions whose last stable version has been released just 1 or 2 years ago.

@JohannesLorenz
Copy link
Contributor Author

Yeah, C++11 is fine and the only possible show stopper is compatibility.
It's not about adding the stdc++ flag or not because omitting it will still
result in failing compilation with older versions of GCC.

Ah yes, I forgot that. Is there anyone who has, randomly, gcc 4.6, to test
compiling our code every now and then? :)

The next days, I'll prepare a small patch to make our code C++11 ready. This
will not yet break any compatibility.

@lukas-w
Copy link
Member

lukas-w commented Feb 15, 2014

Is there anyone who has, randomly, gcc 4.6, to test
compiling our code every now and then? :)

Yes there is: Travis CI. ;)
Good guy Travis uses GCC 4.6.x, see
http://docs.travis-ci.com/user/ci-environment/

2014-02-15 23:39 GMT+01:00 JohannesLorenz [email protected]:

Yeah, C++11 is fine and the only possible show stopper is compatibility.
It's not about adding the stdc++ flag or not because omitting it will
still
result in failing compilation with older versions of GCC.

Ah yes, I forgot that. Is there anyone who has, randomly, gcc 4.6, to test
compiling our code every now and then? :)

The next days, I'll prepare a small patch to make our code C++11 ready.
This
will not yet break any compatibility.

Reply to this email directly or view it on GitHubhttps://issues/317#issuecomment-35170333
.

@pgiblock
Copy link
Contributor

This would be an on-going change. Do we really want to set milestones for this? Let's just say that we accept the subset of C++11 as mentioned by @tobydox. This case can be closed. And now people are free to write code with these features or to port existing code.

@tobydox tobydox closed this as completed Feb 20, 2014
@lukas-w
Copy link
Member

lukas-w commented Feb 20, 2014

No, this is not only an on-going change. Setting -std=c++11 breaks the build due to newly introduced warnings. These need to be fixed first.
Also, there is no point in closing this issue as long as -std=c++11 is not even added to the CMakeLists.txt.

@lukas-w lukas-w reopened this Feb 20, 2014
@JohannesLorenz
Copy link
Contributor Author

Am Donnerstag, 20. Februar 2014, 06:11:21 schrieb Lukas W:

No, this is not only an on-going change. Setting -std=c++11 breaks the
build due to newly introduced warnings. These need to be fixed first. Also,
there is no point in closing this issue as long as -std=c++11 is not even
added to the CMakeLists.txt.

Agreed to reopen. The switching is a short task which will/can be "completed".

As said, I'll make this patch in the next days. However, there should be no
need to have it for the next milestone/version. It is not that urgent ;)

@JohannesLorenz
Copy link
Contributor Author

I do not recall - who has updated the ZynAddSubFX code in our LMMS? It is not
compiling with C++11, though there were fixes made [1]. Why does our file look
different to that one in [1]?

[1]
http://sourceforge.net/p/zynaddsubfx/code/ci/master/tree/src/Effects/Alienwah.cpp

@lukas-w
Copy link
Member

lukas-w commented Feb 22, 2014

@JohannesLorenz
It was @tobydox. The changes have only been made in the stable-0.4-zynaddsubfx-update branch so far.

@JohannesLorenz
Copy link
Contributor Author

It was @tobydox. The changes have only been made in the
stable-0.4-zynaddsubfx-update branch so far.

I wonder why the zynaddsubfx update is still not done. Shouldn't it be made
for 1.0 ?

Btw: Is it possible to use a git submodule instead of copying the code? That
would sound way cleaner to me...

@diizy
Copy link
Contributor

diizy commented Mar 17, 2014

On 03/17/2014 09:54 PM, JohannesLorenz wrote:

It was @tobydox. The changes have only been made in the
stable-0.4-zynaddsubfx-update branch so far.

I wonder why the zynaddsubfx update is still not done. Shouldn't it be
made
for 1.0 ?

Btw: Is it possible to use a git submodule instead of copying the
code? That
would sound way cleaner to me...

It's already done, but it's in a separate branch and it won't be merged
for 1.0.0. Merging new stuff would inevitably cause new issues/bugs that
would have to be fixed, and we've delayed 1.0.0 enough as it is...

The new zyn version does use a submodule, btw.

@tobydox
Copy link
Member

tobydox commented Mar 17, 2014

Exactly :-)

@JohannesLorenz
Copy link
Contributor Author

I really need C++11 now. Reason: We will have to knob ctors, and one should
re-use the other (it will get extremely ugly without this).

The problem: C++11 code needs the new ZASF branch.

Is it good if I fork my C++11 branch from your "stable-0.4-zynaddsubfx-update"
branch? Or must I wait until the new ZASF is integrated into the stable
branch?

And if I can fork from ZASF: How to do it? "git checkout stable-0.4-
zynaddsubfx-update" does not work. Note: I have changes in my local LMMS
branch.

Thanks on advance.

@lukas-w
Copy link
Member

lukas-w commented Mar 22, 2014

The problem: C++11 code needs the new ZASF branch.

Could you explain this a bit more in detail? Why do you need the new ZASF update for using C++11?

@JohannesLorenz
Copy link
Contributor Author

Could you explain this a bit more in detail? Why do you need the new ZASF
update for using C++11?

The current one is not compilable. You get ugly compiler errors. They were
fixed by ZASF meanwhile. Alternatively, I could also fix it all "by hand", but
that does not seem to be a clean solution to me. I think working on the new
ZASF branch might be cleaner.

Btw: I did not get my own message, but your response to it. Is this typically
when you post to the issue tracker?

@diizy
Copy link
Contributor

diizy commented Mar 22, 2014

On 03/22/2014 01:49 PM, JohannesLorenz wrote:

Could you explain this a bit more in detail? Why do you need the new
ZASF
update for using C++11?

The current one is not compilable. You get ugly compiler errors. They
were
fixed by ZASF meanwhile. Alternatively, I could also fix it all "by
hand", but
that does not seem to be a clean solution to me. I think working on
the new
ZASF branch might be cleaner.

Then you could just wait until 1.0.0 is released (which was planned this
weekend) because after that the new ZASF will be merged to stable
branch, and you can then use that.

@pgiblock
Copy link
Contributor

Couldn't third party code such as ZASF and various libraries still be compiled with the older standard? I don't know about CMake, but this is easy to do in waf.

@lukas-w
Copy link
Member

lukas-w commented Mar 22, 2014

It can and should be compiled independently from LMMS itself. Changing such things as the used C standard should not change the building process of third party dependencies.

@JohannesLorenz
Copy link
Contributor Author

Then you could just wait until 1.0.0 is released (which was planned this
weekend) because after that the new ZASF will be merged to stable
branch, and you can then use that.

@toby: Can you please merge in the ZASF branch soon? Maybe this weekend? The
earlier we merge it,

  • the more early code can be written which depends on it
  • the more time we have for testing the new ZASF before it gets token in the
    next release.

Many thanks on advance.

@tobydox
Copy link
Member

tobydox commented Mar 27, 2014

Done, please checkout the master branch and don't forget to run "git submodule init" the first time and "git submodule update" later from time to time.

@JohannesLorenz
Copy link
Contributor Author

Done
It works, thanks. I have the patch complete now. Should the patch already
contain "-std=c++11" in the CMakeLists, or should we wait with that?

I guess since Ubuntu's next LTS comes in 3 weeks, most ppl would like to wait
until that. However, I'm not a big fan of delaying development for 3 weeks ;)

@eagles051387
Copy link

Why not push this to the master branch.
On 29 Mar 2014 16:29, "JohannesLorenz" [email protected] wrote:

Done
It works, thanks. I have the patch complete now. Should the patch already
contain "-std=c++11" in the CMakeLists, or should we wait with that?

I guess since Ubuntu's next LTS comes in 3 weeks, most ppl would like to
wait
until that. However, I'm not a big fan of delaying development for 3 weeks
;)

Reply to this email directly or view it on GitHubhttps://issues/317#issuecomment-38998765
.

@JohannesLorenz
Copy link
Contributor Author

Why not push this to the master branch.

Incompatibility. Ah wait. I think Toby said it must be supported by gcc 4.6.
Ah nvm, then I'll add the flag and make a pull request.

@eagles051387
Copy link

Remember 1.0 branch is for bug fixes. Master branch is for new features
such as this.
On 29 Mar 2014 16:34, "JohannesLorenz" [email protected] wrote:

Why not push this to the master branch.

Incompatibility. Ah wait. I think Toby said it must be supported by gcc
4.6.
Ah nvm, then I'll add the flag and make a pull request.

Reply to this email directly or view it on GitHubhttps://issues/317#issuecomment-38998930
.

@JohannesLorenz
Copy link
Contributor Author

Am Samstag, 29. März 2014, 08:39:50 schrieb eagles051387:

Remember 1.0 branch is for bug fixes. Master branch is for new features
such as this.

Oh, thanks. Please revoke the request, I'll make another one.

@JohannesLorenz
Copy link
Contributor Author

Am Samstag, 29. März 2014, 08:39:50 schrieb eagles051387:

Remember 1.0 branch is for bug fixes. Master branch is for new features
such as this.

Oh no, Travis CI even failed. The c++11 flag has a different name (c++1y) in
4.6. I wish we could switch to supporting only 4.7. Do we have any developers
with 4.6? ;)

@zonkmachine
Copy link
Member

Oh, thanks. Please revoke the request

You can close it yourself. There is an option under the comment box to close the issue/pull request.

@diizy
Copy link
Contributor

diizy commented Mar 29, 2014

On 03/29/2014 05:59 PM, JohannesLorenz wrote:

Am Samstag, 29. März 2014, 08:39:50 schrieb eagles051387:

Remember 1.0 branch is for bug fixes. Master branch is for new features
such as this.

Oh no, Travis CI even failed. The c++11 flag has a different name
(c++1y) in
4.6. I wish we could switch to supporting only 4.7. Do we have any
developers
with 4.6? ;)

I have both 4.6 and 4.8. But it's not only the developers we have to
think about. Packagers and users matter and are actually more important
here IMO. If developers can compile the software but packagers stop
packaging it and users stop using it, who does that benefit?

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

No branches or pull requests

7 participants