Windows Compile Error, keyutils.cpp cannot convert from 'initializer list'

Bug #1627826 reported by Zaren
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mixxx
Fix Released
Undecided
Sébastien BLAISOT

Bug Description

Ran into a compile error on Windows using Visual Studio 2013 and the latest master (as of commit 14e10b29b57ae46ff8bb9ac0b815f8a4cb475848). Compiling fails on src\track\keyutils.cpp with the following errors:

keyutils.cpp
src\track\keyutils.cpp(130) : error C2440: 'initializing' : cannot convert from 'initializer-list' to 'QList<mixxx::track::io::key::ChromaticKey>'
        No constructor could take the source type, or constructor overload resolution was ambiguous
src\track\keyutils.cpp(170) : error C2440: 'initializing' : cannot convert from 'initializer-list' to 'QList<mixxx::track::io::key::ChromaticKey>'
        No constructor could take the source type, or constructor overload resolution was ambiguous
scons: *** [win32_build\track\keyutils.obj] Error 2
scons: building terminated because of errors.

I can't really code too well, so I pass this on to you knowledgeable types.

Revision history for this message
Daniel Schürmann (daschuer) wrote :

Please try these ideas for both initialize lists:
s_sortKeysCircleOfFifths({ ... });
s_sortKeysCircleOfFifths = { .. };
s_sortKeysCircleOfFifths(s_sortKeysCircleOfFifths(std::initializer_list<mixxx::track::io::key::ChromaticKey>({ .. }));
s_sortKeysCircleOfFifths = QList<mixxx::track::io::key::ChromaticKey>({.. });
s_sortKeysCircleOfFifths = static_cast<std::initializer_list<mixxx::track::io::key::ChromaticKey> >({.. });

Revision history for this message
Sébastien BLAISOT (sblaisot) wrote :
Changed in mixxx:
status: New → Confirmed
Revision history for this message
Zaren (czchmate) wrote :

I'm sorry. I don't understand what you mean for me to do with the code above.

I really can't that well (barely at all) and some of the code you wrote looks redundant and incomplete. I just don't have the capability to follow through with your suggestions.

Thanks for the suggestion, though.

Revision history for this message
Zaren (czchmate) wrote :

[edit] I really can't CODE that well.

Revision history for this message
Daniel Schürmann (daschuer) wrote :

Sébastien, are you able to jump in?

We should also verify that the
Q_COMPILER_INITIALIZER_LISTS
macro is set when building Qt on windows.

Revision history for this message
Sébastien BLAISOT (sblaisot) wrote :

yes, I will take a look on that.
please give me some time to do that. Probably by the end of week.

Revision history for this message
Sébastien BLAISOT (sblaisot) wrote :
Revision history for this message
Sébastien BLAISOT (sblaisot) wrote :

adding this piece of code before

#ifndef Q_COMPILER_INITIALIZER_LISTS
#error Q_COMPILER_INITIALIZER_LISTS is undefined!
#endif

results in

src\track\keyutils.cpp(93) : fatal error C1189: #error : Q_COMPILER_INITIALIZER_LISTS is undefined!

so you got it, Q_COMPILER_INITIALIZER_LISTS is undefined with QT4 on windows with VS2013 by default. We have to use another way of coding that or remove support for VS2013 or QT4.

Revision history for this message
Sébastien BLAISOT (sblaisot) wrote :

this is also interresting : http://lists.qt-project.org/pipermail/development/2015-March/020644.html

"""The other problem with initialiser lists is std::initializer_list: even though
this header is technically part of the compiler core language, it's supplied
as part of libstdc++, so it's missing on QNX 6.5 and on OS X if you don't use
libc++."""

So this seems to be a problem on macos too

Revision history for this message
Be (be.ing) wrote :

What version of Qt are you compiling with? QList support for initializer lists was added in Qt 4.8.

There was a problem on OS X as well, which was solved by upgrading XCode: https://github.com/mixxxdj/mixxx/commit/bac0ff9fd4138dd89d623935f3b3e36c22226bd4#commitcomment-19067421

Revision history for this message
Be (be.ing) wrote :

I see Jenkins built with Qt 4.8.7.

Revision history for this message
Sébastien BLAISOT (sblaisot) wrote :

yes, build env is QT 4.8.7, but Q_COMPILER_INITIALIZER_LISTS is undefined

Revision history for this message
Sébastien BLAISOT (sblaisot) wrote :

Qt 4.8 doesn't support QList for initializer lists on MSVC!

see http://code.qt.io/cgit/qt/qt.git/tree/src/corelib/global/qglobal.h#n910 (line 919)

so we have to use another way to achieve this list.

Daniel, I tried all your suggestions, but didn't got it to work with any.

Revision history for this message
Daniel Schürmann (daschuer) wrote :

OK Thank you. So #8 seams to be the root cause.

We may hack around it by adding
#define Q_COMPILER_INITIALIZER_LISTS
at the very top of keyutils.cpp

we need probably also () around the initializer lists.

This might work, because the C list constructor is inlined and the change does not effect the qt dll code.

A clean and in addition malloc free solution would be to move to a plain C array though.

Revision history for this message
Daniel Schürmann (daschuer) wrote :

Yes, the "std::initializer_list" seams not to be established enough.

const mixxx::track::io::key::ChromaticKey s_sortKeysCircleOfFifths[] = { ... };

should work on any compiler. Unfortunately there is more work to do when s_sortKeysCircleOfFifths is used.

Revision history for this message
Zaren (czchmate) wrote :

It seems like the conversation has moved on, but I was using QT 4.8.6, as it says to do in https://www.mixxx.org/wiki/doku.php/compiling_on_windows.

Revision history for this message
Be (be.ing) wrote :

https://msdn.microsoft.com/en-us/library/hh567368.aspx

It seems dropping support for MSVC2013 and moving to MSVC2015 may be a good idea to avoid more issues like this in the future. However, I don't know if that would solve this particular issue.

Revision history for this message
Daniel Schürmann (daschuer) wrote :

@Zaren: you may try the quick hack and put

#define Q_COMPILER_INITIALIZER_LISTS

at the very top of keyutils.cpp

Revision history for this message
Be (be.ing) wrote :
Revision history for this message
Sébastien BLAISOT (sblaisot) wrote :

moving to MSVC2015 is waiting Pegasus job to migrate build env.
Before that, we are tied to MSVC2013 and we don't have any other build env with MSVC 2015.
After that, the build env will switch to MSVC2015.

But... MSVC 2015 alone is not sufficient because if i correctly understand QT 4.8 code, Q_COMPILER_INITIALIZER_LISTS is undefined if using MSVC, regardless of MSVC version.

so either we switch to (QT5 + MSVC2015) or we change compiler (other than MSVC)

Revision history for this message
Sébastien BLAISOT (sblaisot) wrote :

Be: You're right, we faced this bug first in January, found out why we have it (QT4 bug), didn't do anything to avoid facing it again, and finally 8 months later have that "bug"/"compiler feature" again.

Now, that being said :
1) what do we do in the short term to make mixxx compile again on windows with the current QT4 + MSVC2013 build env (mixxx 2.0 build env) ?
2) what can we do to achieve build env migration to mixxx 2.1 build env with QT5 + MSVC2015 ?

we are stuck on this since January.

Revision history for this message
Sébastien BLAISOT (sblaisot) wrote :

@daschuer and @Zaren use this piece of code instead on top of keyutils.cpp. I will try it tonight:

#if _MSC_FULL_VER >= 180030324 // VC 12 SP 2 RC
  #define Q_COMPILER_INITIALIZER_LISTS
#endif /* VC 12 SP 2 RC */

this avoids defining Q_COMPILER_INITIALIZER_LISTS if compiler doesn't support initialiser lists.

Revision history for this message
Be (be.ing) wrote :

As Daniel mentioned in #15, using an old fashioned C array and std::find() instead of QList::indexOf() is a potential work around, but I'd rather not have to do that. If we can use a preprocessor hack until upgrading to MSVC2015 and Qt5, I think that'd be a reasonable short-term solution.

Revision history for this message
Be (be.ing) wrote :

Work was done a while ago to get Mixxx to build with MinGW GCC, but it wasn't merged and I don't know how much work that would require now to get working. I'm not sure if it would be more trouble in the long run to continue using MSVC and putting up with its incomplete standards support or maintaining a MinGW build system.

https://bugs.launchpad.net/mixxx/+bug/1179683
https://sourceforge.net/p/mixxx/mailman/message/30928729/
https://sourceforge.net/p/mixxx/mailman/message/31087596/
https://github.com/mixxxcommunity/mixxx/commits/ulatekh-mingw
https://github.com/mixxxcommunity/mixxx/pull/1

Revision history for this message
Daniel Schürmann (daschuer) wrote :

MSVC2013 does support initializer
https://msdn.microsoft.com/de-de/library/hh567368.aspx

The VC version check is not necessary, because the build will fail anyway if the compiler has no initializer lists.

Revision history for this message
Sean M. Pappalardo (pegasus-renegadetech) wrote :

We found that performance of the MinGW build was noticeably worse than the MSVC one. That may have changed in the time that has passed though.
Also FWIW, Renegade Tech's Windows build machine is already using MSVC2015.

Revision history for this message
Sébastien BLAISOT (sblaisot) wrote :

Renegade Tech's Windows build machine is currently a mirage. The closer you get, you always have the same distance to walk to be there.

but yes, mixxx 2.1 windows build env will use MSVC2015 + QT5

Changed in mixxx:
assignee: nobody → Sébastien BLAISOT (sblaisot)
Revision history for this message
Sébastien BLAISOT (sblaisot) wrote :
Revision history for this message
Sébastien BLAISOT (sblaisot) wrote :

build tested and ok.

Revision history for this message
Zaren (czchmate) wrote :

I altered Daniel's code to:

#if _MSC_VER >= 1800 // VC 12
  #define Q_COMPILER_INITIALIZER_LISTS
#endif /* VC 12 */

and everything works fine. As he pointed out, Visual Studio 2013 allows for initializer lists. Instead of using the fine point _MSC_FULL_VER, I just used _MSC_VER. No need to get any more exact than that.

I think it may be a better hack than https://github.com/mixxxdj/mixxx/pull/1019

We shouldn't be defining Q_COMPILER_INITIALIZER_LISTS if it is not supported by the Microsoft Compiler. It is supported from MSC_VER 1800 or higher.

Changed in mixxx:
status: Confirmed → Fix Committed
Changed in mixxx:
milestone: none → 2.1.0
Changed in mixxx:
status: Fix Committed → Fix Released
Revision history for this message
Swiftb0y (swiftb0y) wrote :

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/8654

lock status: Metadata changes locked and limited to project staff
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.