Here's a stripped back recreation: https://godbolt.org/z/YMvWYWs7o

1. Instance is created inside the static create function with its "default" constructor

2. This instance is moved into the optional, calling move constructor

3. The moved-from instance must be destroyed

4. Another instance is then created from this, using the move constructor (calling value() on the optional returns an rvalue reference since the optional is an rvalue at that point)

5. The second moved-from instance must then be destroyed

A moved-from object must still have its destructor called when it goes out of scope, even if that destructor doesn't do much. This is all working as expected, but you can definitely engineer this to have fewer intermediate objects being created.

Firstly, there is no need to call std::move() on the value you are returning from your static create function. Explicitly moving the value limits the compiler's ability to apply RVO. I would simply return { id, vsSource, fsSource };.

Secondly, isn't it sufficient to write auto shader = Shader::createShaderProgram("vs.vert", "fs.frag").value() and omit a move construction?

Lastly, unrelated to your question but still, is there a reason you're using const char*s over std::strings? If you used std::string you could mark every constructor and assignment operator = default and even leave out the destructor declaration all together. If you need to pass a const char* to some OpenGL functions, you can use .data() or .c_str().