MSVC compiler bug hit with __declspec(extern) and multiple static members

[milancpp]

Hi there,

this is more of a heads up rather than an actual issue (yet).
I'm playing around with the code base (adding a CMake build system), trying to build it with the Visual Studio 2017 compilers. I just hit a frustrating issue which (after many hours) turned out to be a compiler bug in the MSVC compilers.

The problem is hit by macros like #define QSK_STATES( ... ) static const QskAspect::State VA_ARGS; from QskAspects.h. When building as a shared library, the static member variables defined by the macro on exported classes are not all exported, only the first one. The bug has been known for over 17 (!) years and is present in all modern MSVC compilers (see https://jeffpar.github.io/kbarchive/kb/127/Q127900/)

I'm not sure what to suggest. If MSVC is a platform you want to support, the only solution would be to refactor the macro(s) to only define a single member and instantiate it multiple times.

[milancpp]

I stumbled on one more incompatibility in MSVC. Bit fields seem to produce strange padding behaviour, so the only solution I found to get QskAspect::Bits to be 8 bytes (as you enforce using static_assert) is to make the isAnimator field an uint.

[uwerat]

In general QSkinny is supposed to work on platforms, where Qt/Quick is available - even if I don't use and test MSVC environments myself so far. So yes please report any sort of platform related issues ( compile/runtime ) as we don't do any tests beside clang/gcc on Linux so far.

a) Bit fields

I changed the bool to an uint.

b) QSK_STATES/QSK_SUBCONTROLS

This one is annoying as it requires to find some magic preprocessor code, that generates specific statements for each parameters of the variadic macros.

You can have up to 8 different states and 16 sub controls - everything beyond does not fit in the QskAspect bitmask.

Do you have some code for me, that works with MSVC ?

[milancpp]

I did find some preprocessor trickery with variadic macros (courtesy to https://stackoverflow.com/questions/14732803/preprocessor-variadic-for-each-macro-compatible-with-msvc10) that actually works for MSVC compilers (can't test atm whether the other ones do). Here's how I defined both macros:

#define EXPAND(x) x
#define FOR_EACH_1(what, x, ...) what(x)
#define FOR_EACH_2(what, x, ...)\
  what(x);\
  EXPAND(FOR_EACH_1(what,  __VA_ARGS__))
#define FOR_EACH_3(what, x, ...)\
  what(x);\
  EXPAND(FOR_EACH_2(what, __VA_ARGS__))
#define FOR_EACH_4(what, x, ...)\
  what(x);\
  EXPAND(FOR_EACH_3(what,  __VA_ARGS__))
#define FOR_EACH_5(what, x, ...)\
  what(x);\
  EXPAND(FOR_EACH_4(what,  __VA_ARGS__))
#define FOR_EACH_6(what, x, ...)\
  what(x);\
  EXPAND(FOR_EACH_5(what,  __VA_ARGS__))
#define FOR_EACH_7(what, x, ...)\
  what(x);\
  EXPAND(FOR_EACH_6(what,  __VA_ARGS__))
#define FOR_EACH_8(what, x, ...)\
  what(x);\
  EXPAND(FOR_EACH_7(what,  __VA_ARGS__))
#define FOR_EACH_NARG(...) FOR_EACH_NARG_(__VA_ARGS__, FOR_EACH_RSEQ_N())
#define FOR_EACH_NARG_(...) EXPAND(FOR_EACH_ARG_N(__VA_ARGS__))
#define FOR_EACH_ARG_N(_1, _2, _3, _4, _5, _6, _7, _8, N, ...) N
#define FOR_EACH_RSEQ_N() 8, 7, 6, 5, 4, 3, 2, 1, 0
#define CONCATENATE(x,y) x##y
#define FOR_EACH_(N, what, ...) EXPAND(CONCATENATE(FOR_EACH_, N)(what, __VA_ARGS__))
#define FOR_EACH(what, ...) FOR_EACH_(FOR_EACH_NARG(__VA_ARGS__), what, __VA_ARGS__)

#define QSK_SUBCONTROLS_HELPER(name) static const QskAspect::Subcontrol name;
#define QSK_SUBCONTROLS( ... ) FOR_EACH(QSK_SUBCONTROLS_HELPER, __VA_ARGS__)

#define QSK_STATES_HELPER(name) static const QskAspect::State name;
#define QSK_STATES( ... ) FOR_EACH(QSK_STATES_HELPER, __VA_ARGS__)

Though, for the sake of readability, I'd hide/ifdef this abomination unless actually compiling with MSVC

[milancpp]

Having bypassed the former problems, here are a few more I stumbled upon:

QskTextInput.cpp

#define private public
#include <private/qquicktextinput_p.h>
#include <private/qquicktextinput_p_p.h>
#undef private

QskRichTextRenderer.cpp

QSK_QT_PRIVATE_BEGIN
#include <private/qquicktext_p.h>
#include <private/qquicktext_p_p.h>
QSK_QT_PRIVATE_END

QskGraphicTextureFactory.cpp

#define private public
#include <qopengltexture.h>
#undef private

These hacks will sadly not work with MSVC, it simply doesn't link. The reason is that public/private is actually part of the symbol name (in mangled form) so the linker simply will never find the symbols.

In practice those three symbols cannot be linked because Qt actually has them set to private:

"public: class QOpenGLTexturePrivate * __cdecl QOpenGLTexture::d_func(void)"
"public: __cdecl QQuickTextPrivate::ExtraData::ExtraData(void)"
"public: enum QQuickTextInputPrivate::ValidatorState __cdecl QQuickTextInputPrivate::hasAcceptableInput(class QString const &)const "

I'm not really sure how to fix this, since I don't actually know about these functions and what they do. Any ideas?

EDIT: I'm using Qt 5.11.2 by the way

[uwerat]

a) QQuickTextInputPrivate::hasAcceptableInput

This is only a few lines of trivial code and it operates on public members. So probably the best way is to copy them out - or even better, writing some code that does the same so that no copyrights considerations have to be taken into count. I will try to find some code.

For the moment I simply disabled the code and fixup always returns true. As none of the examples is using validators so far this should be good enough until I have a replacement.

b) QQuickTextPrivate::ExtraData::ExtraData

I'm not 100% sure why this one is not exported, but it looks like I can't call QLazilyAllocated::value. To avoid that I added some dummy calls in TextItem::TextItem having the side effect of allocating extraData. Then I can use QLazilyAllocated::operator->() instead.

c) QOpenGLTexture::d_func

This hack falls into the category of trying to be careful with memory, what is one of the fundamental principles of this project.

The problem here is, that I don't want to store a heavy QOpenGLTexture ( QObject + extra data ) when the only relevant data is an uint. Unfortunately the destructor of QOpenGLTexture destroys the texture. So the code copies the textureId out and replaces it by an invalid one.

Probably the best solution here is to avoid using QOpenGLTexture at all and finding the necessary sequence of opengl calls. Actually getting rid of QOpenGLTexture makes sense for more good reasons - I will have a look at it.

For the moment I have changed the code, so that with MSVC it falls back on creating textures using the OpenGL paint engine. In general you have a better quality ( and less bugs ) with the raster paint engine but until I have the new code it should be good enough.

Let me know, what else you find.

[milancpp]

Unfortunately there's another call to hasAcceptableInput in QskTextInput::setEditing. Working around it as follows finally allows me to compile the actual library. I have no idea if the status check is neccessary or just an optimization.

#ifdef _MSC_VER
        Q_EMIT m_data->textInput->editingFinished();
#else
        auto d = QQuickTextInputPrivate::get( m_data->textInput );

        const auto status = d->hasAcceptableInput( d->m_text );
        if ( status == QQuickTextInputPrivate::AcceptableInput )
        {
            if ( fixup() )
                Q_EMIT m_data->textInput->editingFinished();
        }
#endif

By the way, are you interested in adding an optional CMake build system for the project? I'm working on one and could probably contribute it

[uwerat]

Hope I finally fixed all issues you have mentioned by finding a different implementation. Now there is no hacking of the private methods left.

While replacing QOpenGLTexture by native calls is an improvement I'm not 100% happy with the workaround for the MSVC bug. It seems also not being compliant with ISO C++11. and the compilation with gcc fails, when enabling -pedantic.
But for the moment it should be good enough to get things started with MSVC.

By the way: did you see any issues, when running the examples on a Windows platform ?

[milancpp]

Sorry I was out of town and couldn't reply. I haven't gotten around to actually compile the examples since I only compiled the actual library so far, not the theming plugin system. Will report back if I stumble upon more problems