Fun with maxon::BaseArray

C4DS

Here's a little something I stumbled upon by accident.
It might be user-error and not actually a bug, but it sure is weird behaviour in my book.

The following code defines an array, and fills it with 16 values. The values are not really important. Important is that there are 16, which is the default size a BaseArray gets allocated (if I remember correctly).
The array is printed to the console.
Then a simple "shifting" of the array is performed. The amount of shifting isn't important.
And finally the shifted array is printed to the console ...

	maxon::BaseArray<Int32> myArray;
	for (Int32 i = 0; i < 16; ++i)
	{
		iferr(myArray.Append(i))
		{}
	}

	ApplicationOutput("Before shifting: @", myArray);

	const Int32 shift = 10;
	for (Int32 i = 0; i < shift; ++i)
	{
		iferr(myArray.Append(myArray[0]))
		{}
		iferr(myArray.Erase(0))
		{}
	}

	ApplicationOutput("After shifting: @", myArray);

... and here's the result of the above code:

Before shifting: {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15}
After shifting: {10,11,12,13,14,15,1835887981,1,2,3,4,5,6,7,8,9}

Where did value 0 go?
Weird, isn't it?

All we did was append the first value of the array at the end of that array.
Then erase the first value.
Repeat this 10 times.

Now, I know that the first time the MyArray.Append(myArray[0]) is performed the array needs to increment in size. All fine and well, but one would expect that element myArray[0] gets appended to the resized array, and not some uninitialized value.
Sure, one could use Resize or EnsureCapacity and increment the array upfront.

Or alternatively first copy the element, then erase it, and finally append it at the end. This would avoid the need for the array to grow. As in following:

	const Int32 shift = 10;
	for (Int32 i = 0; i < shift; ++i)
	{
		const Int32 tmp = myArray[0];
		iferr(myArray.Erase(0))
		{}
		iferr(myArray.Append(tmp))
		{}
	}

But still ... who would have expected the result from the original code?
I sure didn't expect it.
So, is this a bug, or user-error?

PluginStudent

Just some note: please use proper error handling in your code e.g. implementing iferr_scope_handler.

Putting iferr() {} everywhere is pretty bad style no one should copy.

kbar

@PluginStudent said in Fun with maxon::BaseArray:

Just some note: please use proper error handling in your code e.g. implementing iferr_scope_handler.

Putting iferr() {} everywhere is pretty bad style no one should copy.

I think here @C4DS is just quickly writing a test scenario to highlight the problem. Not a complete final implementation.

r_gigante

Hi @C4DS thanks for reaching out us and sorry for getting late here.

With regard to the strange behavior you've pointed out, it seems to be a bug that is now under review. To temporarily mitigate it you have to cast the value returned from the [] operator before appending it to the proper type.
Something like myArray.Append(Int32(myArray[0])) iferr_return; is expected to work.

Best, R

EDIT: no it's not a bug, it's by design -

r_gigante

I need to spend some further consideration since it looks like I was unprecise.

When the BaseArray::Append() and the capacity of a BaseArray is reached - BaseArray default size is 16 elements - the internal BaseArray::IncreaseCapacity()is called and it sequentially allocates a new memory block, copies the values of the previous block to the new one, and then the old one is released. Being the values passed to the BaseArray::Append() a reference to myArray[0], it references the first element of the old block and because the code in Append() accesses the reference after the old block has been released, that access is illegal.

@r_gigante said in Fun with maxon::BaseArray:

To temporarily mitigate it you have to cast the value returned from the [] operator before appending it to the proper type.

Actually by writing Int32(myArray[0]) - and here I was indeed unprecise because it is NOT casting but temporary local creating of a copy - a local Int32 copy of myArray[0] is made on the stack at first by the compiler (before calling Append) and then Append(Int32(myArray[0])) references that local copy which is legal.

This behavior is NOT a bug - another initial overlook of mine - but rather a behavior by design which for example is also part of std::vector::push_back().

So, as a rule of thumb, cases where references to array elements are used while the array gets resized must be avoided. This note will be added to the BaseArray Manual in our documentation.

Sorry for the confusion I initially generated and, if further clarification are needed, feel free to come back.

R.