Optimise iteration and lookups (#3)

**Optimise iteration and `indexOf` operation**

The regular `[]` operator and `valueAt` methods include range checks which are costly inside a loop.
Adding `unsafe` methods skips these checks.

**Improve Vector search for char* argument**

The `indexOf` method has been specialised for `char*` arguments so that `strlen()` only needs to be called once. Previously it was called on every loop iteration.

**Provide base `==` operator which does binary comparison**

This allows easy searching by value (object content).

**Rationalise string comparison methods**

FlashStrings have three `equals` overloads for `char*`, `String` and `FlashString` arguments.
We also require three `equalsignorecase` equivalents and appropriate `operator==` implementations.

By adding an `ignoreCase` parameter to all three `equals` methods we can simplify code.

**Other improvements**

- Use const references instead of copy
- Move copy/invalidate operations to constructors
- Use constexpr where possible for method return values
- Move `printElement(Print, char)` specialization into source file
- Move `read` method out of header
- Add github workflow
- Put strings in flash

Authored-by: mikee47 <[email protected]>

Author: mikee47

.github/workflows/ci-dispatch.yml

@@ -0,0 +1,20 @@
+name: CI Dispatch
+
+on:
+  workflow_dispatch:
+    inputs:
+      sming_repo:
+        description: 'Full URL for Sming repository'
+        default: 'https://github.com/SmingHub/Sming'
+        type: string
+      sming_branch:
+        description: 'Sming branch to run against'
+        default: 'develop'
+        type: string
+
+jobs:
+  build:
+    uses: SmingHub/Sming/.github/workflows/library.yml@develop
+    with:
+      sming_repo: ${{ inputs.sming_repo }}
+      sming_branch: ${{ inputs.sming_branch }}

.github/workflows/ci-push.yml

@@ -0,0 +1,7 @@
+name: CI Push
+
+on: [push, pull_request]
+
+jobs:
+  build:
+    uses: SmingHub/Sming/.github/workflows/library.yml@develop

src/ArrayPrinter.cpp

@@ -0,0 +1,76 @@
+/****
+ * ArrayPrinter.cpp - Print support for arrays
+ *
+ * Copyright 2019 mikee47 <[email protected]>
+ *
+ * This file is part of the FlashString Library
+ *
+ * This library is free software: you can redistribute it and/or modify it under the terms of the
+ * GNU General Public License as published by the Free Software Foundation, version 3 or later.
+ *
+ * This library is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
+ * without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with this library.
+ * If not, see <https://www.gnu.org/licenses/>.
+ *
+ ****/
+
+#include "include/FlashString/ArrayPrinter.hpp"
+
+namespace FSTR
+{
+size_t printElement(Print& p, char c)
+{
+	auto escape = [](char c) -> char {
+		switch(c) {
+		case '\0':
+			return '0';
+		case '\'':
+			return '\'';
+		case '\"':
+			return '"';
+		case '\?':
+			return '?';
+		case '\\':
+			return '\\';
+		case '\a':
+			return 'a';
+		case '\b':
+			return 'b';
+		case '\f':
+			return 'f';
+		case '\n':
+			return 'n';
+		case '\r':
+			return 'r';
+		case '\t':
+			return 't';
+		case '\v':
+			return 'v';
+		default:
+			return '\0';
+		}
+	};
+
+	char buf[8];
+	char* o = buf;
+	*o++ = '\'';
+	char esc = escape(c);
+	if(esc) {
+		*o++ = '\\';
+		*o++ = esc;
+	} else if(isprint(c)) {
+		*o++ = c;
+	} else {
+		*o++ = '\\';
+		*o++ = 'x';
+		*o++ = hexchar(uint8_t(c) >> 4);
+		*o++ = hexchar(uint8_t(c) & 0x0f);
+	}
+	*o++ = '\'';
+	return p.write(buf, o - buf);
+}
+
+} // namespace FSTR

src/ObjectBase.cpp

@@ -25,6 +25,17 @@ namespace FSTR
 const ObjectBase ObjectBase::empty_{ObjectBase::lengthInvalid};
 constexpr uint32_t ObjectBase::copyBit;
 
+bool ObjectBase::operator==(const ObjectBase& other) const
+{
+	if(this == &other) {
+		return true;
+	}
+	if(length() != other.length()) {
+		return false;
+	}
+	return memcmp(this, &other, sizeof(flashLength_) + size()) == 0;
+}
+
 size_t ObjectBase::readFlash(size_t offset, void* buffer, size_t count) const
 {
 	auto len = length();
@@ -37,15 +48,16 @@ size_t ObjectBase::readFlash(size_t offset, void* buffer, size_t count) const
 	return flashmem_read(buffer, addr, count);
 }
 
-size_t ObjectBase::length() const
+size_t ObjectBase::read(size_t offset, void* buffer, size_t count) const
 {
-	if(isNull()) {
+	auto len = length();
+	if(offset >= len) {
 		return 0;
-	} else if(isCopy()) {
-		return reinterpret_cast<const ObjectBase*>(flashLength_ & ~copyBit)->length();
-	} else {
-		return flashLength_;
 	}
+
+	count = std::min(len - offset, count);
+	memcpy_P(buffer, data() + offset, count);
+	return count;
 }
 
 const uint8_t* ObjectBase::data() const
@@ -71,16 +83,4 @@ const uint8_t* ObjectBase::data() const
 	return reinterpret_cast<const uint8_t*>(&ptr->flashLength_ + 1);
 }
 
-void ObjectBase::invalidate()
-{
-#ifndef ARCH_HOST
-	// Illegal on real flash object
-	assert(!isFlashPtr(this));
-	if(isFlashPtr(this)) {
-		return;
-	}
-#endif
-	flashLength_ = lengthInvalid;
-}
-
 } // namespace FSTR

src/String.cpp

@@ -23,32 +23,44 @@
 
 namespace FSTR
 {
-bool String::equals(const char* cstr, size_t len) const
+bool String::equals(const char* cstr, size_t clen, bool ignoreCase) const
 {
 	// Unlikely we'd want an empty flash string, but check anyway
 	if(cstr == nullptr) {
 		return length() == 0;
 	}
-	// Don't use strcmp as our data may contain nuls
-	if(len == 0) {
-		len = strlen(cstr);
-	}
-	if(len != length()) {
+	auto len = length();
+	if(clen != len) {
 		return false;
 	}
 	LOAD_FSTR(buf, *this);
+	if(ignoreCase) {
+		return memicmp(buf, cstr, len) == 0;
+	}
 	return memcmp(buf, cstr, len) == 0;
 }
 
-bool String::equals(const String& str) const
+bool String::equals(const char* cstr, bool ignoreCase) const
+{
+	return equals(cstr, cstr ? strlen(cstr) : 0, ignoreCase);
+}
+
+bool String::equals(const String& str, bool ignoreCase) const
 {
-	if(data() == str.data()) {
+	auto dataptr = data();
+	auto strdata = str.data();
+	if(dataptr == strdata) {
 		return true;
 	}
-	if(length() != str.length()) {
+	auto len = length();
+	if(len != str.length()) {
 		return false;
 	}
-	return memcmp_aligned(data(), str.data(), length()) == 0;
+	if(ignoreCase) {
+		LOAD_FSTR(buf, *this);
+		return memicmp(dataptr, buf, len) == 0;
+	}
+	return memcmp_aligned(dataptr, strdata, len) == 0;
 }
 
 /* Wiring String support */
@@ -58,25 +70,18 @@ String::operator WString() const
 	return isNull() ? WString() : WString(data(), length());
 }
 
-bool String::equals(const WString& str) const
+bool String::equals(const WString& str, bool ignoreCase) const
 {
 	auto len = str.length();
 	if(len != length()) {
 		return false;
 	}
 	// @todo optimise memcmp_P then we won't need to load entire String into RAM first
 	LOAD_FSTR(buf, *this);
-	return memcmp(buf, str.c_str(), len) == 0;
-}
-
-bool String::equalsIgnoreCase(const WString& str) const
-{
-	auto len = str.length();
-	if(len != length()) {
-		return false;
+	if(ignoreCase) {
+		return memicmp(buf, str.c_str(), len) == 0;
 	}
-	LOAD_FSTR(buf, *this);
-	return memicmp(buf, str.c_str(), len) == 0;
+	return memcmp(buf, str.c_str(), len) == 0;
 }
 
 } // namespace FSTR

src/include/FlashString/Array.hpp

@@ -113,6 +113,8 @@ namespace FSTR
 template <typename ElementType> class Array : public Object<Array<ElementType>, ElementType>
 {
 public:
+	static_assert(!std::is_pointer<ElementType>::value, "Pointer types not supported by Array - use Vector");
+
 	/* Arduino Print support */
 
 	/**

src/include/FlashString/ArrayPrinter.hpp

@@ -1,5 +1,5 @@
 /****
- * ArrayPrinter.cpp - Print support for arrays
+ * ArrayPrinter.hpp - Print support for arrays
  *
  * Copyright 2019 mikee47 <[email protected]>
  *
@@ -24,62 +24,13 @@
 
 namespace FSTR
 {
-template <typename T> typename std::enable_if<!std::is_same<T, char>::value, size_t>::type printElement(Print& p, T e)
+template <typename T>
+typename std::enable_if<!std::is_same<T, char>::value, size_t>::type printElement(Print& p, const T& e)
 {
 	return print(p, e);
 }
 
-template <typename T> typename std::enable_if<std::is_same<T, char>::value, size_t>::type printElement(Print& p, T c)
-{
-	auto escape = [](char c) -> char {
-		switch(c) {
-		case '\0':
-			return '0';
-		case '\'':
-			return '\'';
-		case '\"':
-			return '"';
-		case '\?':
-			return '?';
-		case '\\':
-			return '\\';
-		case '\a':
-			return 'a';
-		case '\b':
-			return 'b';
-		case '\f':
-			return 'f';
-		case '\n':
-			return 'n';
-		case '\r':
-			return 'r';
-		case '\t':
-			return 't';
-		case '\v':
-			return 'v';
-		default:
-			return '\0';
-		}
-	};
-
-	char buf[8];
-	char* o = buf;
-	*o++ = '\'';
-	char esc = escape(c);
-	if(esc) {
-		*o++ = '\\';
-		*o++ = esc;
-	} else if(isprint(c)) {
-		*o++ = c;
-	} else {
-		*o++ = '\\';
-		*o++ = 'x';
-		*o++ = hexchar(uint8_t(c) >> 4);
-		*o++ = hexchar(uint8_t(c) & 0x0f);
-	}
-	*o++ = '\'';
-	return p.write(buf, o - buf);
-}
+size_t printElement(Print& p, char c);
 
 /**
  * @brief Class template to provide a simple way to print the contents of an array

src/include/FlashString/MapPair.hpp

@@ -105,10 +105,10 @@ template <typename KeyType, class ContentType> class MapPair
 
 		if(*this) {
 			count += print(p, key());
-			count += p.print(" => ");
+			count += p.print(_F(" => "));
 			count += print(p, content());
 		} else {
-			count += p.print("(invalid)");
+			count += p.print(_F("(invalid)"));
 		}
 
 		return count;