MSVC 2015 - SCI does not build, STATIC_ASSERT broken?

[quietust]

When building ScummVM using Visual C++ 2015, the final link step to build scummvm.exe fails with 2 unresolved external symbol errors from sci.lib(message.obj) for "int * STATIC_ASSERT_uint16_can_only_be_read_from_int16_or_smaller_spans".

STATIC_ASSERT looks to be a macro (defined in common/scummsys.h) which tries to emulate C++11 "static_assert" (which works fine in MSVC 2015), but it's not working correctly here.

Oddly, if I try using the STATIC_ASSERT macro in a test program, it seems to work fine, aside from assertion failures showing up as "negative subscript" instead of the message provided.

[dreammaster]

That sounds weird. I build with Visual Studio 2015, and I've never had such issues with linking in the SCI engine. A few things I can suggest:
1) make sure you're using the dists/msvc14 folder to build the solution, not msvc15, since that's a VS2017 solution
2) Check if you're using Debug/Win32 as the target. I at least know that that target works with the scummvm_libs zip file we provide. Not sure about the other targets off the top of my head.

If you still have problems, try changing all the lines calling create_project in the dists/msvc14/create_msvc14.bat file to add in a --disable-engine=sci
See if it compiles afterwards. Presuming that you're not building ScummVM specifically to poke around the SCI engine, that would at least hopefully let you bypass the problem you're having and run the executable.

[quietust]

Apparently, the problem is that I was trying to make a Release build - if I do a Debug build, it completes successfully.

[dreammaster]

Ah, good then.

[quietust]

I'm not quite sure how this is "not an issue" - create_project still offers a "Release" build configuration, but it clearly doesn't work with MSVC14.

Currently, the wiki page ​Compiling ScummVM/Visual Studio does not explicitly state that Debug is required, just that it's the default and that it's ideal for playing around with changes. If this is no longer the case, then it should be updated.

[csnover]

At first glance, this seems like an MSVC bug where the symbols are getting optimised in a way that is confusing the linker. Does this problem also occur in Visual C++ 2017? If so, I think we could probably change the macro to use the real static_assert (which, actually, might not be a bad idea in any case) which will hopefully solve the problem since then that symbol would be a string literal for the message parameter of static_assert, and the compiler will know how to deal with its own static assertions, and so much less likely to get screwed up.

[csnover]

quietust, could you please try applying this patch and let me know if it solves building in release mode? Thanks!

[quietust]

That patch does indeed fix Release builds in VC++ 2015. Unfortunately, I don't have VC++ 2017 installed, so I can't test it to see if it works there too.

[csnover]

Great! Thank you for testing. I imagine fewer and fewer problems with MSVC++ going forward since it is now apparently ​far more standards-compliant than it has been historically. I’ll land this tomorrow when I am fresh.

[csnover]

Thanks for your report! A patch for this issue has been added in commit 2de83e09374abd10700e5f2bc4671318f182980b and will be available in daily builds 1.10.0git-5311 and later.