Fix/list operations and compiler safety

inkcpp/array.h

@@ -12,7 +12,7 @@
 #include "traits.h"
 
 #include <limits>
-#include <cstdint>
+#include <new>
 
 namespace ink::runtime::internal
 {
@@ -34,15 +34,16 @@ class managed_array : public snapshot_interface
 	    , _static_data{}
 	{
 		if constexpr (dynamic) {
-			if constexpr (simple) {
-				_dynamic_data = reinterpret_cast<T*>(new char[sizeof(T) * initialCapacity]);
-				inkAssert(
-				    reinterpret_cast<std::uintptr_t>(_dynamic_data) % alignof(T) == 0,
-				    "The data array has a different alignment(%d) then the contained data(%d)",
-				    reinterpret_cast<std::uintptr_t>(_dynamic_data), alignof(T)
-				);
-			} else {
-				_dynamic_data = new T[initialCapacity];
+			if constexpr (initialCapacity > 0) {
+				if constexpr (simple) {
+					_dynamic_data
+					    = reinterpret_cast<T*>(new (std::nothrow) char[sizeof(T) * initialCapacity]);
+					inkAssert(_dynamic_data != nullptr, "Out of memory in inkcpp: managed_array init failed");
+					inkAssert(( ::size_t ) _dynamic_data % alignof(T) == 0);
+				} else {
+					_dynamic_data = new (std::nothrow) T[initialCapacity];
+					inkAssert(_dynamic_data != nullptr, "Out of memory in inkcpp: managed_array init failed");
+				}
 			}
 		}
 	}
@@ -55,10 +56,12 @@ class managed_array : public snapshot_interface
 	virtual ~managed_array()
 	{
 		if constexpr (dynamic) {
-			if constexpr (simple) {
-				delete[] reinterpret_cast<char*>(_dynamic_data);
-			} else {
-				delete[] _dynamic_data;
+			if (_dynamic_data != nullptr) {
+				if constexpr (simple) {
+					delete[] reinterpret_cast<char*>(_dynamic_data);
+				} else {
+					delete[] _dynamic_data;
+				}
 			}
 		}
 	}
@@ -115,11 +118,11 @@ class managed_array : public snapshot_interface
 			if (_size == _capacity) {
 				extend();
 			}
-			inkAssert(_size < _capacity, "Failed to extend full dynamic array!");
 		} else {
-			inkAssert(_size < _capacity, "Try to append to a full array!");
+			inkAssert(_size <= _capacity, "Try to append to a full array!");
 			// TODO(JBenda): Silent fail?
 		}
+		inkAssert(_size < _capacity);
 		return data()[_size++];
 	}
 
@@ -282,31 +285,30 @@ void managed_array<T, dynamic, initialCapacity, simple>::extend(size_t capacity)
 	if constexpr (simple) {
 		// Warning: Allocating typed data in a char* container is potentially unsafe. We need to be sure
 		// the alignment is compatible with the destination type...
-		new_data = reinterpret_cast<T*>(new char[sizeof(T) * new_capacity]);
-		inkAssert(
-		    reinterpret_cast<std::uintptr_t>(new_data) % alignof(T) == 0,
-		    "New allocated array for extansion is aligned(%d) but the data type has an alignment of %d",
-		    reinterpret_cast<std::uintptr_t>(new_data), alignof(T)
-		);
+		new_data = reinterpret_cast<T*>(new (std::nothrow) char[sizeof(T) * new_capacity]);
+		inkAssert(new_data != nullptr, "Out of memory in inkcpp: managed_array extend failed (simple)");
+		inkAssert(( ::size_t ) new_data % alignof(T) == 0);
 
 		// ...and we have to copy the contents byte-by-byte, since client code (_list_handouts)
 		// type-puns between two classes with different vtbls here. Copying these elementwise would
 		// change the stored C++ type.
-		memcpy(static_cast<void*>(new_data), static_cast<const void*>(_dynamic_data), sizeof(T) * _size);
+		if (_dynamic_data) {
+			memcpy(new_data, _dynamic_data, sizeof(T) * _capacity);
+			delete[] reinterpret_cast<char*>(_dynamic_data);
+		}
 	} else {
 		// Allocate and copy typed data normally
-		new_data = new T[new_capacity];
+		new_data = new (std::nothrow) T[new_capacity];
+		inkAssert(new_data != nullptr, "Out of memory in inkcpp: managed_array extend failed (typed)");
 
-		for (size_t i = 0; i < _capacity; ++i) {
-			new_data[i] = _dynamic_data[i];
+		if (_dynamic_data) {
+			for (size_t i = 0; i < _capacity; ++i) {
+				new_data[i] = static_cast<T&&>(_dynamic_data[i]);
+			}
+			delete[] _dynamic_data;
 		}
 	}
 
-	if constexpr (simple) {
-		delete[] reinterpret_cast<char*>(_dynamic_data);
-	} else {
-		delete[] _dynamic_data;
-	}
 	_dynamic_data = new_data;
 	_capacity     = new_capacity;
 }

inkcpp/collections/restorable.h

@@ -159,7 +159,7 @@ class restorable : public snapshot_interface
 	// Iterator that begins at the end of the stack
 	iterator begin() { return iterator(&_buffer[_pos - 1], _buffer - 1); }
 
-	const_iterator begin() const { return iterator(&_buffer[_pos - 1], _buffer - 1); }
+	const_iterator begin() const { return const_iterator(&_buffer[_pos - 1], _buffer - 1); }
 
 	// Iterator that points to the element past the beginning of the stack
 	iterator end() { return iterator(_buffer - 1, _buffer - 1); }
@@ -198,9 +198,11 @@ class restorable : public snapshot_interface
 			_pos = _jump;
 
 		// Move over empty data
-		while (isNull(_buffer[_pos - 1]))
+		while (_pos > 0 && isNull(_buffer[_pos - 1]))
 			_pos--;
 
+		inkAssert(_pos > 0, "Can not pop. No non-null elements to pop!");
+
 		// Decrement and return
 		_pos--;
 		return _buffer[_pos];
@@ -211,10 +213,11 @@ class restorable : public snapshot_interface
 	{
 		inkAssert(_pos > 0, "Can not top. No elememnts to show!");
 		auto pos = _pos;
-		if (_pos == _save)
+		if (pos == _save)
 			pos = _jump;
-		while (isNull(_buffer[pos - 1]))
+		while (pos > 0 && isNull(_buffer[pos - 1]))
 			--pos;
+		inkAssert(pos > 0, "Can not top. No non-null elements to show!");
 		return _buffer[pos - 1];
 	}
 
@@ -228,7 +231,7 @@ class restorable : public snapshot_interface
 
 	// Forward iterate
 	template<typename CallbackMethod, typename IsNullPredicate>
-	void for_each(CallbackMethod callback, IsNullPredicate isNull)
+	void for_each(CallbackMethod callback, IsNullPredicate isNull) const
 	{
 		if (_pos == 0) {
 			return;

inkcpp/list_impl.cpp

@@ -40,11 +40,13 @@ void list_impl::next(const char*& flag_name, const char*& list_name, int& i, boo
 		return;
 	}
 
-	list_flag flag{static_cast<int16_t>(i >> 16), static_cast<int16_t>(i & 0xFF)};
+	list_flag flag{static_cast<int16_t>(i >> 16), static_cast<int16_t>(i & 0x7FFF)};
 	if (flag_name != nullptr) {
 		++flag.flag;
 	}
-	if (static_cast<size_t>(flag.flag) >= _list_table->_list_end[flag.list_id]) {
+	if (flag.list_id < 0 || static_cast<size_t>(flag.list_id) >= _list_table->_list_end.size()
+	    || static_cast<size_t>(flag.flag)
+	           >= (_list_table->_list_end[flag.list_id] - _list_table->listBegin(flag.list_id))) {
 next_list:
 		if (one_list_only) {
 			i = -1;
@@ -53,7 +55,7 @@ void list_impl::next(const char*& flag_name, const char*& list_name, int& i, boo
 		flag.flag = 0;
 		do {
 			++flag.list_id;
-			if (static_cast<size_t>(flag.list_id) >= _list_table->_list_end.size()) {
+			if (flag.list_id < 0 || static_cast<size_t>(flag.list_id) >= _list_table->_list_end.size()) {
 				i = -1;
 				return;
 			}
@@ -66,15 +68,24 @@ void list_impl::next(const char*& flag_name, const char*& list_name, int& i, boo
 			goto next_list;
 		}
 	}
-	flag_name = _list_table->_flag_names[_list_table->toFid(flag)];
-	list_name = _list_table->_list_names[flag.list_id];
+	int fid   = _list_table->toFid(flag);
+	flag_name = (fid >= 0 && static_cast<size_t>(fid) < _list_table->_flag_names.size())
+	              ? _list_table->_flag_names[fid]
+	              : nullptr;
+	list_name
+	    = (flag.list_id >= 0 && static_cast<size_t>(flag.list_id) < _list_table->_list_names.size())
+	        ? _list_table->_list_names[flag.list_id]
+	        : nullptr;
 
-	i = (flag.list_id << 16) | flag.flag;
+	i = (flag.list_id << 16) | (flag.flag & 0x7FFF);
 }
 
 list_interface::iterator list_impl::begin(const char* list_name) const
 {
-	size_t list_id = _list_table->get_list_id(list_name).list_id;
-	return ++new_iterator(nullptr, list_id << 16, true);
+	list_flag lf = _list_table->get_list_id(list_name);
+	if (lf.list_id < 0) {
+		return end();
+	}
+	return ++new_iterator(nullptr, lf.list_id << 16, true);
 }
 } // namespace ink::runtime::internal

inkcpp/list_operations.h

@@ -332,9 +332,30 @@ class operation<Command::LIST_VALUE, value_type::list_flag, void>
 	void operator()(basic_eval_stack& stack, value* vals)
 	{
 		inkAssert(vals[0].type() == value_type::list_flag, "LIST_VALUE only works on list_flag values");
-		stack.push(value{}.set<value_type::int32>(
-		    static_cast<int32_t>(vals[0].get<value_type::list_flag>().flag) + 1
-		));
+		list_flag flag = vals[0].get<value_type::list_flag>();
+		if (flag.list_id < 0 || flag.flag < 0) {
+			stack.push(value{}.set<value_type::int32>(0));
+		} else {
+			stack.push(value{}.set<value_type::int32>(_list_table.get_flag_value(flag)));
+		}
+	}
+};
+
+template<>
+class operation<Command::LIST_VALUE, value_type::list, void> : public operation_base<list_table>
+{
+public:
+	using operation_base::operation_base;
+
+	void operator()(basic_eval_stack& stack, value* vals)
+	{
+		list_table::list l        = vals[0].get<value_type::list>();
+		list_flag        max_flag = _list_table.max(l);
+		if (max_flag.list_id < 0 || max_flag.flag < 0) {
+			stack.push(value{}.set<value_type::int32>(0));
+		} else {
+			stack.push(value{}.set<value_type::int32>(_list_table.get_flag_value(max_flag)));
+		}
 	}
 };