Using unique_ptr with VS2017

Question

I have a couple of questions relating to this code but I will ask them seperately.

I am familar with the concept of smart pointers as I have used them before with different libraries (like Teigha for DGN). However, recently a comment was made to me here on StackOverflow that I shoudl consider using unique_ptr instead of new and delete.

I have been working on my own code to add menu item bitmaps to my program and it is going well. I thought it would be a good candidate for unique pointers.

I have a map:

using MenuBitmapsMap = map<UINT, CBitmap*>;

And I have declared that as an variable in my class. Then I am calling this method multiple times to build the map and update my menu:

void CChristianLifeMinistryEditorDlg::UpdateMenuBitmap(CMenu *pMenu, UINT uCommandID, UINT uBMPResource, bool bDisabled /*false*/)
{
    CBitmap *pBitmap = new CBitmap;                       // Create a new CBitmap pointer
    if (pBitmap == nullptr)
        return;

    if (pBitmap->LoadBitmap(uBMPResource) == NULL)        // Load the image from the resources
    {
        delete pBitmap;
        return;
    }

    theApp.SetBitmapBackgroundAsMenuColour(*pBitmap);        // Update the image background
    m_mapMenuBitmap.emplace(uCommandID, pBitmap);            // Add the menu / bitmap pair to the map

    if (!pMenu->SetMenuItemBitmaps(uCommandID, MF_BYCOMMAND, // Update the menu bitmaps
        m_mapMenuBitmap.at(uCommandID),
        m_mapMenuBitmap.at(uCommandID)))
    {
        // #UpdateMenuBitmap Failed to set the menu item bitmaps
    }
}

It works fine:

I don't apprear to have memory leaks anyway because I understand that the map takes ownership of the CBitmap* and deletes them itself.

But I was trying to use:

unique_ptr<CBitmap> pBitmap = make_unique<CBitmap>;

I have added #include <memory> but I get this error:

no suitable constructor exists to convert from "" to "std::unique_ptr>"

Update

I changed my map to:

using MenuBitmapsMap = map<UINT, unique_ptr<CBitmap>>;

And in my function:

unique_ptr<CBitmap> pBitmap = make_unique<CBitmap>();

That all compiles now. But now I can't do this bit:

if (!pMenu->SetMenuItemBitmaps(uCommandID, MF_BYCOMMAND, // Update the menu bitmaps
    m_mapMenuBitmap.at(uCommandID),
    m_mapMenuBitmap.at(uCommandID)))
{
    // #UpdateMenuBitmap Failed to set the menu item bitmaps
}

no suitable conversion function from "std::unique_ptr<CBitmap, std::default_delete<CBitmap>>" to "const CBitmap *" exists

SetMenuItemBitmaps expects CBitmap * parameters. It seems to me to be easier to stay with CBitmap* and using new.

Confused.

Update

If I change this bit:

if (!pMenu->SetMenuItemBitmaps(uCommandID,
    bByPosition ? MF_BYPOSITION : MF_BYCOMMAND,      // Update the menu bitmaps
    m_mapMenuBitmap.at(uCommandID).get(),
    m_mapMenuBitmap.at(uCommandID).get()))
{
    // #UpdateMenuBitmap Failed to set the menu item bitmaps
}

... so now it uses the get() method I get more compiler errors:

4>c:\program files (x86)\microsoft visual studio\2017\community\vc\tools\msvc\14.13.26128\include\xmemory0(919): error C2661: 'std::pair<const _Kty,_Ty>::pair': no overloaded function takes 2 arguments
4>        with
4>        [
4>            _Kty=UINT,
4>            _Ty=std::unique_ptr<CBitmap,std::default_delete<CBitmap>>
4>        ]
4>c:\program files (x86)\microsoft visual studio\2017\community\vc\tools\msvc\14.13.26128\include\xtree(774): note: see reference to function template instantiation 'void std::_Default_allocator_traits<_Alloc>::construct<_Ty,UINT&,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>&>(_Alloc &,_Objty *const ,UINT &,std::unique_ptr<CBitmap,std::default_delete<CBitmap>> &)' being compiled
4>        with
4>        [
4>            _Alloc=std::allocator<std::_Tree_node<std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>,std::_Default_allocator_traits<std::allocator<std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>>>::void_pointer>>,
4>            _Ty=std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>,
4>            _Objty=std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>
4>        ]
4>c:\program files (x86)\microsoft visual studio\2017\community\vc\tools\msvc\14.13.26128\include\xtree(773): note: see reference to function template instantiation 'void std::_Default_allocator_traits<_Alloc>::construct<_Ty,UINT&,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>&>(_Alloc &,_Objty *const ,UINT &,std::unique_ptr<CBitmap,std::default_delete<CBitmap>> &)' being compiled
4>        with
4>        [
4>            _Alloc=std::allocator<std::_Tree_node<std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>,std::_Default_allocator_traits<std::allocator<std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>>>::void_pointer>>,
4>            _Ty=std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>,
4>            _Objty=std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>
4>        ]
4>c:\program files (x86)\microsoft visual studio\2017\community\vc\tools\msvc\14.13.26128\include\xtree(961): note: see reference to function template instantiation 'std::_Tree_node<_Ty,std::_Default_allocator_traits<_Alloc>::void_pointer> *std::_Tree_comp_alloc<_Traits>::_Buynode<UINT&,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>&>(UINT &,std::unique_ptr<CBitmap,std::default_delete<CBitmap>> &)' being compiled
4>        with
4>        [
4>            _Ty=std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>,
4>            _Alloc=std::allocator<std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>>,
4>            _Traits=std::_Tmap_traits<UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>,std::less<UINT>,std::allocator<std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>>,false>
4>        ]
4>c:\program files (x86)\microsoft visual studio\2017\community\vc\tools\msvc\14.13.26128\include\xtree(961): note: see reference to function template instantiation 'std::_Tree_node<_Ty,std::_Default_allocator_traits<_Alloc>::void_pointer> *std::_Tree_comp_alloc<_Traits>::_Buynode<UINT&,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>&>(UINT &,std::unique_ptr<CBitmap,std::default_delete<CBitmap>> &)' being compiled
4>        with
4>        [
4>            _Ty=std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>,
4>            _Alloc=std::allocator<std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>>,
4>            _Traits=std::_Tmap_traits<UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>,std::less<UINT>,std::allocator<std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>>,false>
4>        ]
4>d:\my programs\2017\meetschedassist\meeting schedule assistant\christianlifeministryeditordlg.cpp(6204): note: see reference to function template instantiation 'std::pair<std::_Tree_iterator<std::_Tree_val<std::_Tree_simple_types<_Ty>>>,bool> std::_Tree<std::_Tmap_traits<_Kty,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>,_Pr,_Alloc,false>>::emplace<UINT&,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>&>(UINT &,std::unique_ptr<CBitmap,std::default_delete<CBitmap>> &)' being compiled
4>        with
4>        [
4>            _Ty=std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>,
4>            _Kty=UINT,
4>            _Pr=std::less<UINT>,
4>            _Alloc=std::allocator<std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>>
4>        ]
4>d:\my programs\2017\meetschedassist\meeting schedule assistant\christianlifeministryeditordlg.cpp(6204): note: see reference to function template instantiation 'std::pair<std::_Tree_iterator<std::_Tree_val<std::_Tree_simple_types<_Ty>>>,bool> std::_Tree<std::_Tmap_traits<_Kty,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>,_Pr,_Alloc,false>>::emplace<UINT&,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>&>(UINT &,std::unique_ptr<CBitmap,std::default_delete<CBitmap>> &)' being compiled
4>        with
4>        [
4>            _Ty=std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>,
4>            _Kty=UINT,
4>            _Pr=std::less<UINT>,
4>            _Alloc=std::allocator<std::pair<const UINT,std::unique_ptr<CBitmap,std::default_delete<CBitmap>>>>
4>        ]

It all boils down I think to:

std::pair<const _Kty,_Ty>::pair': no overloaded function takes 2 arguments

I don't get it.

Update

I saw this question which implies that using get() was the right thing to do to pass the CBitmap * objects. So I don't understand the complication failure.

As Requested

void CChristianLifeMinistryEditorDlg::UpdateMenuBitmap(CMenu *pMenu, UINT uCommandID, UINT uBMPResource, 
                                                       bool bByPosition /*false*/, bool bDisabled /*false*/)
{
    if (pMenu == nullptr)
        return;

    // See if the bitmap already exists. If it does, then delete it.
    // This will happen when we are changing the image state to/from grayscale.
    if (m_mapMenuBitmap.count(uCommandID) > 0)
        m_mapMenuBitmap.erase(uCommandID);

    unique_ptr<CBitmap> pBitmap = make_unique<CBitmap>();
    //CBitmap *pBitmap = new CBitmap;
    if (pBitmap == nullptr)
        return;

    if (pBitmap->LoadBitmap(uBMPResource) == NULL)
    {
        //delete pBitmap;
        return;
    }

    theApp.SetBitmapBackgroundAsMenuColour(*pBitmap);

    if (bDisabled)
        theApp.SetBitmapAsGrayScale(*pBitmap);

    m_mapMenuBitmap.emplace(uCommandID, pBitmap);

    if (!pMenu->SetMenuItemBitmaps(uCommandID,
                    bByPosition ? MF_BYPOSITION : MF_BYCOMMAND, 
                    m_mapMenuBitmap.at(uCommandID).get(),
                    m_mapMenuBitmap.at(uCommandID).get()))
    {
        // #UpdateMenuBitmap Failed to set the menu item bitmaps
    }
}

Line 6204 is the first if statement in the function.

Calling `std::unique_ptr::get()` seems to be right. Please include the code of "christianlifeministryeditordlg.cpp", line 6204 (maybe a few lines more to show context). — zett42, Apr 17 '18 at 17:12
@zett42 I found out it is the `emplace` function call. If I comment it out I get no warnings. So how to fix that? — Andrew Truckle, Apr 17 '18 at 18:14
@zett42 I found this answer: https://stackoverflow.com/a/21156152/2287576 I tried `m_mapMenuBitmap.emplace(uCommandID, move(pBitmap));` and it compiles. Testing now. — Andrew Truckle, Apr 17 '18 at 18:20
@zett42 Side comment: Based on that answer I linked to, does that mean I do not need my code that deletes the existing entry from the map first? — Andrew Truckle, Apr 17 '18 at 18:31
I see you already found out yourself. Yes, you have to `move()` the `unique_ptr` because it can only have a single owner. — zett42, Apr 17 '18 at 19:45
"_does that mean I do not need my code that deletes the existing entry from the map first?_" -- `std::map::emplace()` does not insert a new element, if an element with the same key already exists. So if you want to update existing elements, you have to do something like that. I think you could optimize it a bit. No need for `std::map::count()` because `std::map::erase()` already checks if the key exists. Call `std::map::erase()` only if you could not create or not load the new bitmap. Replace the `emplace()` call by `m_mapMenuBitmap[uCommandID] = std::move(pBitmap);` to insert or update. — zett42, Apr 17 '18 at 19:56
@zett42 No need for `count` nor `erase` because `m_mapMenuBitmap[uCommandID] = std::move(pBitmap);` like you said covers it all. :) — Andrew Truckle, Apr 17 '18 at 20:11
Thinking about this, you actually don't even need `std::unique_ptr`. Instead, `std::map` would be sufficient. Usage like `CBitmap& bitmap = m_mapMenuBitmap[uCommandID]; if(bitmap) bitmap.DeleteObject(); bitmap.LoadBitmap(uBMPResource);`. — zett42, Apr 18 '18 at 14:20
@zett42 The main thing I guess is that the bitmap pointers are valid for the life of the menu. It works with the unique pointers. — Andrew Truckle, Apr 18 '18 at 15:43
...to avoid the cast you could also use `bitmap.GetSafeHandle()` — zett42, Apr 19 '18 at 10:36

Using unique_ptr with VS2017

Update

Update

Update

As Requested

0 Answers0