[BUILD] Introduce CXX 20 CI pipeline for MSVC/Windows #2450
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2450 +/- ##
=======================================
Coverage 87.06% 87.06%
=======================================
Files 199 199
Lines 6087 6087
=======================================
Hits 5299 5299
Misses 788 788 |
api/test/nostd/variant_test.cc
Outdated
| @@ -48,7 +48,7 @@ TEST(VariantTest, Get) | |||
| EXPECT_EQ(nostd::get<int>(w), 12); | |||
| EXPECT_EQ(*nostd::get_if<int>(&v), 12); | |||
| EXPECT_EQ(nostd::get_if<float>(&v), nullptr); | |||
| #if __EXCEPTIONS | |||
| #if __EXCEPTIONS || (defined(OPENTELEMETRY_STL_VERSION) && (OPENTELEMETRY_STL_VERSION >= 2020)) | |||
There was a problem hiding this comment.
std::variant is introduced in C++17, not C++20.
Please investigate (local CXX 17 MSVC build) if the logic should be OPENTELEMETRY_STL_VERSION >= 2017 instead.
Also, somehow the existing code passed in C++14, C++17, C++20 builds in CI with GCC.
Not sure which platform and/or which compiler do set __EXCEPTIONS, please clarify if you know the context.
There was a problem hiding this comment.
Found out that __EXCEPTIONS is set by GCC and Clang, except when building with -fno-exceptions.
This is likely why a GCC build with exceptions passed this test.
There was a problem hiding this comment.
Thanks for looking into it. Will try MSVC C++ 17 build.
There was a problem hiding this comment.
Verified that bad_variant_access exception was thrown out with MSVC/C++17. Thanks for the catch.
api/test/nostd/string_view_test.cc
Outdated
| #if __EXCEPTIONS || (defined(OPENTELEMETRY_STL_VERSION) && (OPENTELEMETRY_STL_VERSION >= 2020)) | ||
| EXPECT_THROW((void)s.substr(10), std::out_of_range); |
There was a problem hiding this comment.
I suspect the existing code was incorrect, as std::string_view is introduced in C++17, not C++20.
Please investigate (local CXX 17 MSVC build) if the logic should be OPENTELEMETRY_STL_VERSION >= 2017 instead.
There was a problem hiding this comment.
Verified that std::out_of_range exception was thrown out with MSVC/C++17. Thanks for the catch.
Fixes # (issue)
Changes
Please provide a brief description of the changes here.
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes