msvc

[ghost]

Here's the latest. It's flawed (read TODO notes). It works with 2015. Depending on the merge in zynaddsubfx repo, you might have to change some links.

I'll make a new branch, so you can pick and choose what modification you accept (if any). I haven't tested it on linux.

[fundamental]

Parroting one comment from the PR on lmms's zyn fork. std::vector should NOT appear in realtime code. LMMS has numerous problems in terms of realtime safety already and it should not make the situation worse for itself.

[lieff]

-    		QAction *presetActions[list1.size()];
+    		std::vector<QAction*> presetActions(list1.size());

Right, std::vector internally calls malloc() while original code uses stack pointer advance.
alloca() can be used for dynamic stack allocation, it works on all modern compilers.

[fundamental]

Or just use MSVC 2017, which to the best of my knowledge supports VLAs.

[lukas-w]

If I recall correctly, MSVC 2017 doesn't support VLAs either, which is also what the documentation says.

We can use QVarLengthArray where Qt is available, just like in #4000.

[fundamental]

@lukas-w the linked page is dated 11/04/2016.

[lukas-w]

@fundamental Good catch. :)

[fundamental]

Additionally QVarLengthArray does fall back onto using malloc()/free() in some cases.
See https://github.com/qt/qtbase/blob/e843e3bb00d26c841bd7132c00779c085368eab3/src/corelib/tools/qvarlengtharray.h and search around for free/malloc.

[lukas-w]

@fundamental I'd argue that falling back to malloc is better than failing silently. On the other hand, I didn't realise QVarLengthArray doesn't even use alloca, but just a fixed-size preallocated array. So I guess we're left with checking if MSVC2017 supports VLAs or using alloca if it doesn't.

[ghost]

It doesn't.
IIRC, VLA is only in the C99 standard and never made it into the c++ standard. Using VLA in c++ code is a gnu extension to the c++ standard.

edit: not making a judgement call whether this is good/bad or anything.

[ghost]

We're getting a bit off track by focusing on the details.

I did this in the first place to have debug symbols.
It's maybe a controversial idea, but if nobody uses this setup upstream (or even in lmms) then maybe it's better not to try to merge. It works upstream. The changes are still public so if someone wants to tackle it in the future they have a reference of the mistakes and successes of the initial attempt.

(feel free to skip the rest, it's only an example why debug symbols are important to me)

The latest commit to lmms in this PR is an example of that (jack midi thing), it took me literally less than 5 minutes to find the cause.

Lmms crashed on exit, I attached visual studio and I could navigate the call stack with source code displayed. I've compiled all the libs lmms is using with debug information.

It crashed inside jack, I have jack compiled with debug info so I know where it crashed in jack. Going up the stack frame, I saw it was called from a destructor of MidiJack (typing from memory, so the names might be wrong). Further up the stack: mixer destructor was called which destroyed audiojack first and midijack shared the connection with audiojack.

[lieff]

Build with -g -rdynamic should produce good call stacks on linux too. But I think it's good to support msvc using alloca.