martinmoene / expected-lite

expected lite - Expected objects in C++11 and later in a single-file header-only library
Boost Software License 1.0
389 stars 35 forks source link

Fix r-value ref miscompile on MSVC v19.22. #41

Closed jacobly0 closed 2 years ago

jacobly0 commented 2 years ago

Returning an r-value reference through the comma operator is miscompiled by MSVC v19.22, as evidenced here.

Examples of tests failures without the expected.hpp changes on MSVC v19.16:

expected-lite\test\expected.t.cpp(1812): failed: pr-41: *std::move(a) == 7 for -858993460 == 7
expected-lite\test\expected.t.cpp(1813): failed: pr-41: std::move(b).error() == 7 for -858993460 == 7
expected-lite\test\expected.t.cpp(1814): failed: pr-41: std::move(c).error() == 7 for -858993460 == 7
martinmoene commented 2 years ago

Thanks for your PR.

The comma expression is/was used for C++11 constexpr-ness.

Correctness comes first naturally.

Have to think a bit what approach I'd like.

jacobly0 commented 2 years ago

That's what I get for not checking clang warnings. Since l-value refs aren't affected, the simplest solution is to just rearrange the expression.