Is this behavior of std::expected with move-only types a MSVC bug or undefined behavior?

170 Views Asked by At

I've encountered a curious issue with my C++ code when compiling with Microsoft Visual Studio's MSVC compiler, and I'm trying to determine whether it's a compiler bug or undefined behavior according to the C++ standard.

The minimal reproducible example is below:

#include <utility>
#include <iostream>
#include <memory>
#include <expected>

struct UniqueHandle {
    UniqueHandle() { 
        val = nullptr; 
    }
    ~UniqueHandle() { 
        std::cout << "~UniqueHandle(" << val << ")" << std::endl; 
    }
    UniqueHandle(void* val) : val(val) {}
    UniqueHandle(UniqueHandle&& other) { 
        std::swap(val, other.val);
    }
    UniqueHandle& operator=(UniqueHandle&& other) {
        std::swap(val, other.val);
        return *this;
    }
    void* val;
};
std::expected<UniqueHandle, int> foo() {
    return UniqueHandle {};
}
int main() {
    auto exp_res = foo();
    UniqueHandle result {};
    if (exp_res) {
        result = std::move(exp_res.value());
    }
    return 0;
}

The program produces the following output with the VS 2022 toolchain in the Debug build:

~UniqueHandle(CCCCCCCCCCCCCCCC)
~UniqueHandle(0000000000000000)
~UniqueHandle(0000000000000000)

In the Release build, the first line is some garbage value. I fail to see where this value comes from. However, it works as expected when compiled by g++, live example here:

https://gcc.godbolt.org/z/zMh5W6MMj

Any insights or explanations would be greatly appreciated!

2

There are 2 best solutions below

0
On BEST ANSWER

In your move constructor this->val is uninitialised. The std::swap therefore sets other.val to this uninitialised value. MSVC's debug runtime sets uninitialised memory to 0xCCCCCCCC so this is the value that is printed. You can sort of see the same behaviour in GCC by adding a member initialiser: https://gcc.godbolt.org/z/hMa9Ka9cb

Move constructors can be implemented with std::exchange to help avoid this problem:

UniqueHandle(UniqueHandle&& other): val{std::exchange(other.val, nullptr)} { 
}

By always using member initialisers it's clearer that all members have been initialised before use and the new value for the moved from member is explicitly stated.

0
On

The accepted answer is the perfect solution. But I would use default+swap idiom(my made-up idiom) in general case:

void UniqueHandler::swap(UniqueHandler&);
UniqueHandler::UniqueHandler(UniqueHandler&& rvalue)
: UniqueHandler{/*delegate to default*/}
{ this->swap(rvalue); }; //swap

This is a use case of delegating constructors.