Skip to content

Commit

Permalink
Get rid of copies
Browse files Browse the repository at this point in the history 
This is the main source of issues with clang compatibility, hence portability.
Makes code much simpler and removes need for casting.
  • Loading branch information
mikee47 committed Jun 9, 2024
1 parent c7f2b60 commit bf8b3dc
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 340 deletions.
21 changes: 1 addition & 20 deletions src/ObjectBase.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
namespace FSTR
{
const ObjectBase ObjectBase::empty_{ObjectBase::lengthInvalid};
constexpr uint32_t ObjectBase::copyBit;

bool ObjectBase::operator==(const ObjectBase& other) const
{
Expand Down Expand Up @@ -62,25 +61,7 @@ size_t ObjectBase::read(size_t offset, void* buffer, size_t count) const

const uint8_t* ObjectBase::data() const
{
if(isNull()) {
// Return a pointer to a valid memory location
return reinterpret_cast<const uint8_t*>(&flashLength_);
}

auto ptr = this;

if(isCopy()) {
// Get real object
ptr = reinterpret_cast<const ObjectBase*>(flashLength_ & ~copyBit);
}

// Cannot yet differentiate memory addresses on Host
#ifndef ARCH_HOST
// Check we've got a real flash pointer
assert(isFlashPtr(ptr));
#endif

return reinterpret_cast<const uint8_t*>(&ptr->flashLength_ + 1);
return reinterpret_cast<const uint8_t*>(&flashLength_ + 1);
}

} // namespace FSTR
8 changes: 4 additions & 4 deletions src/include/FlashString/Array.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
*/
#define DEFINE_FSTR_ARRAY_LOCAL(name, ElementType, ...) \
static DEFINE_FSTR_ARRAY_DATA(FSTR_DATA_NAME(name), ElementType, __VA_ARGS__); \
static FSTR_CONSTEXPR DEFINE_FSTR_REF_NAMED(name, FSTR::Array<ElementType>);
static constexpr const FSTR::Array<ElementType>& name = FSTR_DATA_NAME(name).object;

/**
* @brief Define an Array data structure
Expand All @@ -73,8 +73,8 @@
* @param ... List of ElementType items
*/
#define DEFINE_FSTR_ARRAY_DATA_SIZED(name, ElementType, size, ...) \
FSTR_CONSTEXPR const struct { \
FSTR::ObjectBase object; \
constexpr const struct { \
FSTR::Array<ElementType> object; \
ElementType data[size]; \
} FSTR_PACKED FSTR_ALIGNED name PROGMEM = {{sizeof(ElementType) * size}, {__VA_ARGS__}}; \
FSTR_CHECK_STRUCT(name);
Expand Down Expand Up @@ -140,7 +140,7 @@ template <typename ElementType> class Array : public Object<Array<ElementType>,
{
return printer().printTo(p);
}
};
} FSTR_PACKED;

} // namespace FSTR

Expand Down
10 changes: 5 additions & 5 deletions src/include/FlashString/Map.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
*/
#define DEFINE_FSTR_MAP_LOCAL(name, KeyType, ContentType, ...) \
static DEFINE_FSTR_MAP_DATA(FSTR_DATA_NAME(name), KeyType, ContentType, __VA_ARGS__); \
static FSTR_CONSTEXPR DEFINE_FSTR_REF_NAMED(name, DECL((FSTR::Map<KeyType, ContentType>)));
static constexpr const FSTR::Map<KeyType, ContentType>& name = FSTR_DATA_NAME(name).object;

/**
* @brief Define a Map Object with global reference, specifying the number of elements
Expand All @@ -77,7 +77,7 @@
*/
#define DEFINE_FSTR_MAP_SIZED_LOCAL(name, KeyType, ContentType, size, ...) \
static DEFINE_FSTR_MAP_DATA_SIZED(FSTR_DATA_NAME(name), KeyType, ContentType, size, __VA_ARGS__); \
static FSTR_CONSTEXPR DEFINE_FSTR_REF_NAMED(name, DECL((FSTR::Map<KeyType, ContentType>)));
static constexpr DEFINE_FSTR_REF_NAMED(name, DECL((FSTR::Map<KeyType, ContentType>)));

/**
* @brief Define a Map data structure
Expand All @@ -100,8 +100,8 @@
* @param ... List of MapPair definitions { key, &content }
*/
#define DEFINE_FSTR_MAP_DATA_SIZED(name, KeyType, ContentType, size, ...) \
FSTR_CONSTEXPR const struct { \
FSTR::ObjectBase object; \
constexpr const struct { \
FSTR::Map<KeyType, ContentType> object; \
FSTR::MapPair<KeyType, ContentType> data[size]; \
} FSTR_PACKED FSTR_ALIGNED name PROGMEM = {{sizeof(FSTR::MapPair<KeyType, ContentType>) * size}, {__VA_ARGS__}}; \
FSTR_CHECK_STRUCT(name);
Expand Down Expand Up @@ -202,7 +202,7 @@ class Map : public Object<Map<KeyType, ContentType>, Pair>
{
return printer().printTo(p);
}
};
} FSTR_PACKED;

} // namespace FSTR

Expand Down
44 changes: 8 additions & 36 deletions src/include/FlashString/Object.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,18 +79,14 @@
* We can only do this of course if the data structure is in scope.
*
*/
#define FSTR_PTR(objref) static_cast<std::remove_reference<decltype(objref)>::type*>(&FSTR_DATA_NAME(objref).object)
#define FSTR_PTR(objref) (&FSTR_DATA_NAME(objref).object)

/**
* @brief Check structure is POD-compliant and correctly aligned
*/
#ifdef __clang__
#define FSTR_CHECK_STRUCT(name)
#else
#define FSTR_CHECK_STRUCT(name) \
static_assert(std::is_pod<decltype(name)>::value, "FSTR structure not POD"); \
static_assert(std::is_standard_layout<decltype(name)>::value, "FSTR structure not Standard Layout"); \
static_assert(offsetof(decltype(name), data) == sizeof(uint32_t), "FSTR structure alignment error");
#endif

/**
* @brief Import an object from an external file with reference
Expand All @@ -110,8 +106,8 @@
*/
#define IMPORT_FSTR_OBJECT_LOCAL(name, ObjectType, file) \
IMPORT_FSTR_DATA(FSTR_DATA_NAME(name), file) \
extern "C" __attribute__((visibility("hidden"))) const FSTR::ObjectBase FSTR_DATA_NAME(name); \
static FSTR_CONSTEXPR DEFINE_FSTR_REF(name, ObjectType, FSTR_DATA_NAME(name));
extern "C" __attribute__((visibility("hidden"))) const ObjectType FSTR_DATA_NAME(name); \
static constexpr const ObjectType& name = FSTR_DATA_NAME(name);

namespace FSTR
{
Expand All @@ -127,34 +123,10 @@ template <class ObjectType, typename ElementType> class Object : public ObjectBa
using DataPtrType = const ElementType*;
using Iterator = ObjectIterator<ObjectType, ElementType>;

/**
* @brief Creates a null object
*/
Object() : ObjectBase{lengthInvalid}
{
#ifndef ARCH_HOST
// Illegal on real flash object
assert(!isFlashPtr(this));
#endif
}

/**
* @brief Copy constructor
* @note Objects are usually passed around by reference or as a pointer,
* but for ease of use we need a working copy constructor.
* A copy contains a pointer to the real object.
*/
Object(const Object& obj) : ObjectBase{obj.isCopy() ? obj.flashLength_ : uint32_t(&obj) | copyBit}
{
}

~Object()
{
}

Object(const Object&& obj) = delete;
Object& operator=(const Object& other) = delete;
Object& operator=(const Object&& other) = delete;
Object(const Object&) = delete;
Object(const Object&&) = delete;
Object& operator=(const Object&) = delete;
Object& operator=(const Object&&) = delete;

Iterator begin() const
{
Expand Down
17 changes: 2 additions & 15 deletions src/include/FlashString/ObjectBase.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,7 @@ class ObjectBase
*/
FSTR_NOINLINE constexpr const size_t length() const
{
if(isNull()) {
return 0;
}
if(isCopy()) {
// NOLINTNEXTLINE
return reinterpret_cast<const ObjectBase*>(flashLength_ & ~copyBit)->length();
}
return flashLength_;
return flashLength_ & ~lengthInvalid;
}

/**
Expand Down Expand Up @@ -99,11 +92,6 @@ class ObjectBase
*/
size_t readFlash(size_t offset, void* buffer, size_t count) const;

FSTR_INLINE constexpr const bool isCopy() const
{
return (flashLength_ & copyBit) != 0;
}

/**
* @brief Indicates an invalid String, used for return value from lookups, etc.
* @note A real String can be zero-length, but it cannot be null
Expand All @@ -120,8 +108,7 @@ class ObjectBase

protected:
static const ObjectBase empty_;
static constexpr uint32_t copyBit = 0x80000000U; ///< Set to indicate copy
static constexpr uint32_t lengthInvalid = copyBit | 0; ///< Indicates null string in a copy
static constexpr uint32_t lengthInvalid = 0x80000000U; ///< Indicates null string
};

} // namespace FSTR
8 changes: 4 additions & 4 deletions src/include/FlashString/String.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ typedef const __FlashStringHelper* flash_string_t;
*/
#define FS_PTR(str) \
(__extension__({ \
static DEFINE_FSTR_DATA(__fstr__, str); \
static_cast<const FSTR::String*>(&__fstr__.object); \
static FSTR_VOLATILE DEFINE_FSTR_DATA(FSTR_DATA_NAME(tmp), str); \
const_cast<const FSTR::String*>(&FSTR_DATA_NAME(tmp).object); \
}))

/**
Expand Down Expand Up @@ -83,7 +83,7 @@ typedef const __FlashStringHelper* flash_string_t;
*/
#define DEFINE_FSTR_LOCAL(name, str) \
static DEFINE_FSTR_DATA(FSTR_DATA_NAME(name), str); \
static FSTR_CONSTEXPR DEFINE_FSTR_REF_NAMED(name, FSTR::String);
static constexpr const FSTR::String& name = FSTR_DATA_NAME(name).object;

/**
* @brief Define a FSTR::String data structure
Expand All @@ -92,7 +92,7 @@ typedef const __FlashStringHelper* flash_string_t;
*/
#define DEFINE_FSTR_DATA(name, str) \
constexpr const struct { \
FSTR::ObjectBase object; \
FSTR::String object; \
char data[ALIGNUP4(sizeof(str))]; \
} name PROGMEM = {{sizeof(str) - 1}, str}; \
FSTR_CHECK_STRUCT(name);
Expand Down
14 changes: 7 additions & 7 deletions src/include/FlashString/Vector.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
*/
#define DEFINE_FSTR_VECTOR_LOCAL(name, ObjectType, ...) \
static DEFINE_FSTR_VECTOR_DATA(FSTR_DATA_NAME(name), ObjectType, __VA_ARGS__); \
static FSTR_CONSTEXPR DEFINE_FSTR_REF_NAMED(name, FSTR::Vector<ObjectType>);
static constexpr const FSTR::Vector<ObjectType>& name = FSTR_DATA_NAME(name).object;

/**
* @brief Define a Vector Object with global reference, specifying the number of elements
Expand All @@ -73,7 +73,7 @@
*/
#define DEFINE_FSTR_VECTOR_SIZED_LOCAL(name, ObjectType, size, ...) \
static DEFINE_FSTR_VECTOR_DATA_SIZED(FSTR_DATA_NAME(name), ObjectType, size, __VA_ARGS__); \
static FSTR_CONSTEXPR DEFINE_FSTR_REF_NAMED(name, FSTR::Vector<ObjectType>);
static constexpr DEFINE_FSTR_REF_NAMED(name, FSTR::Vector<ObjectType>);

/**
* @brief Define a Vector data structure
Expand All @@ -83,7 +83,7 @@
* @note Size will be calculated
*/
#define DEFINE_FSTR_VECTOR_DATA(name, ObjectType, ...) \
DEFINE_FSTR_VECTOR_DATA_SIZED(name, ObjectType, FSTR_VA_NARGS(ObjectType*, __VA_ARGS__), __VA_ARGS__)
DEFINE_FSTR_VECTOR_DATA_SIZED(name, ObjectType, FSTR_VA_NARGS(void*, __VA_ARGS__), __VA_ARGS__)

/**
* @brief Define a Vector data structure and specify the number of elements
Expand All @@ -94,10 +94,10 @@
* @note Use in situations where the array size cannot be automatically calculated
*/
#define DEFINE_FSTR_VECTOR_DATA_SIZED(name, ObjectType, size, ...) \
FSTR_CONSTEXPR const struct { \
FSTR::ObjectBase object; \
constexpr const struct { \
FSTR::Vector<ObjectType> object; \
const ObjectType* data[size]; \
} name PROGMEM = {{sizeof(ObjectType*) * size}, {__VA_ARGS__}}; \
} name PROGMEM = {{sizeof(void*) * size}, {__VA_ARGS__}}; \
FSTR_CHECK_STRUCT(name);

namespace FSTR
Expand Down Expand Up @@ -175,7 +175,7 @@ template <class ObjectType> class Vector : public Object<Vector<ObjectType>, con
auto ptr = dataptr[index];
return ptr ? *ptr : ObjectType::empty();
}
};
} FSTR_PACKED;

} // namespace FSTR

Expand Down
Loading

0 comments on commit bf8b3dc

Please sign in to comment.