Fix for Windows compiler, missing object initializer #4188
Conversation
|
One question - is it possible to port this change to 2.10.0 or add to future fix version? |
| @@ -1037,7 +1037,7 @@ template <typename... Args> | |||
| || (defined(__clang__) && __clang_major__ == 5) | |||
| static constexpr detail::overload_cast_impl<Args...> overload_cast = {}; | |||
| # else | |||
| static constexpr detail::overload_cast_impl<Args...> overload_cast; | |||
| static constexpr detail::overload_cast_impl<Args...> overload_cast{}; | |||
There was a problem hiding this comment.
Uhm ... wait, do we actually need this ugly #ifdef?
There was a problem hiding this comment.
You mean maybe the new version works on all compilers?
There was a problem hiding this comment.
Also why weren't we seeing the errors with newer MSVC? We should be testing with 2019 & 2022 (maybe not 2017 anywhere anymore).
There was a problem hiding this comment.
You mean maybe the new version works on all compilers?
Yes, I tried it out, CI is green: #4190
Whatever made the #ifdef necessary in the past, it doesn't seem to be an issue anymore with the compilers we support today.
There was a problem hiding this comment.
Uhm ... wait, do we actually need this ugly #ifdef?
Actually if the {} is present, this #ifdef can be removed. I will cover that in next commit and run checks.
Also why weren't we seeing the errors with newer MSVC?
My bad, I have got lost in the logs of the project and actual MSVC build version is 1924. Maybe this version is not covered? Also I see that -DCMAKE_CXX_STANDARD=17/20 is set for Windows builds, is C++14 covered as well?
There was a problem hiding this comment.
I bet no one ever tried it with {}, but only with = {}.
I'm hoping to get a patch release out in ~1 week or so. Still have a few items on #4125 left.
There was a problem hiding this comment.
@henryiii Thanks for the answer, can't wait to finally bump version in OpenVINO!
There was a problem hiding this comment.
There was a problem hiding this comment.
It should. I think there might be one or two breaking changes which could go in before it releases, which means we'll branch from there, but so far master is 2.10.1. I'm going to finish investigation into the CMake caching issue in the next couple of days, then make a release.
There was a problem hiding this comment.
Missed my target, but 2.10.1 is out now.
|
@rwgk Did we actually test if this works C++14 on MSVC? Seems like our CI might not be running that edge case? |
|
Opened #4191 to make sure |
Description
Added missing object initializer due to error while compiling with Visual Studio 2019 MSC v. 1924:
Suggested changelog entry:
* Fix MSVC 2019 v.1924 & C++14 mode error for `overload_cast`.