DEV-43622 : API change (no functionality change) to fix a design error in LLSD

I made it about a year and a half ago; Zero found it while reading code. I had added a return value to LLSD::insert(), but a) did it wrong, and b) broke the STL-like semantics of insert(). So I've put insert() back to returning void and created LLSD::with(), which does what my earlier insert() did. The compiler then caught all the cases where insert()'s return value were being used, and I changed those to use with() instead.

2 files changed, 14 insertions, 24 deletions

diff --git a/indra/llcommon/llsd.cpp b/indra/llcommon/llsd.cpp
index 9140ebb3f3..c863d4e266 100644
--- a/indra/llcommon/llsd.cpp
+++ b/indra/llcommon/llsd.cpp
@@ -355,7 +355,7 @@ namespace
 		using LLSD::Impl::erase; // Unhiding erase(LLSD::Integer)
 		using LLSD::Impl::ref; // Unhiding ref(LLSD::Integer)
 		virtual LLSD get(const LLSD::String&) const; 
-		        LLSD& insert(const LLSD::String& k, const LLSD& v);
+		void insert(const LLSD::String& k, const LLSD& v);
 		virtual void erase(const LLSD::String&);
 		              LLSD& ref(const LLSD::String&);
 		virtual const LLSD& ref(const LLSD::String&) const;
@@ -394,14 +394,9 @@ namespace
 		return (i != mData.end()) ? i->second : LLSD();
 	}
 	
-	LLSD& ImplMap::insert(const LLSD::String& k, const LLSD& v)
+	void ImplMap::insert(const LLSD::String& k, const LLSD& v)
 	{
 		mData.insert(DataMap::value_type(k, v));
-		#ifdef LL_MSVC7
-			return *((LLSD*)this);
-		#else
-			return *dynamic_cast<LLSD*>(this);
-		#endif
 	}
 	
 	void ImplMap::erase(const LLSD::String& k)
@@ -450,7 +445,7 @@ namespace
 		virtual int size() const; 
 		virtual LLSD get(LLSD::Integer) const;
 		        void set(LLSD::Integer, const LLSD&);
-		        LLSD& insert(LLSD::Integer, const LLSD&);
+		        void insert(LLSD::Integer, const LLSD&);
 		        void append(const LLSD&);
 		virtual void erase(LLSD::Integer);
 		              LLSD& ref(LLSD::Integer);
@@ -499,14 +494,10 @@ namespace
 		mData[index] = v;
 	}
 	
-	LLSD& ImplArray::insert(LLSD::Integer i, const LLSD& v)
+	void ImplArray::insert(LLSD::Integer i, const LLSD& v)
 	{
 		if (i < 0) {
-			#ifdef LL_MSVC7
-				return *((LLSD*)this);
-			#else
-				return *dynamic_cast<LLSD*>(this);
-			#endif
+			return;
 		}
 		DataVector::size_type index = i;
 		
@@ -516,11 +507,6 @@ namespace
 		}
 		
 		mData.insert(mData.begin() + index, v);
-		#ifdef LL_MSVC7
-			return *((LLSD*)this);
-		#else
-			return *dynamic_cast<LLSD*>(this);
-		#endif
 	}
 	
 	void ImplArray::append(const LLSD& v)
@@ -763,11 +749,12 @@ LLSD LLSD::emptyMap()
 
 bool LLSD::has(const String& k) const	{ return safe(impl).has(k); }
 LLSD LLSD::get(const String& k) const	{ return safe(impl).get(k); } 
+void LLSD::insert(const String& k, const LLSD& v) {	makeMap(impl).insert(k, v); }
 
-LLSD& LLSD::insert(const String& k, const LLSD& v)
+LLSD& LLSD::with(const String& k, const LLSD& v)
 										{ 
 											makeMap(impl).insert(k, v); 
-											return *dynamic_cast<LLSD*>(this);
+											return *this;
 										}
 void LLSD::erase(const String& k)		{ makeMap(impl).erase(k); }
 
@@ -788,8 +775,9 @@ int LLSD::size() const					{ return safe(impl).size(); }
  
 LLSD LLSD::get(Integer i) const			{ return safe(impl).get(i); } 
 void LLSD::set(Integer i, const LLSD& v){ makeArray(impl).set(i, v); }
+void LLSD::insert(Integer i, const LLSD& v) { makeArray(impl).insert(i, v); }
 
-LLSD& LLSD::insert(Integer i, const LLSD& v)
+LLSD& LLSD::with(Integer i, const LLSD& v)
 										{ 
 											makeArray(impl).insert(i, v); 
 											return *this;
diff --git a/indra/llcommon/llsd.h b/indra/llcommon/llsd.h
index 552bb57498..135133c19c 100644
--- a/indra/llcommon/llsd.h
+++ b/indra/llcommon/llsd.h
@@ -223,8 +223,9 @@ public:
 		
 		bool has(const String&) const;
 		LLSD get(const String&) const;
-		LLSD& insert(const String&, const LLSD&);
+		void insert(const String&, const LLSD&);
 		void erase(const String&);
+		LLSD& with(const String&, const LLSD&);
 		
 		LLSD& operator[](const String&);
 		LLSD& operator[](const char* c)			{ return (*this)[String(c)]; }
@@ -238,9 +239,10 @@ public:
 		
 		LLSD get(Integer) const;
 		void set(Integer, const LLSD&);
-		LLSD& insert(Integer, const LLSD&);
+		void insert(Integer, const LLSD&);
 		void append(const LLSD&);
 		void erase(Integer);
+		LLSD& with(Integer, const LLSD&);
 		
 		const LLSD& operator[](Integer) const;
 		LLSD& operator[](Integer);