Modify

Ticket #7100 (closed Bugs: fixed)

Opened 22 months ago

Last modified 22 months ago

unorderd_set<shared_ptr<T> > > - insert does not increment count

Reported by: Rich Eakin <reakinator@…> Owned by: danieljames
Milestone: Boost 1.51.0 Component: unordered
Version: Boost 1.50.0 Severity: Problem
Keywords: unordered_set shared_ptr Cc:

Description

It seems unordered_set::insert has stopped working for shared_ptr's between v 1.48 and now. Consider the following example:

#include <iostream>

#define USING_BOOST 1

#if USING_LIBCPP

	#include <unordered_set>
	#include <memory>

#elif USING_BOOST

	#include "boost/shared_ptr.hpp"
	#include "boost/unordered_set.hpp"

	using namespace boost;

#endif

using namespace std;

int main()
{
	unordered_set<shared_ptr<int> > mySet;
	shared_ptr<int> intSP( new int( 1 ) );

	cout << "use_count A: " << intSP.use_count() << endl;
	
	mySet.insert( intSP );

	cout << "use_count B: " << intSP.use_count() << endl;
 
    return 0;
}

In boost-trunk (and release version 1.50), use_count after the insert is still 1. In boost version 1.48, the use_count is incremented by the insert to 2. This is also the case with libc++.

Sorry, I don't know what the results are in 1.49.

This was tested clang on Mac OS X:

$ clang --version
Apple clang version 4.0 (tags/Apple/clang-421.10.48) (based on LLVM 3.1svn)
Target: x86_64-apple-darwin11.4.0
Thread model: posix

Attachments

clang_error_nomatchingfunction.txt Download (10.0 KB) - added by Rich Eakin <reakinator@…> 22 months ago.
clang compile error log "No Matching Function.."

Change History

comment:1 Changed 22 months ago by johnmaddock

  • Owner changed from johnmaddock to danieljames
  • Component changed from TR1 to unordered

comment:2 Changed 22 months ago by danieljames

(In [79358]) Unordered: Fix using a C++03 allocator with C++11 compiler.

Because the nodes had an implicit constructor, the has_construct traits was detecting that the nodes could be constructed by construction then copy, which really wasn't wanted. Also add a check that nodes aren't been copy constructed to make sure this doesn't happen again. Refs #7100.

Changed 22 months ago by Rich Eakin <reakinator@…>

clang compile error log "No Matching Function.."

comment:3 Changed 22 months ago by Rich Eakin <reakinator@…>

Thanks for the fix, I can confirm that insert increments the ref count with boost::unordered_set<boost::shared_ptr<T< >.

However, I think it has created a problem when using std::tr1::shared_ptr. Albeit an odd setup, the following example produces a compile time error:

#include "boost/unordered_set.hpp"
#include <tr1/memory>

int main()
{
	boost::unordered_set<std::tr1::shared_ptr<int> > mySet;
	std::tr1::shared_ptr<int> intSP( new int( 1 ) );
	
	mySet.insert( intSP ); // <-- Error: (Semantic Issue) "no matching function for call to 'hash_value'"

    return 0;
}

I have attached the full error log from clang.

comment:4 Changed 22 months ago by danieljames

  • Status changed from new to assigned
  • Version changed from Boost 1.51.0 to Boost 1.50.0
  • Milestone changed from To Be Determined to Boost 1.51.0

I'm afraid that's a feature, not a bug. There's no support for std::shared_ptr or std::tr1::shared_ptr, older versions were accidentally picking up the implicit conversion to bool and hashing based on that - which is really bad as it only returns two possible hash values. So 1.51 is going to prevent that happening.

I might add support for std::shared_ptr in a future version, but if you really to use it, the best thing to do is to use a custom hash function, something like (untested):

struct hash_shared_ptr {
    std::size_t operator()(std::tr1::shared_ptr<int> const& x) {
        boost::hash<int*> hf;
        return hf(x.get());
    }
};

If you really need boost::hash to support shared_ptr, then you probably should specialize it.

Btw. you probably know this, but when you use a shared_ptr as a set element, it stores it based on the address, not the value it points to. I just thought I should mention it as people sometimes get confused.

comment:5 Changed 22 months ago by Rich Eakin <reakinator@…>

Ah, right you are! I had forgotten about that and had a bug in my own code. That must mean that std::unordered_set is also using the implicit bool to hash? It is really nice that you can catch it, as it is almost certainly not what was intended.

Btw, yes the goal is to map unique objects based on their address. Thank you nonetheless for your kind and thorough advice!

All is working well now, cheers.

comment:6 Changed 22 months ago by danieljames

No, std::hash is specialized for std::shared_ptr, so I should definitely do the same. It doesn't have this problem, it's an unintended consequence of boost::hash's extensions. I think I'll add support for std::shared_ptr in 1.51, I am lagging behind a bit with C++11 support.

comment:7 Changed 22 months ago by danieljames

  • Status changed from assigned to closed
  • Resolution set to fixed

(In [79547]) Unordered: Merge allocator fix + improved tests. Fixes #7100.

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.