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

Partial c++11 support #2490

Merged
merged 6 commits into from Oct 3, 2020
Merged

Partial c++11 support #2490

merged 6 commits into from Oct 3, 2020

Conversation

@criezy
Copy link
Member

@criezy criezy commented Oct 3, 2020

What I am proposing in with this pull request is to:

  • Allow any engine to use c++11
  • Allow any backend to use c++11
  • Restrict c++11 in common code to features that can be disabled

The idea is that I would like to avoid breaking compilation for ports that do not yet support c++11, while allowing more c++11 use in the project.

The way I went about this is:

  1. Add a cxx11 feature that can be used as a dependency by engines that use c++11. This will automatically disable those engines when c++11 is not enabled. This is added for the petka engine, and should fix the compilation on buildbot for the canoo, dingux, gp2xwiz, openpandora, and osx_intel builders.
  2. Add a USE_CXX11 define when C++11 is enabled. This can be used in common code to add code such as constructors with the move semantics or intializer list. I added those to the Array class as an example.
  3. Add check in configure, and replacement in common for std::initializer_list when not supported by the standard library (as is the case when compiling for Mac OS X 10.6 on a more recent system (where the compiler supports c++11, but the libstdc++ used when targeting those system does not - Apple switched to libc++ in Mac OS X 10.9, and only that one fully support c++11).

Feedback on both the proposal and the actual commits in this PR are welcome.
Would you for example prefer is we stopped supporting non-c++11 builds and just always enabled c++11 everywhere in the project? And do you see an issue with the commits in this PR?

@henke37
Copy link
Contributor

@henke37 henke37 commented Oct 3, 2020

So much for my plan to add move semantics to the custom containers.

@criezy
Copy link
Member Author

@criezy criezy commented Oct 3, 2020

So much for my plan to add move semantics to the custom containers.

You are welcome to add more. I only added it to Common::Array ;-)

@henke37
Copy link
Contributor

@henke37 henke37 commented Oct 3, 2020

Been there, done that. It's a branch on my fork.

@sev-
Copy link
Member

@sev- sev- commented Oct 3, 2020

Yes, this is a great improvement, thank you!

Of course, now it cannot apply cleanly.

I would like to make c++11 as a must-have feature, and allow it in the common code as well. Currently, this will affect only intel port, the other Dingux-based ports are dead anyway.

criezy added 6 commits Oct 3, 2020
The idea is that engines that want to use c++11 can specify it as
a dependency in their configure.engine file so that the engine
is automatically disabled when c++11 is not available.
…std lib

The c++11 standard includes some features that do not only depend on the
compiler supporting it, but also the c++ standard library having support
for it. This is the case for the initializer list feature. So when we
compile with a modern compiler but using an old std lib we need a
replacement for it.
@criezy criezy force-pushed the criezy:cpp11 branch from 9c9c121 to bd55545 Oct 3, 2020
@deepcode-ci-bot
Copy link

@deepcode-ci-bot deepcode-ci-bot bot commented Oct 3, 2020

DeepCode's analysis on #bd5554 found:

  • ⚠️ 1 warning 👇

Top issues

Description Example fixes
Duplicate expressions on both sides of a binary operator is probably a mistake. Occurrences: 🔧 Example fixes

👉 View analysis in DeepCode’s Dashboard | Configure the bot

@criezy
Copy link
Member Author

@criezy criezy commented Oct 3, 2020

I just rebased it and it can be merged again.

@criezy
Copy link
Member Author

@criezy criezy commented Oct 3, 2020

I would like to make c++11 as a must-have feature, and allow it in the common code as well. Currently, this will affect only intel port, the other Dingux-based ports are dead anyway.

Yes, that should be the end goal. But I though this intermediate step where we restrict how much c++11 we can use in common code gives us more time to deploy the new buildbot and gives porter more time to update their toolchains.

@sev-
Copy link
Member

@sev- sev- commented Oct 3, 2020

Thanks!

@sev- sev- merged commit 786aac5 into scummvm:master Oct 3, 2020
0 of 5 checks passed
0 of 5 checks passed
Windows (win32, x86-windows, x86, --enable-faad --enable-mpeg2 --enable-discord --disable-fribidi... Windows (win32, x86-windows, x86, --enable-faad --enable-mpeg2 --enable-discord --disable-fribidi...
Details
Windows (x64, x64, x64-windows, --enable-faad --enable-mpeg2 --enable-discord --disable-fribidi, ... Windows (x64, x64, x64-windows, --enable-faad --enable-mpeg2 --enable-discord --disable-fribidi, ...
Details
Windows (arm64, arm64, arm64-windows, --enable-faad --enable-mpeg2 --enable-discord --disable-fri... Windows (arm64, arm64, arm64-windows, --enable-faad --enable-mpeg2 --enable-discord --disable-fri...
Details
deepcode-ci-bot Found 1 warning issue.
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
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

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