some refactoring

Foundation/include/Poco/Any.h

@@ -60,6 +60,8 @@ class Any
 
 #ifndef POCO_NO_SOO
 
+	union PH;
+
 public:
 
 	Any()
@@ -81,7 +83,8 @@ public:
 	Any(const Any& other)
 		/// Copy constructor, works with both empty and initialized Any values.
 	{
-		construct(other);
+		if ((this != &other) && !other.empty())
+			construct(other);
 	}
 
 	~Any()
@@ -90,7 +93,7 @@ public:
 	{
 		if(!empty())
 		{
-			if(isLocal())
+			if(_placeholder.isLocal())
 				content()->~Placeholder();
 			else
 				delete content();
@@ -100,18 +103,20 @@ public:
 	Any& swap(Any& other)
 		/// Swaps the content of the two Anys.
 		/// 
-		/// When small object optimizaton (SOO) is enabled,
+		/// When small object optimizaton is enabled,
 		/// swap is only exception-safe when both (*this and
 		/// other) objects are allocated on the heap.
 	{
-		if (!isLocal() && !other.isLocal())
+		if (this == &other) return *this;
+
+		if (!_placeholder.isLocal() && !other._placeholder.isLocal())
 		{
 			std::swap(_placeholder.pHolder, other._placeholder.pHolder);
 		}
 		else
 		{
 			Any tmp(*this);
-			if (isLocal()) this->~Any();
+			if (_placeholder.isLocal()) this->~Any();
 			construct(other);
 			other = tmp;
 		}
@@ -127,14 +132,18 @@ public:
 		///   Any a = 13; 
 		///   Any a = string("12345");
 	{
-		Any(rhs).swap(*this);
+		construct(rhs);
 		return *this;
 	}
 	
-	Any& operator = (Any rhs)
+	Any& operator = (const Any& rhs)
 		/// Assignment operator for Any.
 	{
-		Any(rhs).swap(*this);
+		if ((this != &rhs) && !rhs.empty())
+			construct(rhs);
+		else if ((this != &rhs) && rhs.empty())
+			_placeholder.erase();
+
 		return *this;
 	}
 	
@@ -165,7 +174,7 @@ private:
 		}
 
 		virtual const std::type_info & type() const = 0;
-		virtual void clone(Placeholder**) const = 0;
+		virtual void clone(Any::PH*) const = 0;
 	};
 
 	template<typename ValueType>
@@ -181,22 +190,17 @@ private:
 			return typeid(ValueType);
 		}
 
-		static void setFlag(Placeholder** pMem, unsigned char val)
+		virtual void clone(Any::PH* pPlaceholder) const
-		{
-			reinterpret_cast<unsigned char*>(pMem)[POCO_SMALL_OBJECT_SIZE] = val;
-		}
-
-		virtual void clone(Placeholder** ppMem) const
 		{
 			if ((sizeof(Holder<ValueType>) <= POCO_SMALL_OBJECT_SIZE))
 			{
-				new (ppMem) Holder(_held);
+				new ((Placeholder*) pPlaceholder->holder) Holder(_held);
-				setFlag(ppMem, 1);
+				pPlaceholder->setLocal(true);
 			}
 			else
 			{
-				*ppMem = new Holder(_held);
+				pPlaceholder->pHolder = new Holder(_held);
-				setFlag(ppMem, 0);
+				pPlaceholder->setLocal(false);
 			}
 		}
 
@@ -208,10 +212,7 @@ private:
 
 	Placeholder* content() const
 	{
-		if(isLocal())
+		return _placeholder.content();
-			return reinterpret_cast<Placeholder*>(&(_placeholder.holder));
-		else
-			return _placeholder.pHolder;
 	}
 
 	template<typename ValueType>
@@ -220,26 +221,21 @@ private:
 		if (sizeof(Holder<ValueType>) <= POCO_SMALL_OBJECT_SIZE)
 		{
 			new (_placeholder.holder) Holder<ValueType>(value);
-			isLocal() = 1;
+			_placeholder.setLocal(true);
 		}
 		else
 		{
 			_placeholder.pHolder = new Holder<ValueType>(value);
-			isLocal() = 0;
+			_placeholder.setLocal(false);
 		}
 	}
 
 	void construct(const Any& other)
 	{
-		if(other.empty())
+		if(!other.empty())
-			erase(_placeholder.holder);
+			other.content()->clone(&_placeholder);
 		else
-			other.content()->clone(&_placeholder.pHolder);
+			_placeholder.erase();
-	}
-	
-	inline static void erase(unsigned char* pHolder, std::size_t sz= POCO_SMALL_OBJECT_SIZE + 1)
-	{
-		std::memset(pHolder, 0, sz);
 	}
 
 	union PH
@@ -251,18 +247,36 @@ private:
 	{
 		PH ()
 		{
-			erase(holder);
+			erase();
+		}
+
+		void erase()
+		{
+			std::memset(holder, 0, sizeof(PH));
+		}
+
+		bool isLocal() const
+		{
+			return holder[POCO_SMALL_OBJECT_SIZE] != 0;
+		}
+
+		void setLocal(bool local) const
+		{
+			holder[POCO_SMALL_OBJECT_SIZE] = local ? 1 : 0;
+		}
+
+		Placeholder* content() const
+		{
+			if(isLocal())
+				return reinterpret_cast<Placeholder*>(&holder[0]);
+			else
+				return pHolder;
 		}
 
 		Placeholder*          pHolder;
 		mutable unsigned char holder[POCO_SMALL_OBJECT_SIZE + 1];
 	} _placeholder;
 
-	unsigned char& isLocal() const
-	{
-		return _placeholder.holder[POCO_SMALL_OBJECT_SIZE];
-	}
-
 
 #else // if POCO_NO_SOO
 

Foundation/testsuite/src/AnyTest.cpp

@@ -103,7 +103,7 @@ void AnyTest::testCopyCtor()
 	assert (original.type() == copy.type());
 	assert (AnyCast<std::string>(original) == AnyCast<std::string>(copy));
 	assert (text == AnyCast<std::string>(copy));
-	assert(AnyCast<std::string>(&original) != AnyCast<std::string>(&copy));
+	assert (AnyCast<std::string>(&original) != AnyCast<std::string>(&copy));
 }
 
 
@@ -113,12 +113,19 @@ void AnyTest::testCopyAssign()
 	Any original = text, copy;
 	Any* assignResult = &(copy = original);
 	
-	assert(!copy.empty());
+	assert (!copy.empty());
-	assert(original.type() == copy.type());
+	assert (original.type() == copy.type());
-	assert(AnyCast<std::string>(original) == AnyCast<std::string>(copy));
+	assert (AnyCast<std::string>(original) == AnyCast<std::string>(copy));
-	assert(text == AnyCast<std::string>(copy));
+	assert (text == AnyCast<std::string>(copy));
-	assert(AnyCast<std::string>(&original) != AnyCast<std::string>(&copy));
+	assert (AnyCast<std::string>(&original) != AnyCast<std::string>(&copy));
-	assert(assignResult == &copy);
+	assert (assignResult == &copy);
+
+	// test self assignment
+	Any& ref = original;
+	original = ref;
+	assert (AnyCast<std::string>(original) == AnyCast<std::string>(copy));
+	original = original;
+	assert (AnyCast<std::string>(original) == AnyCast<std::string>(copy));
 }
 
 
@@ -129,12 +136,12 @@ void AnyTest::testConvertingAssign()
 	Any* assignResult = &(value = text);
 	
 	assert (!value.empty());
-	assert(value.type() == typeid(std::string));
+	assert (value.type() == typeid(std::string));
-	assert(0 == AnyCast<int>(&value));
+	assert (0 == AnyCast<int>(&value));
-	assert(0 != AnyCast<std::string>(&value));
+	assert (0 != AnyCast<std::string>(&value));
-	assert(AnyCast<std::string>(value) == text);
+	assert (AnyCast<std::string>(value) == text);
-	assert(AnyCast<std::string>(&value) != &text);
+	assert (AnyCast<std::string>(&value) != &text);
-	assert(assignResult == &value);
+	assert (assignResult == &value);
 }
 
 
@@ -203,7 +210,9 @@ void AnyTest::testSwap()
 	assert (swapped.type() == typeid(std::string));
 	assert (text == AnyCast<std::string>(swapped));
 	assert (0 != originalPtr);
+#ifdef POCO_NO_SOO // pointers only match when heap-allocated
 	assert (originalPtr == AnyCast<std::string>(&swapped));
+#endif
 	assert (swapResult == &original);
 }