openexr icon indicating copy to clipboard operation
openexr copied to clipboard

IlmImfTest failures on Windows

Open darbyjohnston opened this issue 5 years ago • 4 comments

Hi,

I'm running into two test failures in IlmImfTest with Windows 10 and Visual Studio 2019. The failure is in testTypedAttribute(), on lines 759 and 792. It looks like the tests are checking that the various copy/move constructors are working, but for some reason the results are different with VS2019 than what happens on Linux and macOS (maybe due to C++ "return value optimization"? I'm definitely not an expert...). Is looks like the tests are expecting this sequence of operations:

default constructor
move assignment operator
destructor

But this is what I'm seeing instead:

default constructor
move constructor
destructor
move assignment operator
destructor

Interestingly if I change the code from:

static TestType func()
{
    TestType t;
    return t;
}

To this:

static TestType func()
{
    return TestType();
}

The tests complete successfully, but I'm not sure if that is a good fix. I did some searching on RVO and it seems the behavior might be different for C++ versions, so I also tried recompiling with C++11, C++14, and C++17, but that didn't seem to help.

darbyjohnston avatar Aug 08 '20 02:08 darbyjohnston

Maybe the thing to do is to explicitly test copy constructors by invoking them directly, and to test move via std::move?

meshula avatar Aug 08 '20 23:08 meshula

Thanks Nick, I'll try that and submit a PR.

darbyjohnston avatar Aug 11 '20 17:08 darbyjohnston

Hi, I'm seeing similar failures (using VS2019 16.9.x and 16.11 preview 3), in the same function, albeit in a slightly different location.

The underlying cause is that the tests (seemingly) expect NRVO to get applied, which is an optional optimization -- and it seems that, at least in this case, MSVC chooses not to apply it in Debug builds.

arekz avatar Aug 04 '21 14:08 arekz

I think the resolution should be either that the tests run in Release mode, or that the test be ifdef'd for _MSC_VER and DEBUG.

meshula avatar Aug 04 '21 16:08 meshula