4

I have a class (A) with its copy constructors deleted (=delete;), move constructor and move assignment declared and defined. The class holds a resource that must not be freed twice, hence the move-only restriction.

I have another class (B) that holds an instance of A (not a pointer). I've written a move assignment operator for class B which calls std::move() on its member of class A.

A map holds many instances of B. I create an instance of B and assign its A member directly with assignment. Then I try to insert B into the map. I've tried operator[], insert and emplace in different ways, but I keep getting an error claiming I'm attempting to use a deleted function with the signature A(const A&). So it's trying to use the copy constructor despite it being deleted?

Everything is fine until I try to insert it into the map.

I'm using VS2013.

Edit: It's important for my project (probably unrelated to the problem in question) that the method I end up using does not perform any library interaction with the WinAPI (kernel32/ntdll etc.), so any methods calling operator new() or similar internally of any sort that delegates to the WinAPI I will be unable to use. The map has been pre-allocated with reserve() to try to alleviate this.

Code (the call to emplace is what causes the compilation error):

/* main.cpp */

#include "smart_handle.h"
#include <unordered_map>
#include <Windows.h>

struct MetadataStruct
{
    MetadataStruct& operator=(MetadataStruct&& other)
    {
        if (this != &other)
        {
            handle = std::move(other.handle);
            // Invalidate other.handle somehow...
        }
        return *this;
    }

    Util::SmartHandle handle;
    // Also has other members.
};

std::unordered_map<DWORD, MetadataStruct> metadataStructs;

Util::SmartHandle GetHandle()
{
    // Doing lots of stuff here to get the handle, then finally wraps
    // it before returning, hence the need for its own function.
    const HANDLE openedFileHandle = (HANDLE)-1; // Just an example handle.
    return std::move(Util::SmartHandle(openedFileHandle, NULL));
}

void F()
{
    MetadataStruct metadataStruct;
    metadataStruct.handle = GetHandle();

    metadataStructs.emplace(0, std::move(metadataStruct));
}

int main(int argc, char** argv)
{
    F();
    return 0;
}

/* smart_handle.h */
#pragma once

#include <stdexcept>
#include <Windows.h>

namespace Util
{
    class SmartHandle
    {
    public:
        SmartHandle() = default;
        SmartHandle(HANDLE handle, HANDLE invalidHandleValue);

        SmartHandle(const SmartHandle& other) = delete;
        SmartHandle& operator=(const SmartHandle& other) = delete;

        SmartHandle(SmartHandle&& other);
        SmartHandle& operator=(SmartHandle&& other);

        ~SmartHandle();

        HANDLE GetValue() const;
        bool IsValid() const;
        void Close();
    private:
        static const HANDLE uninitializedHandleValue;

        HANDLE handle = uninitializedHandleValue;
        HANDLE invalidHandleValue = uninitializedHandleValue;

        void Set(HANDLE handle, HANDLE invalidHandleValue, bool throwIfInvalid = true);
    };
}

/* smart_handle.cpp */

#include "smart_handle.h"

namespace Util
{
    const HANDLE SmartHandle::uninitializedHandleValue = (HANDLE)-2;

    SmartHandle::SmartHandle(const HANDLE handle, const HANDLE invalidHandleValue)
    {
        Set(handle, invalidHandleValue);
    }

    SmartHandle::SmartHandle(SmartHandle&& other)
    {
        handle = other.handle;
        invalidHandleValue = other.invalidHandleValue;

        other.handle = uninitializedHandleValue;
        other.invalidHandleValue = uninitializedHandleValue;
    }

    SmartHandle& SmartHandle::operator=(SmartHandle&& other)
    {
        if (this != &other)
        {
            handle = other.handle;
            invalidHandleValue = other.invalidHandleValue;

            other.handle = uninitializedHandleValue;
            other.invalidHandleValue = uninitializedHandleValue;
        }
        return *this;
    }

    SmartHandle::~SmartHandle()
    {
        Close();
    }

    void SmartHandle::Set(const HANDLE handle, const HANDLE invalidHandleValue, const bool throwIfInvalid)
    {
        this->handle = handle;
        this->invalidHandleValue = invalidHandleValue;

        if (throwIfInvalid && !IsValid())
        {
            this->handle = uninitializedHandleValue;
            this->invalidHandleValue = uninitializedHandleValue;
            throw std::invalid_argument("The handle used to initialize the object is not a valid handle");
        }
    }

    HANDLE SmartHandle::GetValue() const
    {
        if (handle == uninitializedHandleValue)
        {
            throw std::exception("Handle value not initialized");
        }

        return handle;
    }

    bool SmartHandle::IsValid() const
    {
        return handle != uninitializedHandleValue && handle != invalidHandleValue;
    }

    void SmartHandle::Close()
    {
        const bool isPseudoHandle = handle == (HANDLE)-1;

        if (IsValid() && !isPseudoHandle)
        {
            if (!CloseHandle(handle))
            {
                throw std::exception("CloseHandle failed");
            }

            handle = invalidHandleValue;
        }
    }
}
1
  • A side note: there are simpler ways to manage a resource. They may require a bit more code (not much), but you won't have to put up with non-copyable objects. The simplest way (and the one requiring even less code): wrap your resource into std::shared_ptr. It will take of everything for you. Commented Feb 8, 2017 at 12:47

1 Answer 1

7

You can use emplace but you need to use std::move in conjunction with it in order to cast the object you already have into an rvalue reference. std::unique_ptr is only movable and you can put it into a map just like

int main() {
    std::unique_ptr<int> foo;
    std::map<int, std::unique_ptr<int>> bar;
    bar.emplace(1, std::move(foo));
}
Sign up to request clarification or add additional context in comments.

6 Comments

But unique_ptr only works with pointers, right? When I try to encapsulate object A in a smart pointer it tells me it doesn't support it, but if I prepend the object creation with new then it works. I need to create the object on the stack so I can't use new.
@Mikubyte The std::unique_ptr is on the stack. In the code I never used new. This is just an example since you didn't show any code. If you update your question with what you actually have I can give you a more tailored code block.
Added some example code which captures the gist of what I'm trying to do.
@Mikubyte Okay. So what you need is m.emplace(..., std::move(b)); instead of m.emplace(..., b); That will move what b holds into the map and leave b in a valid/unspecified but destructible state.
@Mikubyte Your code has a lot of errors and I cannot get it to compile. After doing what I could I get this which does work. I can't really help further without an minimal reproducible example
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.