diff --git a/cores/esp8266/StreamString.h b/cores/esp8266/StreamString.h index a868f28d17..1d0cc958d1 100644 --- a/cores/esp8266/StreamString.h +++ b/cores/esp8266/StreamString.h @@ -236,7 +236,6 @@ class StreamString: public String, public S2Stream StreamString(const String& string): String(string), S2Stream(this) { } StreamString(const __FlashStringHelper *str): String(str), S2Stream(this) { } StreamString(String&& string): String(string), S2Stream(this) { } - StreamString(StringSumHelper&& sum): String(sum), S2Stream(this) { } explicit StreamString(char c): String(c), S2Stream(this) { } explicit StreamString(unsigned char c, unsigned char base = 10): String(c, base), S2Stream(this) { } @@ -281,13 +280,6 @@ class StreamString: public String, public S2Stream resetpp(); return *this; } - - StreamString& operator= (StringSumHelper&& rval) - { - String::operator=(rval); - resetpp(); - return *this; - } }; #endif // __STREAMSTRING_H diff --git a/cores/esp8266/WString.cpp b/cores/esp8266/WString.cpp index 19a4143ecf..93b9c43a59 100644 --- a/cores/esp8266/WString.cpp +++ b/cores/esp8266/WString.cpp @@ -21,7 +21,7 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ -#include +#include "Arduino.h" #include "WString.h" #include "stdlib_noniso.h" @@ -56,11 +56,6 @@ String::String(String &&rval) noexcept { move(rval); } -String::String(StringSumHelper &&rval) noexcept { - init(); - move(rval); -} - String::String(unsigned char value, unsigned char base) { init(); char buf[1 + 8 * sizeof(unsigned char)]; @@ -390,98 +385,92 @@ unsigned char String::concat(const __FlashStringHelper *str) { } /*********************************************/ -/* Concatenate */ +/* Insert */ /*********************************************/ -StringSumHelper &operator +(const StringSumHelper &lhs, const String &rhs) { - StringSumHelper &a = const_cast(lhs); - if (!a.concat(rhs.buffer(), rhs.len())) - a.invalidate(); - return a; -} +String &String::insert(size_t position, const char *other, size_t other_length) { + if (position > length()) + return *this; -StringSumHelper &operator +(const StringSumHelper &lhs, const char *cstr) { - StringSumHelper &a = const_cast(lhs); - if (!cstr || !a.concat(cstr, strlen(cstr))) - a.invalidate(); - return a; -} + auto len = length(); + auto total = len + other_length; + if (!reserve(total)) + return *this; -StringSumHelper &operator +(const StringSumHelper &lhs, char c) { - StringSumHelper &a = const_cast(lhs); - if (!a.concat(c)) - a.invalidate(); - return a; -} + auto left = len - position; + setLen(total); -StringSumHelper &operator +(const StringSumHelper &lhs, unsigned char num) { - StringSumHelper &a = const_cast(lhs); - if (!a.concat(num)) - a.invalidate(); - return a; -} + auto *start = wbuffer() + position; + memmove(start + other_length, start, left); + memmove_P(start, other, other_length); + wbuffer()[total] = '\0'; -StringSumHelper &operator +(const StringSumHelper &lhs, int num) { - StringSumHelper &a = const_cast(lhs); - if (!a.concat(num)) - a.invalidate(); - return a; + return *this; } -StringSumHelper &operator +(const StringSumHelper &lhs, unsigned int num) { - StringSumHelper &a = const_cast(lhs); - if (!a.concat(num)) - a.invalidate(); - return a; +String &String::insert(size_t position, const __FlashStringHelper *other) { + auto *p = reinterpret_cast(other); + return insert(position, p, strlen_P(p)); } -StringSumHelper &operator +(const StringSumHelper &lhs, long num) { - StringSumHelper &a = const_cast(lhs); - if (!a.concat(num)) - a.invalidate(); - return a; +String &String::insert(size_t position, char other) { + char tmp[2] { other, '\0' }; + return insert(position, tmp, 1); } -StringSumHelper &operator +(const StringSumHelper &lhs, unsigned long num) { - StringSumHelper &a = const_cast(lhs); - if (!a.concat(num)) - a.invalidate(); - return a; +String &String::insert(size_t position, const char *other) { + return insert(position, other, strlen(other)); } -StringSumHelper &operator +(const StringSumHelper &lhs, long long num) { - StringSumHelper &a = const_cast(lhs); - if (!a.concat(num)) - a.invalidate(); - return a; +String &String::insert(size_t position, const String &other) { + return insert(position, other.c_str(), other.length()); } -StringSumHelper &operator +(const StringSumHelper &lhs, unsigned long long num) { - StringSumHelper &a = const_cast(lhs); - if (!a.concat(num)) - a.invalidate(); - return a; +String operator +(const String &lhs, String &&rhs) { + String res; + auto total = lhs.length() + rhs.length(); + if (rhs.capacity() > total) { + rhs.insert(0, lhs); + res = std::move(rhs); + } else { + res.reserve(total); + res += lhs; + res += rhs; + rhs.invalidate(); + } + + return res; } -StringSumHelper &operator +(const StringSumHelper &lhs, float num) { - StringSumHelper &a = const_cast(lhs); - if (!a.concat(num)) - a.invalidate(); - return a; +String operator +(String &&lhs, String &&rhs) { + String res; + auto total = lhs.length() + rhs.length(); + if ((total > lhs.capacity()) && (total < rhs.capacity())) { + rhs.insert(0, lhs); + res = std::move(rhs); + } else { + lhs += rhs; + rhs.invalidate(); + res = std::move(lhs); + } + + return res; } -StringSumHelper &operator +(const StringSumHelper &lhs, double num) { - StringSumHelper &a = const_cast(lhs); - if (!a.concat(num)) - a.invalidate(); - return a; +String operator +(char lhs, const String &rhs) { + String res; + res.reserve(rhs.length() + 1); + res += lhs; + res += rhs; + return res; } -StringSumHelper &operator +(const StringSumHelper &lhs, const __FlashStringHelper *rhs) { - StringSumHelper &a = const_cast(lhs); - if (!a.concat(rhs)) - a.invalidate(); - return a; +String operator +(const char *lhs, const String &rhs) { + String res; + res.reserve(strlen_P(lhs) + rhs.length()); + res += lhs; + res += rhs; + return res; } /*********************************************/ diff --git a/cores/esp8266/WString.h b/cores/esp8266/WString.h index beae31a9ab..458431e27b 100644 --- a/cores/esp8266/WString.h +++ b/cores/esp8266/WString.h @@ -23,14 +23,15 @@ #define String_class_h #ifdef __cplusplus -#include -#include -#include #include -// An inherited class for holding the result of a concatenation. These -// result objects are assumed to be writable by subsequent concatenations. -class StringSumHelper; +#include +#include +#include +#include + +#include +#include // an abstract class used as a means to proide a unique pointer type // but really has no body @@ -38,6 +39,10 @@ class __FlashStringHelper; #define FPSTR(pstr_pointer) (reinterpret_cast(pstr_pointer)) #define F(string_literal) (FPSTR(PSTR(string_literal))) +// support libraries that expect this name to be available +// replace with `using StringSumHelper = String;` in case something wants this constructible +class StringSumHelper; + // The string class class String { // use a function pointer to allow for "if (s)" without the @@ -60,7 +65,6 @@ class String { String(const String &str); String(const __FlashStringHelper *str); String(String &&rval) noexcept; - String(StringSumHelper &&rval) noexcept; explicit String(char c) { sso.buff[0] = c; sso.buff[1] = 0; @@ -104,8 +108,10 @@ class String { String &operator =(const char *cstr); String &operator =(const __FlashStringHelper *str); String &operator =(String &&rval) noexcept; - String &operator =(StringSumHelper &&rval) noexcept { - return operator =((String &&)rval); + String &operator =(char c) { + char buffer[2] { c, '\0' }; + *this = buffer; + return *this; } // concatenate (works w/ built-in types) @@ -130,72 +136,11 @@ class String { // if there's not enough memory for the concatenated value, the string // will be left unchanged (but this isn't signalled in any way) - String &operator +=(const String &rhs) { + template + String &operator +=(const T &rhs) { concat(rhs); return *this; } - String &operator +=(const char *cstr) { - concat(cstr); - return *this; - } - String &operator +=(char c) { - concat(c); - return *this; - } - String &operator +=(unsigned char num) { - concat(num); - return *this; - } - String &operator +=(int num) { - concat(num); - return *this; - } - String &operator +=(unsigned int num) { - concat(num); - return *this; - } - String &operator +=(long num) { - concat(num); - return *this; - } - String &operator +=(unsigned long num) { - concat(num); - return *this; - } - String &operator +=(long long num) { - concat(num); - return *this; - } - String &operator +=(unsigned long long num) { - concat(num); - return *this; - } - String &operator +=(float num) { - concat(num); - return *this; - } - String &operator +=(double num) { - concat(num); - return *this; - } - String &operator +=(const __FlashStringHelper *str) { - concat(str); - return *this; - } - - friend StringSumHelper &operator +(const StringSumHelper &lhs, const String &rhs); - friend StringSumHelper &operator +(const StringSumHelper &lhs, const char *cstr); - friend StringSumHelper &operator +(const StringSumHelper &lhs, char c); - friend StringSumHelper &operator +(const StringSumHelper &lhs, unsigned char num); - friend StringSumHelper &operator +(const StringSumHelper &lhs, int num); - friend StringSumHelper &operator +(const StringSumHelper &lhs, unsigned int num); - friend StringSumHelper &operator +(const StringSumHelper &lhs, long num); - friend StringSumHelper &operator +(const StringSumHelper &lhs, unsigned long num); - friend StringSumHelper &operator +(const StringSumHelper &lhs, long long num); - friend StringSumHelper &operator +(const StringSumHelper &lhs, unsigned long long num); - friend StringSumHelper &operator +(const StringSumHelper &lhs, float num); - friend StringSumHelper &operator +(const StringSumHelper &lhs, double num); - friend StringSumHelper &operator +(const StringSumHelper &lhs, const __FlashStringHelper *rhs); // comparison (only works w/ Strings and "strings") operator StringIfHelperType() const { @@ -338,6 +283,14 @@ class String { const char *buffer() const { return wbuffer(); } char *wbuffer() const { return isSSO() ? const_cast(sso.buff) : ptr.buff; } // Writable version of buffer + // concatenation is done via non-member functions + // make sure we still have access to internal methods, since we optimize based on capacity of both sides and want to manipulate internal buffers directly + friend String operator +(const String &lhs, String &&rhs); + friend String operator +(String &&lhs, String &&rhs); + friend String operator +(char lhs, String &&rhs); + friend String operator +(const char *lhs, String &&rhs); + friend String operator +(const __FlashStringHelper *lhs, String &&rhs); + protected: void init(void) __attribute__((always_inline)) { sso.buff[0] = 0; @@ -359,54 +312,81 @@ class String { void invalidate(void); unsigned char changeBuffer(unsigned int maxStrLen); - // copy and move + // copy or insert at a specific position String ©(const char *cstr, unsigned int length); String ©(const __FlashStringHelper *pstr, unsigned int length); + + String &insert(size_t position, char); + String &insert(size_t position, const char *); + String &insert(size_t position, const __FlashStringHelper *); + String &insert(size_t position, const char *, size_t length); + String &insert(size_t position, const String &); + + // rvalue helper void move(String &rhs) noexcept; }; -class StringSumHelper: public String { - public: - StringSumHelper(const String &s) : - String(s) { - } - StringSumHelper(const char *p) : - String(p) { - } - StringSumHelper(char c) : - String(c) { - } - StringSumHelper(unsigned char num) : - String(num) { - } - StringSumHelper(int num) : - String(num) { - } - StringSumHelper(unsigned int num) : - String(num) { - } - StringSumHelper(long num) : - String(num) { - } - StringSumHelper(unsigned long num) : - String(num) { - } - StringSumHelper(long long num) : - String(num) { - } - StringSumHelper(unsigned long long num) : - String(num) { - } - StringSumHelper(float num) : - String(num) { - } - StringSumHelper(double num) : - String(num) { - } - StringSumHelper(const __FlashStringHelper *s) : - String(s) { - } -}; +// concatenation (note that it's done using non-method operators to handle both possible type refs) + +inline String operator +(const String &lhs, const String &rhs) { + String res; + res.reserve(lhs.length() + rhs.length()); + res += lhs; + res += rhs; + return res; +} + +inline String operator +(String &&lhs, const String &rhs) { + lhs += rhs; + return std::move(lhs); +} + +String operator +(const String &lhs, String &&rhs); +String operator +(String &&lhs, String &&rhs); + +template >>> +inline String operator +(const String &lhs, const T &value) { + String res(lhs); + res += value; + return res; +} + +template >>> +inline String operator +(String &&lhs, const T &value) { + lhs += value; + return std::move(lhs); +} + +// `String(char)` is explicit, but we used to have StringSumHelper silently allowing the following: +// `String x; x = 'a' + String('b') + 'c';` +// For comparison, `std::string(char)` does not exist. However, we are allowed to chain `char` as both lhs and rhs + +String operator +(char lhs, const String &rhs); + +inline String operator +(char lhs, String &&rhs) { + return std::move(rhs.insert(0, lhs)); +} + +// both `char*` and `__FlashStringHelper*` are implicitly converted into `String()`, calling the `operator+(const String& ...);` +// however, here we: +// - do an automatic `reserve(total length)` for the resulting string +// - possibly do rhs.insert(0, ...), when &&rhs capacity could fit both + +String operator +(const char *lhs, const String &rhs); + +inline String operator +(const char *lhs, String &&rhs) { + return std::move(rhs.insert(0, lhs)); +} + +inline String operator +(const __FlashStringHelper *lhs, const String &rhs) { + return reinterpret_cast(lhs) + rhs; +} + +inline String operator +(const __FlashStringHelper *lhs, String &&rhs) { + return std::move(rhs.insert(0, lhs)); +} extern const String emptyString; diff --git a/tests/host/core/test_string.cpp b/tests/host/core/test_string.cpp index e080be9507..6193132706 100644 --- a/tests/host/core/test_string.cpp +++ b/tests/host/core/test_string.cpp @@ -555,3 +555,42 @@ TEST_CASE("Replace and string expansion", "[core][String]") REQUIRE(l.length() == strlen(buff)); } } + +TEST_CASE("String chaining", "[core][String]") +{ + const char* chunks[] { + "~12345", + "67890", + "qwertyuiopasdfghjkl", + "zxcvbnm" + }; + + String all; + for (auto* chunk : chunks) { + all += chunk; + } + + // make sure we can chain a combination of things to form a String + REQUIRE((String(chunks[0]) + String(chunks[1]) + String(chunks[2]) + String(chunks[3])) == all); + REQUIRE((chunks[0] + String(chunks[1]) + F(chunks[2]) + chunks[3]) == all); + REQUIRE((String(chunks[0]) + F(chunks[1]) + F(chunks[2]) + String(chunks[3])) == all); + REQUIRE(('~' + String(&chunks[0][0] + 1) + chunks[1] + String(chunks[2]) + F(chunks[3])) == all); + REQUIRE((String(chunks[0]) + '6' + (&chunks[1][0] + 1) + String(chunks[2]) + F(chunks[3])) == all); + + // these are still invalid (and also cannot compile at all): + // - `F(...)` + `F(...)` + // - `F(...)` + `const char*` + // - `const char*` + `F(...)` + // we need `String()` as either rhs or lhs + + // ensure chaining reuses the buffer + // (internal details...) + { + String tmp(chunks[3]); + tmp.reserve(2 * all.length()); + auto* ptr = tmp.c_str(); + String result("~1" + String(&chunks[0][0] + 2) + F(chunks[1]) + chunks[2] + std::move(tmp)); + REQUIRE(result == all); + REQUIRE(static_cast(result.c_str()) == static_cast(ptr)); + } +}