Page 1 of 1

Array allocator

Posted: Thu Dec 26, 2013 7:41 pm
by 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/

Re: Array allocator

Posted: Fri Dec 27, 2013 9:01 am
by 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"?

Re: Array allocator

Posted: Fri Dec 27, 2013 10:50 am
by 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;
};
 

Re: Array allocator

Posted: Mon Jan 06, 2014 9:16 am
by chronologicaldot
Ah, okay. Thanks for the explanation. :)

Re: Array allocator

Posted: Tue Jan 07, 2014 7:47 pm
by Darktib
Any news on correction in the trunk ?