compiler.pri: improve modern C++ detection #2630

mkrautz · 2016-11-08T23:40:32Z

This PR adds a cplusplus.pri file (which gets included by compiler.pri).

Its purpose is to handle selection of C++ standards at build time via qmake's CONFIG variable.

Builders can now specify, say, qmake -recursive -Wall main.pro CONFIG+="release c++11" to select C++ mode.

Besides the ability to explicitly select a C++ standard, cplusplus.pri will auto-detect an appropriate C++ standard for the current system.

Supercedes PRs #2608 and #2628

mkrautz · 2016-11-08T23:43:40Z

Can't properly document this for now (other than the code comments).
Feel free to take a look. Will add more docs (commit subjects, commit body, PR description) later.

mkrautz · 2016-11-08T23:49:34Z

compiler.pri

-		# But Qt 4 won't, so we add it manually.
-		lessThan(QT_MAJOR_VERSION, 5) {
-			QMAKE_CXXFLAGS *= -std=c++11
+	# First, auto-populate CONFIG with the latest C++ standard


These checks should probably go outside the unix{} block.

EmperorArthur

It's the best I've seen so far for dealing with all C++ variants, but it has a few failure modes, that should probably be addressed.

EmperorArthur · 2016-11-10T02:49:27Z

compiler.pri

+		}
+		CONFIG(c++1z) {
+			QMAKE_CXXFLAGS += -std=c++1z
+			QMAKE_LFLAGS += -std=c++1z
 		}


This section won't work if more than one CONFIG option is set. g++ doesn't like having multiple -std=... lines passed to it.

If QT is compiled with support for more than one of these, then more than one CONFIG option is set.

Fixed in latest revision.

EmperorArthur · 2016-11-10T02:52:30Z

compiler.pri

+		contains(QT_CONFIG, c++11): CONFIG *= c++11
+		contains(QT_CONFIG, c++14): CONFIG *= c++14
+		contains(QT_CONFIG, c++1z): CONFIG *= c++1z
+	}


If QT is compiled with support for more than one of these, then more than one CONFIG option is set.

This causes compile errors in the lessThan(QT_MAJOR_VERSION, 5) { block.

If qmake has a break statement, I would suggest putting breaks in between each of these lines. If not, you are going to have to separate this out into three blocks.

Fixed in latest revision.

EmperorArthur · 2016-11-10T02:57:10Z

compiler.pri

+	# populate QMAKE_CXXFLAGS and QMAKE_LFLAGS with the
+	# appproiate -std=c++XX flags. So, for Qt 4, we'll
+	# have to set them up manually.
+	lessThan(QT_MAJOR_VERSION, 5) {


This comment and check are, unfortunately, incorrect.

Qt 5 does not populate QMAKE_CXXFLAGS with any -std=c++XX.

You can see for yourself by adding this right before/after the check:

message("QMAKE_CXXFLAGS=$${QMAKE_CXXFLAGS}")

If we want mumble to build on GCC 6.0 and older with Qt 5.7 or later, we're going to have to manually set QMAKE_CXXFLAGS.

I'd recommend removing the check altogether.

I'm pretty sure I checked that this was the case.
They should be populated via the default_post.prf qmake feature file.
See: https://github.com/qt/qtbase/blob/1e97037ed97aade9eb107bbc595785c2de76f7dc/mkspecs/features/default_post.prf#L98

But I'll double-check.

And, the reason you can't print them via message is because they're populated after processing the application's .pro file.

OK, I finally had a chance to test this on a Linux box.

Qt 5.6.1 on my Debian stretch install does in fact pass -std=gnu++1z.

mkrautz · 2016-11-11T21:37:53Z

@EmperorArthur PTAL

Kissaki · 2016-11-11T21:55:19Z

cplusplus.pri

+	# The expectation is that this is a general convention,
+	# so add it to our library search path in C++11, C++14
+	# and C++1z mode.
+	CONFIG(c++11):CONFIG(c++14):CONFIG(c++1z) {


Should be ORs

Kissaki · 2016-11-11T22:04:39Z

cplusplus.pri

+unix {
+	# Qt 4, as opposed to Qt 5, doesn't automatically
+	# populate QMAKE_CXXFLAGS and QMAKE_LFLAGS with the
+	# appproiate -std=c++XX flags. So, for Qt 4, we'll


typo appproiate -> appropriate

Kissaki

Typo in commit message: "Wile" -> "While"

mkrautz · 2016-11-11T22:16:15Z

Typo in commit message: "Wile" -> "While"

Done.

mkrautz · 2016-11-11T22:41:14Z

Hmmm, seems like the QT_CONFIG checks aren't working on my Debian stretch system. (Qt 5.6) They work on Arch, though. (Qt 5.7) Testing...

mkrautz · 2016-11-11T22:49:22Z

OK, the problem is that mkspecs/features/qt_module_headers.prf will add -std=c++98 if CONFIG(c++11) isn't set... So it seems we'll have to add c++11 for all standards above c++11... Hmmm...

mkrautz · 2016-11-11T23:02:22Z

Hmmm, seems like the QT_CONFIG checks aren't working on my Debian stretch system. (Qt 5.6) They work on Arch, though. (Qt 5.7) Testing...

[...]

OK, the problem is that mkspecs/features/qt_module_headers.prf will add -std=c++98 if CONFIG(c++11) isn't set... So it seems we'll have to add c++11 for all standards above c++11... Hmmm...

Fixed in latest upload.

mkrautz · 2016-11-11T23:02:26Z

PTAL

mkrautz · 2016-11-11T23:03:04Z

cplusplus.pri

+# First, auto-populate CONFIG with the latest C++ standard
+# that Qt has detected support for.
+!CONFIG(c++03):!CONFIG(c++11):!CONFIG(c++14):!CONFIG(c++1z) {
+	# For the c++1z and c++14 below, we also add


"For the c++1z and c++14 cases below"

maybe?

mkrautz · 2016-11-11T23:03:27Z

cplusplus.pri

+	#
+	# For example, Qt 5.6's
+	# mkspecs/features/qt_module_headers.prf
+	# will automatically add -std=c++98 to CXXFLAGS


file will

maybe?

mkrautz · 2016-11-11T23:04:00Z

cplusplus.pri

+	# The result is that if we ONLY put c++1z in
+	# CONFIG, that check will cause us to have
+	# *both* -std=c++98 *and* -std=gnu++1z in
+	# CXXFLAGS, which is bad.


CXXFLAGS when building on Qt 5.6, which is bad.

mkrautz · 2016-11-11T23:04:25Z

cplusplus.pri

+	#
+	# In order to be compatible with such checks,
+	# ensure that we have all prior "modern"
+	# standards in CONFIG, to please Qt.


standards -> # C++ standards

EmperorArthur · 2016-11-13T04:52:35Z

Looks good to me.

This commit adds a cplusplus.pri file that allows users to pass 'c++03', 'c++11', 'c++14' or 'c++1z' in CONFIG when building Mumble via qmake. For example, to explicitly build Mumble in c++1z mode, one would use: $ qmake -recursive -Wall main.pro CONFIG+="release c++1z" While it is possible to explicitly specify a target C++ standard when building Mumble, the cplusplus.pri will auto-detect an appropriate C++ standard to target for the system it is building on.

In the past, Debian stretch put C+11 ZeroC Ice libraries in a special C++11-specific library search path. Since it doesn't hurt to look there, keep this around, even though it's probably not needed anymore.

mkrautz mentioned this pull request Nov 8, 2016

Fix issue #2600 #2628

Closed

mkrautz commented Nov 8, 2016

View reviewed changes

EmperorArthur suggested changes Nov 10, 2016

View reviewed changes

mkrautz force-pushed the moderncpp branch from 2a8686a to c21d294 Compare November 11, 2016 21:36

Kissaki reviewed Nov 11, 2016

View reviewed changes

mkrautz force-pushed the moderncpp branch from c21d294 to 71a101f Compare November 11, 2016 21:57

Kissaki reviewed Nov 11, 2016

View reviewed changes

Kissaki requested changes Nov 11, 2016

View reviewed changes

mkrautz force-pushed the moderncpp branch from 71a101f to c5546a4 Compare November 11, 2016 22:15

Kissaki approved these changes Nov 11, 2016

View reviewed changes

mkrautz force-pushed the moderncpp branch from c5546a4 to 324b2a0 Compare November 11, 2016 23:00

mkrautz commented Nov 11, 2016

View reviewed changes

mkrautz added 2 commits November 13, 2016 10:59

cplusplus.pri: handle Debian C++11 library path.

4a91dd7

In the past, Debian stretch put C+11 ZeroC Ice libraries in a special C++11-specific library search path. Since it doesn't hurt to look there, keep this around, even though it's probably not needed anymore.

mkrautz force-pushed the moderncpp branch from 324b2a0 to 4a91dd7 Compare November 13, 2016 09:59

mkrautz merged commit 713e2aa into mumble-voip:master Nov 13, 2016

mkrautz mentioned this pull request Nov 13, 2016

Build fails with Qt 5.7 #2600

Closed

radioactiveman mentioned this pull request Aug 30, 2018

Fails to build with protobuf 3.6.0 due to c++11 requirement #3486

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

compiler.pri: improve modern C++ detection #2630

compiler.pri: improve modern C++ detection #2630

mkrautz commented Nov 8, 2016 •

edited

Loading

mkrautz commented Nov 8, 2016

mkrautz Nov 8, 2016

EmperorArthur left a comment

EmperorArthur Nov 10, 2016

mkrautz Nov 11, 2016

EmperorArthur Nov 10, 2016

mkrautz Nov 11, 2016

EmperorArthur Nov 10, 2016

mkrautz Nov 10, 2016

mkrautz Nov 10, 2016

mkrautz Nov 11, 2016

mkrautz commented Nov 11, 2016

Kissaki Nov 11, 2016

mkrautz Nov 11, 2016

Kissaki Nov 11, 2016

mkrautz Nov 11, 2016

Kissaki left a comment

mkrautz commented Nov 11, 2016

mkrautz commented Nov 11, 2016

mkrautz commented Nov 11, 2016

mkrautz commented Nov 11, 2016

mkrautz commented Nov 11, 2016

mkrautz Nov 11, 2016

mkrautz Nov 11, 2016 •

edited

Loading

mkrautz Nov 11, 2016

mkrautz Nov 11, 2016

EmperorArthur commented Nov 13, 2016

compiler.pri: improve modern C++ detection #2630

compiler.pri: improve modern C++ detection #2630

Conversation

mkrautz commented Nov 8, 2016 • edited Loading

mkrautz commented Nov 8, 2016

Choose a reason for hiding this comment

EmperorArthur left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkrautz commented Nov 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kissaki left a comment

Choose a reason for hiding this comment

mkrautz commented Nov 11, 2016

mkrautz commented Nov 11, 2016

mkrautz commented Nov 11, 2016

mkrautz commented Nov 11, 2016

mkrautz commented Nov 11, 2016

Choose a reason for hiding this comment

mkrautz Nov 11, 2016 • edited Loading

Choose a reason for hiding this comment

file will

Choose a reason for hiding this comment

CXXFLAGS when building on Qt 5.6, which is bad.

Choose a reason for hiding this comment

standards -> # C++ standards

EmperorArthur commented Nov 13, 2016

mkrautz commented Nov 8, 2016 •

edited

Loading

mkrautz Nov 11, 2016 •

edited

Loading