Fix build errors on VS2019 MSVC #8688
Conversation
src/frontend/mame/infoxml.cpp
Outdated
| @@ -434,7 +434,7 @@ void info_xml_creator::output(std::ostream &out, const std::function<bool(const | |||
| prepared_info() = default; | |||
| prepared_info(const prepared_info &) = delete; | |||
| prepared_info(prepared_info &&) = default; | |||
| prepared_info &operator=(const prepared_info &) = delete; | |||
| prepared_info &operator=(const prepared_info &) = default; | |||
There was a problem hiding this comment.
Why have you changed this? The object is written this way to prevent accidental copy assignment, which is expensive. The objects shouldn’t be copied at all, only moved.
There was a problem hiding this comment.
Without this, this results in the following build error:
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(324,17): error C2280: 'info_xml_creator::output::prepared_info &info_xml_creator::output::prepared_info::operator =(const info_xml_creator::output::prepared_info &)': attempting to reference a deleted function
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(324,17): error C2280: _Result = _STD forward<_Ty>(_Val);
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(324,17): error C2280: ^
36>C:\msys64\home\friedkiwi\mame-upstream\src\frontend\mame\infoxml.cpp(437): message : see declaration of 'info_xml_creator::output::prepared_info::operator ='
36>C:\msys64\home\friedkiwi\mame-upstream\src\frontend\mame\infoxml.cpp(437): message : prepared_info &operator=(const prepared_info &) = delete;
36>C:\msys64\home\friedkiwi\mame-upstream\src\frontend\mame\infoxml.cpp(437,18): message : 'info_xml_creator::output::prepared_info &info_xml_creator::output::prepared_info::operator =(const info_xml_creator::output::prepared_info &)': function was explicitly deleted
36>C:\msys64\home\friedkiwi\mame-upstream\src\frontend\mame\infoxml.cpp(437,18): message : prepared_info &operator=(const prepared_info &) = delete;
36>C:\msys64\home\friedkiwi\mame-upstream\src\frontend\mame\infoxml.cpp(437,18): message : ^
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(319): message : while compiling class template member function 'void std::_Associated_state<_Ty>::_Set_value_raw(_Ty &&,std::unique_lockstd::mutex *,bool)'
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(319): message : with
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(319): message : [
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(319): message : _Ty=info_xml_creator::output::prepared_info
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(319): message : ]
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(319): message : bool _At_thread_exit) { // store a result while inside a locked block
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(315): message : see reference to function template instantiation 'void std::_Associated_state<_Ty>::_Set_value_raw(_Ty &&,std::unique_lockstd::mutex *,bool)' being compiled
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(315): message : with
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(315): message : [
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(315): message : _Ty=info_xml_creator::output::prepared_info
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(315): message : ]
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(315): message : _Set_value_raw(_STD forward<_Ty>(_Val), &_Lock, _At_thread_exit);
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(788): message : see reference to class template instantiation 'std::_Associated_state<_Ty>' being compiled
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(788): message : with
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(788): message : [
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(788): message : _Ty=info_xml_creator::output::prepared_info
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(788): message : ]
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(788): message : return _Assoc_state->_Get_value(_Get_only_once);
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(783): message : while compiling class template member function '_Ty &std::_State_manager<_Ty>::_Get_value(void) const'
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(783): message : with
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(783): message : [
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(783): message : _Ty=info_xml_creator::output::prepared_info
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(783): message : ]
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(783): message : _Ty& _Get_value() const {
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(899): message : see reference to function template instantiation '_Ty &std::_State_manager<_Ty>::_Get_value(void) const' being compiled
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(899): message : with
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(899): message : [
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(899): message : _Ty=info_xml_creator::output::prepared_info
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(899): message : ]
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(899): message : return _STD move(_Local._Get_value());
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(875): message : see reference to class template instantiation 'std::_State_manager<_Ty>' being compiled
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(875): message : with
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(875): message : [
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(875): message : _Ty=info_xml_creator::output::prepared_info
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(875): message : ]
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\future(875): message : class future : public _State_manager<_Ty> {
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\deque(552): message : see reference to class template instantiation 'std::future<info_xml_creator::output::prepared_info>' being compiled
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\deque(552): message : static constexpr size_t _Bytes = sizeof(value_type);
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\deque(595): message : see reference to class template instantiation 'std::_Deque_val<std::_Deque_simple_types<_Ty>>' being compiled
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\deque(595): message : with
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\deque(595): message : [
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\deque(595): message : _Ty=std::future<info_xml_creator::output::prepared_info>
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\deque(595): message : ]
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\deque(595): message : static constexpr int _Block_size = _Scary_val::_Block_size;
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\queue(27): message : see reference to class template instantiation 'std::deque<std::future<info_xml_creator::output::prepared_info>,std::allocator<std::future<info_xml_creator::output::prepared_info>>>' being compiled
36>C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\queue(27): message : using value_type = typename _Container::value_type;
40>ssfindo.cpp
36>C:\msys64\home\friedkiwi\mame-upstream\src\frontend\mame\infoxml.cpp(466): message : see reference to class template instantiation 'std::queue<std::future<info_xml_creator::output::prepared_info>,std::deque<std::future<info_xml_creator::output::prepared_info>,std::allocator<std::future<info_xml_creator::output::prepared_info>>>>' being compiled
36>C:\msys64\home\friedkiwi\mame-upstream\src\frontend\mame\infoxml.cpp(466): message : std::queue<std::future<prepared_info>> tasks;
There was a problem hiding this comment.
So is it instantiating the copy assignment operator when a std::queue is instantiated there? I don’t see any code where it tries assigning an lvalue reference to a queue element – it only emplaces default-constructed elements, and sets elements with rvalues returned by the task lambda.
The only copy I can see is at line 517, but that’s a copy of an rvalue anyway. The local pi is constructed with the result of std::future::get which returns by value.
Is this a regression in a new version of the Microsoft C++ standard library? If it is, it needs to be reported.
src/frontend/mame/ui/icorender.cpp
Outdated
| @@ -179,7 +179,7 @@ bool load_ico_image(util::core_file &fp, unsigned count, unsigned index, bitmap_ | |||
| { | |||
| // read the directory entry | |||
| std::error_condition err; | |||
| size_t actual; | |||
| size_t actual = 0; | |||
There was a problem hiding this comment.
The trouble with this is it will mask other issues when running under a memory analyser. The variable can’t be used uninitialised:
- If
erris clear at line 185,fp.readwill setactual. actualwill only be checked at line 187 iferris clear – you can’t get here witherrclear iffp.readhas not been called.
If you initialise the variable like this, then it can mask bugs where a code path allows actual to be used without being set because the zero initialisation counts as being set for the purpose of the memory analyser.
There was a problem hiding this comment.
While I do agree with you, not doing so will result in a C2220 error (warnings treated as errors) because it triggers a C4701 (potentially uninitalised variable 'actual' used) warning on line 220. This breaks the build on MSVC 2019
There was a problem hiding this comment.
MSVC has lots of issues with spurious C2220 – is it not possible to configure it to not treat C2220 as an error? As far as I know our default settings for MSVC projects don’t treat warnings as errors at all because of spurious errors like this.
There was a problem hiding this comment.
I'm using the project files generated using make vs2019 as per the documentation, maybe the project files generated contain the /WX parameter somewhere? If so, that's another bug.
There was a problem hiding this comment.
I don’t think we’ve ever been able to build MAME with MSVC with warnings as errors. You can probably force errors as warnings off by adding NOWERROR=1 to the make command when generating project files. I’m not sure what the defaults are since I rarely use MSVC to build MAME. (It’s slow to build, uses a lot of RAM, and produces slow code.)
You may have better luck using the clang-cl toolchain with Visual Studio, but if there are issues with the Microsoft standard library, that will need to be resolved for it to work with clang-cl as well.
There was a problem hiding this comment.
I essentially followed the instructions on https://docs.mamedev.org/initialsetup/compilingmame.html#building-with-microsoft-visual-studio ; I'll try again with the NOWERROR=1 option. I ran into this while trying to add Sun4/25 support after having dumped the ROM of the machine I have.
src/frontend/mame/ui/icorender.cpp
Outdated
| size_t actual = 0; | ||
| icon_dir_t header{}; |
There was a problem hiding this comment.
Same comment as the other case with actual:
- If
erris clear at line 218,fp.readwill setactual. actualwill only be checked at line 220 iferris clear – you can’t get here witherrclear iffp.readhas not been called.
Additionally, default-initialising header is unnecessary work when it won’t be used if fp.read fails or returns short.
There was a problem hiding this comment.
Not default initilising it causes MSVC to bail again with a 'warnings as errors' message because it thinks it might be used.
There was a problem hiding this comment.
Yes, but once again, that’s a problem with MSVC, and this code is in the hot path for getting stuff on the screen in the internal UI, because it’s used for parsing DIB data in ICO files for the system and software selection lists.
We don’t treat GCC unused attribute warnings as errors because GCC produces spurious warnings. These spurious warnings need to be treated the same way.
|
~~I'm currently rebuilding the upstream branch to reproduce the exact error messages, will update in a bit. ~~ Comments added. |
|
Can you make a separate PR for this commit? It was unrelated to build error fixes: |
I'm on this, and it wasn't finished yet either - I accidentally pushed it to the same branch as this. Currently getting somewhere with Sun4/25 support using the ROM dumped from one of my machines (there weren't any dumps yet). |
Due to warnings being treated as errors in some files, the latest version of VS2019 and MSVC (16.11.4) reports warnings that result in compilation errors.
The types of things that have been changed are:
src/frontend/mame/infoxml.cpp.