Increased safety by reference counting in ReadableString.

Source/DFPSR/api/configAPI.cpp

@@ -26,11 +26,11 @@
 using namespace dsr;
 
 void dsr::config_parse_ini(const ReadableString& content, ConfigIniCallback receiverLambda) {
-	List<ReadableString> lines = string_dangerous_split(content, U'\n');
+	List<String> lines = string_split(content, U'\n');
 	String block = U"";
 	for (int l = 0; l < lines.length(); l++) {
 		// Get the current line
-		ReadableString command = lines[l];
+		String command = lines[l];
 		// Skip comments
 		int commentIndex = string_findFirst(command, U';');
 		if (commentIndex > -1) {
@@ -39,8 +39,8 @@ void dsr::config_parse_ini(const ReadableString& content, ConfigIniCallback rece
 		// Find assignments
 		int assignmentIndex = string_findFirst(command, U'=');
 		if (assignmentIndex > -1) {
-			ReadableString key = string_removeOuterWhiteSpace(string_before(command, assignmentIndex));
-			ReadableString value = string_removeOuterWhiteSpace(string_after(command, assignmentIndex));
+			String key = string_removeOuterWhiteSpace(string_before(command, assignmentIndex));
+			String value = string_removeOuterWhiteSpace(string_after(command, assignmentIndex));
 			receiverLambda(block, key, value);
 		} else {
 			int blockStartIndex = string_findFirst(command, U'[');

Source/DFPSR/base/text.cpp

@@ -141,6 +141,7 @@ String dsr::string_lowerCase(const ReadableString &text) {
 	return result;
 }
 
+// Allow passing literals without allocating heap memory for the result
 ReadableString dsr::string_removeOuterWhiteSpace(const ReadableString &text) {
 	int64_t first = -1;
 	int64_t last = -1;
@@ -636,17 +637,25 @@ DsrChar ReadableString::operator[] (int64_t index) const {
 }
 
 ReadableString::ReadableString() {}
+
 ReadableString::~ReadableString() {}
 
 ReadableString::ReadableString(const DsrChar *content)
 : readSection(content), length(strlen_utf32(content)) {}
 
+ReadableString::ReadableString(const String& source) {
+	this->readSection = source.readSection;
+	this->length = source.length;
+	this->buffer = source.buffer;
+}
+
 // Not the fastest constructor, but won't bloat the public header
 // Hopefully most compilers know how to optimize this
-static ReadableString createSubString(const DsrChar *content, int64_t length) {
+static ReadableString createSubString(const DsrChar *content, int64_t length, const Buffer &buffer) {
 	ReadableString result;
 	result.readSection = content;
 	result.length = length;
+	result.buffer = buffer;
 	return result;
 }
 
@@ -654,11 +663,32 @@ String::String() {}
 String::String(const char* source) { this->append(source); }
 String::String(const char32_t* source) { this->append(source); }
 String::String(const std::string& source) { this->append(source); }
-String::String(const ReadableString& source) { this->append(source); }
-String::String(const String& source) { this->append(source); }
+String::String(const String& source) {
+	// Share immutable buffer
+	this->readSection = source.readSection;
+	this->length = source.length;
+	this->buffer = source.buffer;
+	this->writeSection = source.writeSection;
+}
+String::String(const ReadableString& source) {
+	const String* sharedSource = dynamic_cast<const String*>(&source);
+	if (sharedSource != nullptr) {
+		// Share immutable buffer when assigning String referred to as ReadableString
+		// + Passing String as a ReadableString reference is okay
+		// - Assigning String to a ReadableString loses access to the buffer
+		//   by having to construct a read-only string pointing to the data
+		this->readSection = sharedSource->readSection;
+		this->length = sharedSource->length;
+		this->buffer = sharedSource->buffer;
+		this->writeSection = sharedSource->writeSection;
+	} else {
+		// No buffer to share, just appending the content
+		this->append(source);
+	}
+}
 
 String::String(Buffer buffer, DsrChar *content, int64_t length)
- : ReadableString(createSubString(content, length)), buffer(buffer), writeSection(content) {}
+ : ReadableString(createSubString(content, length, buffer)), writeSection(content) {}
 
 int64_t String::capacity() {
 	if (this->buffer.get() == nullptr) {
@@ -741,19 +771,6 @@ void String::reserve(int64_t minimumLength) {
 	this->expand(minimumLength, false);
 }
 
-void String::write(int64_t index, DsrChar value) {
-	this->cloneIfShared();
-	if (index < 0 || index >= this->length) {
-		// TODO: Give a warning
-	} else {
-		this->writeSection[index] = value;
-	}
-}
-
-void String::clear() {
-	this->length = 0;
-}
-
 // This macro has to be used because a static template wouldn't be able to inherit access to private methods from the target class.
 //   Better to use a macro without type safety in the implementation than to expose yet another template in a global header.
 // Proof that appending to one string doesn't affect another:
@@ -771,7 +788,7 @@ void String::clear() {
 	int64_t oldLength = (TARGET)->length; \
 	(TARGET)->expand(oldLength + (int64_t)(LENGTH), true); \
 	for (int64_t i = 0; i < (int64_t)(LENGTH); i++) { \
-		(TARGET)->write(oldLength + i, ((SOURCE)[i]) & MASK); \
+		(TARGET)->writeSection[oldLength + i] = ((SOURCE)[i]) & MASK; \
 	} \
 }
 // TODO: See if ascii litterals can be checked for values above 127 in compile-time
@@ -879,8 +896,7 @@ void dsr::string_split_callback(std::function<void(ReadableString)> action, cons
 	}
 }
 
-// TODO: Try to implement this using reference counting so that it doesn't have to allocate heap memory
-List<String> dsr::string_split_clone(const ReadableString& source, DsrChar separator, bool removeWhiteSpace) {
+List<String> dsr::string_split(const ReadableString& source, DsrChar separator, bool removeWhiteSpace) {
 	List<String> result;
 	string_split_callback([&result, removeWhiteSpace](ReadableString element) {
 		if (removeWhiteSpace) {
@@ -998,7 +1014,7 @@ ReadableString dsr::string_exclusiveRange(const ReadableString& source, int64_t
 	if (inclusiveStart < 0) { inclusiveStart = 0; }
 	if (exclusiveEnd > source.length) { exclusiveEnd = source.length; }
 	// Return the overlapping interval
-	return createSubString(&(source.readSection[inclusiveStart]), exclusiveEnd - inclusiveStart);
+	return createSubString(&(source.readSection[inclusiveStart]), exclusiveEnd - inclusiveStart, source.buffer);
 }
 
 ReadableString dsr::string_inclusiveRange(const ReadableString& source, int64_t inclusiveStart, int64_t inclusiveEnd) {
@@ -1093,6 +1109,6 @@ bool dsr::string_isDouble(const ReadableString& source, bool allowWhiteSpace) {
 	}
 }
 
-int64_t dsr::string_getBufferUseCount(const String& text) {
+int64_t dsr::string_getBufferUseCount(const ReadableString& text) {
 	return text.buffer.use_count();
 }

Source/DFPSR/base/text.h

@@ -59,12 +59,18 @@ enum class LineEncoding {
 	Lf // Linux and Macintosh compatible (Might not work on non-portable text editors on Microsoft Windows)
 };
 
+class String;
+
 // Replacing String with a ReadableString reference for input arguments can make passing of U"" literals faster.
-//   Unlike String, it cannot be constructed from a "" literal, because UTF-32 is used internally.
+//   Unlike String, it cannot be constructed from a "" literal,
+//   because it's not allowed to create a new allocation for the UTF-32 conversion.
 // Only use by reference for input arguments or to hold temporary results!
 //   A ReadableString created as a sub-string from String will stop working once the parent String is freed.
 class ReadableString {
 IMPL_ACCESS:
+	// A reference counted pointer to the buffer to allow passing strings around without having to clone the buffer each time
+	// ReadableString only uses it for reference counting but String use it for reallocating
+	Buffer buffer;
 	// A local pointer to the sub-allocation
 	const char32_t* readSection = nullptr;
 	// The length of the current string in characters
@@ -81,6 +87,8 @@ public:
 	//   Do not use ReadableString for heap allocated allocations that might be freed during the string's life!
 	//   String can handle dynamic memory and should be used in that case.
 	ReadableString(const DsrChar *content);
+	// Create from String while keeping check of reference counting
+	ReadableString(const String& source);
 	// Destructor
 	virtual ~ReadableString();
 public:
@@ -90,8 +98,6 @@ public:
 	virtual std::string toStdString() const;
 };
 
-class String;
-
 // Used as format tags around numbers passed to string_append or string_combine
 // New types can implement printing to String by making wrappers from this class
 class Printable {
@@ -115,9 +121,7 @@ public:
 //     No combined characters allowed, use precomposed instead, so that the strings can guarantee a fixed character size
 class String : public ReadableString {
 IMPL_ACCESS:
-	// A reference counted pointer to the buffer to allow passing strings around without having to clone the buffer each time
-	Buffer buffer;
-	// Same as readSection, but with write access
+	// Same as readSection, but with write access for appending more text
 	char32_t* writeSection = nullptr;
 	// Internal constructor
 	String(Buffer buffer, DsrChar *content, int64_t length);
@@ -149,10 +153,6 @@ public:
 	void append(const std::string& source);
 	// Extend the String using another character
 	void appendChar(DsrChar source);
-public:
-	// Access
-	void write(int64_t index, DsrChar value);
-	void clear();
 };
 
 // Define this overload for non-virtual source types that cannot inherit from Printable
@@ -246,7 +246,7 @@ ReadableString string_after(const ReadableString& source, int64_t exclusiveStart
 // The separating characters are excluded from the resulting strings.
 // The number of strings returned in the list will equal the number of separating characters plus one, so the result may contain empty strings.
 // Each string in the list clones content to its own dynamic buffer. Use string_split_callback if you don't need long term storage.
-List<String> string_split_clone(const ReadableString& source, DsrChar separator, bool removeWhiteSpace = false);
+List<String> string_split(const ReadableString& source, DsrChar separator, bool removeWhiteSpace = false);
 // The fastest and most powerful split implementation
 // Split a string without needing a list to store the result.
 //   Call it twice using different lambda functions if you want to count the size before allocating a buffer.
@@ -257,6 +257,10 @@ void string_split_callback(std::function<void(ReadableString)> action, const Rea
 inline void string_split_callback(const ReadableString& source, DsrChar separator, bool removeWhiteSpace, std::function<void(ReadableString)> action) {
 	string_split_callback(action, source, separator, removeWhiteSpace);
 }
+
+
+// TODO: ReadableString is no longer supposed to be unsafe, but String is also meant to be faster.
+
 // Warning! May cause a crash.
 //   Do not use a ReadableString generated by splitting a String past the String's lifetime.
 //   ReadableString does not allocate any heap memory but is only a view for data allocated elsewhere.
@@ -278,6 +282,8 @@ List<ReadableString> string_dangerous_split(const ReadableString& source, DsrCha
 //   If appendResult is false (default), any pre-existing elements in the target list will be cleared before writing the result.
 //   If appendResult is true, the result is appended to the existing target list.
 void string_dangerous_split_inPlace(List<ReadableString> &target, const ReadableString& source, DsrChar separator, bool appendResult = false);
+
+
 // Split source using separator, only to return the number of splits.
 // Useful for pre-allocation.
 int64_t string_splitCount(const ReadableString& source, DsrChar separator);
@@ -368,7 +374,7 @@ String string_mangleQuote(const ReadableString &rawText);
 String string_unmangleQuote(const ReadableString& mangledText);
 
 // Post-condition: Returns the number of strings using the same buffer, including itself.
-int64_t string_getBufferUseCount(const String& text);
+int64_t string_getBufferUseCount(const ReadableString& text);
 
 // Ensures safely that at least minimumLength characters can he held in the buffer
 inline void string_reserve(String& target, int64_t minimumLength) {

Source/DFPSR/machine/VirtualMachine.cpp

@@ -49,7 +49,7 @@ VirtualMachine::VirtualMachine(const ReadableString& code, const std::shared_ptr
 		if (colonIndex > -1) {
 			ReadableString command = string_removeOuterWhiteSpace(string_before(currentLine, colonIndex));
 			ReadableString argumentLine = string_after(currentLine, colonIndex);
-			List<String> arguments = string_split_clone(argumentLine, U',', true);
+			List<String> arguments = string_split(argumentLine, U',', true);
 			this->interpretMachineWord(command, arguments);
 		} else if (string_length(currentLine) > 0) {
 			throwError("Unexpected line \"", currentLine, "\".\n");

Source/test/tests/StringTest.cpp

@@ -5,7 +5,7 @@
 //       Cover everything using a single dsr::String type?
 //       Use "" operand as only constructor?
 void fooInPlace(dsr::String& target, const dsr::ReadableString& a, const dsr::ReadableString& b) {
-	target.clear();
+	string_clear(target);
 	target.append(U"Foo(");
 	target.append(a);
 	target.appendChar(U',');
@@ -209,13 +209,14 @@ START_TEST(String)
 	ASSERT_MATCH(dsr::string_unmangleQuote(dsr::string_mangleQuote(U"c\"d")), U"c\"d");
 	// Mangle things
 	dsr::String randomText;
+	string_reserve(randomText, 100);
 	for (int i = 1; i < 100; i++) {
 		// Randomize previous characters
 		for (int j = 1; j < i - 1; j++) {
-			randomText.write(j, (DsrChar)((i * 21 + j * 49 + 136) % 1024));
+			string_appendChar(randomText, (DsrChar)((i * 21 + j * 49 + 136) % 1024));
 		}
 		// Add a new random character
-		randomText.appendChar((i * 21 + 136) % 256);
+		string_appendChar(randomText, (i * 21 + 136) % 256);
 		ASSERT_MATCH(dsr::string_unmangleQuote(dsr::string_mangleQuote(randomText)), randomText);
 	}
 	// Number serialization
@@ -243,19 +244,34 @@ START_TEST(String)
 	ASSERT_EQUAL(string_toInteger(U"123"), 123);
 	ASSERT_EQUAL(string_toDouble(U"123"), 123.0);
 	ASSERT_EQUAL(string_toDouble(U"123.456"), 123.456);
-	{ // Clone splitting
+	{ // Assigning strings using reference counting
+		String a = U"Some text";
+		ASSERT_EQUAL(string_getBufferUseCount(a), 1);
+		String b = a;
+		ASSERT_EQUAL(string_getBufferUseCount(a), 2);
+		ASSERT_EQUAL(string_getBufferUseCount(b), 2);
+		String c = b;
+		ASSERT_EQUAL(string_getBufferUseCount(a), 3);
+		ASSERT_EQUAL(string_getBufferUseCount(b), 3);
+		ASSERT_EQUAL(string_getBufferUseCount(c), 3);
+	}
+	{ // String splitting by shared reference counted buffer
+		String source = U"a . b . c . d";
 		List<String> result;
-		result = string_split_clone(U"a . b . c . d", U'.', false);
+		result = string_split(source, U'.', false);
+		ASSERT_EQUAL(string_getBufferUseCount(source), 1);
 		ASSERT_EQUAL(result.length(), 4);
 		ASSERT_MATCH(result[0], U"a ");
 		ASSERT_MATCH(result[1], U" b ");
 		ASSERT_MATCH(result[2], U" c ");
 		ASSERT_MATCH(result[3], U" d");
-		result = string_split_clone(U"a . b .\tc", U'.', true);
+		//ASSERT_EQUAL(string_getBufferUseCount(source), 5);
+		result = string_split(U"a . b .\tc", U'.', true);
 		ASSERT_EQUAL(result.length(), 3);
 		ASSERT_MATCH(result[0], U"a");
 		ASSERT_MATCH(result[1], U"b");
 		ASSERT_MATCH(result[2], U"c");
+		//ASSERT_EQUAL(string_getBufferUseCount(source), 4);
 	}
 	{ // Callback splitting
 		String numbers = U"1, 3, 5, 7, 9";
@@ -272,5 +288,6 @@ START_TEST(String)
 	}
 	// TODO: Test taking a part of a parent string with a start offset, leaving the parent scope,
 	//       and expanding with append while the buffer isn't shared but has an offset from buffer start.
+	// TODO: Test sharing the same buffer between strings, then clear and append more text without overwriting other strings.
 	// TODO: Assert that buffers are shared when they should, but prevents side-effects when one is being written to.
 END_TEST