Array allocator

[Darktib]

There was 2 lines in irrArray where the allocator is not used, in insert. Here is a patch to fix it:

Code: Select all

 
Index: irrArray.h
===================================================================
--- irrArray.h  (revision 4621)
+++ irrArray.h  (working copy)
@@ -181,10 +181,12 @@
                // move the rest of the array content
                for (u32 i=used-1; i>index; --i)
                {
-                   data[i] = data[i-1];
+                   //data[i] = data[i-1];
+                   allocator.construct(&data[i], data[i-1]);
                }
                // insert the new element
-               data[index] = element;
+               //data[index] = element;
+               allocator.construct(&data[index], element);
            }
            else
            {
 
 

Now the array can hold non-copy-assignable types without problems.
Here is the patch request: https://sourceforge.net/p/irrlicht/patches/278/

[chronologicaldot]

I don't get it. If you've allocated space, then why use the allocator to move the contents of one allocated slot to another? Shouldn't the normal code work on all platforms anyways? What's a non-copy-assignable type? "const"? "static"?

[Darktib]

Before or after the patch, the code will work on all platforms, that's not the problem here.

Without using an allocator, you're either:
- using operator=, which is what is done in the 2 lines I fixed. The drawback is that you can only make arrays of assignable types, ie those with an operator=. This restriction disallow types containing a reference, for example. Also note that this restriction existed for standard containers in C++03, but has been removed in C++11 (so you can have a std::vector of a non-assignable type).
- using placement new, uglier, but more powerful. It is what is done currently in the allocator. The restriction is that the type should be copy-constructible (easier to fulfill).
- using memcpy/memmove/mem... : no restrictions on the type, it can be non-copy-constructible nor copy-assignable, but this method is not used in irr::core::array currently (and is not doable using the current state of allocators).

The best way is to give the user the choice, that's why there is an allocator. And these two lines in the patch are the last not using allocator (everything else does).

Note: the following type is not copy-assignable, since references are not assignable:

Code: Select all

 
struct ComplexType;
struct NonCopyAssignable
{
    ComplexType& complex;
};
 

[chronologicaldot]

Ah, okay. Thanks for the explanation.

[Darktib]

Any news on correction in the trunk ?