Recommended Posts

I can't tell if I'm running into a  bug or just don't know how to write this.

Basically I have my own extended VECTOR class derived from std::vector, which is non-copiable. This is by design, because I want to enforce strict rules on when complex objects can be duplicated or how they can be stored. Hence every class that contains a VECTOR is by definition also non-copiable by default.

What I'm writing is an allocator proxy, which is used by the type library during abstract processes like deserialization - it takes a base pointer, casts it to a container type and does stuff with it. It can handle smart pointers and raw objects, but currently has no way of differentiating between the two. Which becomes a problem when a type is non-copiable. This is where things start getting weird.

In Visual Studio I can successfully use the allocator on some complex types (eg types with one or more vector members whose copy assignment and constructor are either not provided), which should not be possible (unless, I guess, VS performs some selective on-demand compilation even in debug mode). In other cases the test fails correctly and the code does not compile.

So I figured, okay - I can just modify the allocator to conditionally compile only the parts that do not handle raw non-copiable objects in containers. All I would have to do is use something like is_copy_constructible<> to  determine which parts get compiled. But no - ALL of the classes I tested this with return true - both classes that contain non-copiable members with no copy constructor defined as well as simple classes that should be trivially copiable. Conversely, is_trivially_copy_constructible<> returns false to ALL of the classes I tried. Okay, then - maybe I'm just too dumb. Besides, the compiler does tell me the offending function is the auto-generated copy assignment operator, not the constructor. So I tried is_copy_assignable<>...

Which doesn't compile:

Error    1    error C2280: 'VECTOR<IActor *> &VECTOR<IActor *>::operator =(const VECTOR<IActor *> &)' : attempting to reference a deleted function   

 

 

I'm confused - maybe I'm completely misinterpreting the docs, but how do I determine whether an object is copiable at compile time? Why does is_copy_assignable<> apparently generate the assignment operator, which then fails to compile? Isn't the entire point of type traits to determine whether a condition is true without altering the behavior of the code in the process? Is this a bug in VS2013, because cppreference.com clearly states: "checks if a type has a copy assignment operator"?

Most importantly - what is a possible solution here?

Share this post


Link to post
Share on other sites

Extending a standard library container class is almost always the wrong approach.  There are very few things in the standard library designed for extension, and those are clearly indicated.  It is possible to extend some of them, but it usually comes with unexpected ramifications.

Your desire to mix smart pointers and raw pointers, as well as trying to describe what can be copied all reek of a common problem.  Controlling the lifetime of objects is a critical aspect of software, especially true in games. What you describe sounds like a symptom of not understanding what code owns the lifetime.  What is the REAL problem you are trying to solve here? You came up with a solution of trying to do complex things with a vector, but what problem drove you to that solution?

Share this post


Link to post
Share on other sites
1 hour ago, frob said:

Extending a standard library container class is almost always the wrong approach.  There are very few things in the standard library designed for extension, and those are clearly indicated.  It is possible to extend some of them, but it usually comes with unexpected ramifications.

Your desire to mix smart pointers and raw pointers, as well as trying to describe what can be copied all reek of a common problem.  Controlling the lifetime of objects is a critical aspect of software, especially true in games. What you describe sounds like a symptom of not understanding what code owns the lifetime.  What is the REAL problem you are trying to solve here? You came up with a solution of trying to do complex things with a vector, but what problem drove you to that solution?

I'm doing three things in my overloaded vector class:

1) provide a couple of wrapper functions like quick_erase()
2) disable the copy constructor and copy assignment operator
3) provide a custom allocator

I don't see how any of these violate the extensibility of any standard library class.

I'm not mixing shared and raw pointers. Neither is this particular issue in any way related to controlling lifetimes, although fundamentally it does stem from it. The real problem is exactly the one I described in my post. Given (untested pseudocode):


struct NonCopiable {
	// disables copy semantics
	VECTOR<int32> data;
};
      
template<typename T, bool ISCOPIABLE>
class Allocator {
  public:
    // should always work
	void AllocPointer(void* basePtr)
    	{
      	VECTOR<shared_ptr<T>>* container = reinterpret_cast<VECTOR<shared_ptr<T>>*>(basePtr);
      
     	container->push_back(NewShared<T>());
		}
      
    // should not work with T = NonCopiable. I want to selectively compile this into a stub that throws,
    // indicating an error...
	template <boolean ISCOPIABLE2>
	typename std::enable_if < ISCOPIABLE2, void* >::type
	void DoAllocPlain(void* basePtr)
    	{
      	VECTOR<T>* container = reinterpret_cast<VECTOR<T>*>(basePtr);
      
     	container->push_back(T());
		}
    // ... which means that for T = NonCopiable it should become
	template <boolean ISCOPIABLE2>
	typename std::enable_if <!ISCOPIABLE2, void* >::type      
	void DoAllocPlain(void* basePtr)
    	{
      	throw("You done goofed");
		}  
      
     void AllocPlain() { DoAllocPlain<ISCOPIABLE>(basePtr); }
};
      
...

void LoadToContainer(void *basePtr, int32 contType)
{
      
      
     Allocator<NonCopiable, std::is_copy_assignable<NonCopiable>> allocator;
     ...
     if(contType == ContainerOfSharedPointers)
      	allocator->AllocPointer(basePtr);
     else
      	allocator->AllocPlain(basePtr);
}

It would make no difference if I was using std::vector directly and passing it a plain non-copiable type. It wouldn't compile, resulting in the same situation.

So, to reiterate - my question is about type traits and how to properly employ them to decide which version of AllocPlain() should be compiled.

Edit: added enable_if to the sample code and fixed a few mistakes. PS - code editing in the new forums is apparently exceptionally broken.

Edited by irreversible

Share this post


Link to post
Share on other sites

std::vector is resizable, which means it needs to be able to copy its data to a new location if it runs out of space, wouldn't it? Not sure how you can store non-copiable elements in it, my guess is not.

Resizability also means the actual vector data is not the container<> object, as far as I kcan see. If the actual vector data was in the container<> object, it could move if you add an element and it needs resizing, and the STL API doesn't say anything about that. Therefore, your allocation is perhaps not even allocating what you think you're allocating?

Instead of fighting the STL, why not make your own wrapper class around a new-ed T[] array? That would be much simpler to get working, I suspect.

Share this post


Link to post
Share on other sites
27 minutes ago, Alberth said:

std::vector is resizable, which means it needs to be able to copy its data to a new location if it runs out of space, wouldn't it? Not sure how you can store non-copiable elements in it, my guess is not.

Resizability also means the actual vector data is not the container<> object, as far as I kcan see. If the actual vector data was in the container<> object, it could move if you add an element and it needs resizing, and the STL API doesn't say anything about that. Therefore, your allocation is perhaps not even allocating what you think you're allocating?

Instead of fighting the STL, why not make your own wrapper class around a new-ed T[] array? That would be much simpler to get working, I suspect.

I'm just going to assume that I'm extremely poor at communicating my problem at this point :)

This is exactly the thing I'm trying to not to do. I want to automatically NOT generate code in a generic class that would handle non-copiables in a container, throwing instead if I accidentally I do end up trying to do so. 

I feel like a really bad communicator here. How could I further clarify my question?

Share this post


Link to post
Share on other sites
2 hours ago, Hodgman said:

A derived class can never renege on features provided by the base class; doing so is an violation of OO's LSP rule, so this approach is invalid. OO would force you to use composition instead of inheritance here. Make your own class from scratch which contains a vector within the private implementation details.

Fair enough - that makes sense. Thanks for the heads up, although I'm somewhat curious how this becomes invalid in practice so long as I use the derived class - the compiler doesn't seem to be bothered by it.

Also, that being said, why does the below code generate a deleted function compile time error for both is_copy_assignable and is_trivially_copy_assignable:

error C2280: 'myvec<int32> myvec<int32>::operator =(const myvec<int32> &)' : attempting to reference a deleted function  

is_trivially_copy_constructible yields false and is_copy_constructible yields true.

My expectation would be that all of these cases would a) compile and b) yield false on the account of my explicitly disabling the copy semantics. As mentioned above, I'm using VS2013.
 

template<typename T>
class myvec {
    private:
        std::vector<T> detail;

    public:
  		myvec() { }
  
        myvec<T> operator=(IN const myvec<T>& copy) = delete;
        myvec(IN const myvec<T>& copy) = delete;
};

  
class NonCopiableA {
        myvec<int32>	vec;
};

  
... (DOUTEX() is a debug output macro) ...
  
    DOUTEX(std::is_copy_assignable<NonCopiableA>::value);
    DOUTEX(std::is_trivially_copy_assignable<NonCopiableA>::value);
    DOUTEX(std::is_copy_constructible<NonCopiableA>::value);
    DOUTEX(std::is_trivially_copy_constructible<NonCopiableA>::value);

Share this post


Link to post
Share on other sites
5 minutes ago, irreversible said:

Also, that being said, why does the below code generate a deleted function compile time error for both is_copy_assignable and is_trivially_copy_assignable:

Nowadays, C++ compilers seem pretty much ok with having implementations only for code that you actually use. My g++ happily compiles plain classes that have unused method declarations without any implementation. In the past it would throw "invalid v-table" errors during linking.

My guess is that your test-case doesn't use all functionality, and only hits the error when you access a method that uses the deleted functionality.

Share this post


Link to post
Share on other sites
15 minutes ago, Alberth said:

Nowadays, C++ compilers seem pretty much ok with having implementations only for code that you actually use. My g++ happily compiles plain classes that have unused method declarations without any implementation. In the past it would throw "invalid v-table" errors during linking.

My guess is that your test-case doesn't use all functionality, and only hits the error when you access a method that uses the deleted functionality.

The problem is that in the above test case I don't see where the code would be used :). I'm not omitting anything.

Share this post


Link to post
Share on other sites
28 minutes ago, irreversible said:

I'm somewhat curious how this becomes invalid in practice so long as I use the derived class - the compiler doesn't seem to be bothered by it.

It's a violation of OO theory, not the OOP constructs of C++ in practice. OO theory says that any algorithm that works on a base class, must also work on all derived types. You can break these rules in theory, but that means that you can't really claim to be writing "OO" code, even though you're using OOP language features...

Another C++-specific solution would be to use private (implementation) inheritance, which sidesteps the theoretical rules that come with public (interface) inheritance, as implementation inheritance doesn't exist in the theory.

28 minutes ago, irreversible said:

Also, that being said, why does the below code generate a deleted function compile time error for both is_copy_assignable and is_trivially_copy_assignable:

That is confusing... however it doesn't happen for me on MSVC2015... Also I get:
std::is_copy_assignable<NonCopiableA>::value == false
std::is_trivially_copy_assignable<NonCopiableA>::value == false
std::is_copy_constructible<NonCopiableA>::value == false
std::is_trivially_copy_constructible<NonCopiableA>::value == false

Can you post a compilable example and note the line that causes the compile error?

Share this post


Link to post
Share on other sites
2 minutes ago, Hodgman said:

It's a violation of OO theory, not the OOP constructs of C++ in practice. OO theory says that any algorithm that works on a base class, must also work on all derived types. You can break these rules in theory, but that means that you can't really claim to be writing "OO" code, even though you're using OOP language features...

Another C++-specific solution would be to use private (implementation) inheritance, which sidesteps the theoretical rules that come with public (interface) inheritance, as implementation inheritance doesn't exist in the theory.

That is confusing... however it doesn't happen for me on MSVC2015... Also I get:
std::is_copy_assignable<NonCopiableA>::value == false
std::is_trivially_copy_assignable<NonCopiableA>::value == false
std::is_copy_constructible<NonCopiableA>::value == false
std::is_trivially_copy_constructible<NonCopiableA>::value == false

Right. I might look into upgrading to 2017 after all then. The behavior was suspicious enough that I actually googled for a compiler bug and as noted above, apparently there's some truth to that. Thanks for confirming.

is_standard_layout seems to cut it for now, although I'm unclear what the shortcomings might be in the grand scheme.

Share this post


Link to post
Share on other sites

All of that still leaves the fundamental question, why do you have this problem in the first place:

On 6/24/2017 at 7:47 AM, irreversible said:

It can handle smart pointers and raw objects, but currently has no way of differentiating between the two. Which becomes a problem when a type is non-copiable. This is where things start getting weird.

You seem to still be trying to make a single container that stores two different types of things. The two things are one kind that has lifetime managed elsewhere, and another thing that has lifetime managed by the container.

I'd be more focused on figuring out how to not have the situation in the first place rather than trying to force a complex solution onto an avoidable problem.

Share this post


Link to post
Share on other sites
11 minutes ago, frob said:

All of that still leaves the fundamental question, why do you have this problem in the first place:

You seem to still be trying to make a single container that stores two different types of things. The two things are one kind that has lifetime managed elsewhere, and another thing that has lifetime managed by the container.

I'd be more focused on figuring out how to not have the situation in the first place rather than trying to force a complex solution onto an avoidable problem.

And still, the description applies to the allocator proxy, not the container itself.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now


  • Announcements

  • Forum Statistics

    • Total Topics
      628333
    • Total Posts
      2982139
  • Similar Content

    • By noodleBowl
      I was wondering if someone could explain this to me
      I'm working on using the windows WIC apis to load in textures for DirectX 11. I see that sometimes the WIC Pixel Formats do not directly match a DXGI Format that is used in DirectX. I see that in cases like this the original WIC Pixel Format is converted into a WIC Pixel Format that does directly match a DXGI Format. And doing this conversion is easy, but I do not understand the reason behind 2 of the WIC Pixel Formats that are converted based on Microsoft's guide
      I was wondering if someone could tell me why Microsoft's guide on this topic says that GUID_WICPixelFormat40bppCMYKAlpha should be converted into GUID_WICPixelFormat64bppRGBA and why GUID_WICPixelFormat80bppCMYKAlpha should be converted into GUID_WICPixelFormat64bppRGBA
      In one case I would think that: 
      GUID_WICPixelFormat40bppCMYKAlpha would convert to GUID_WICPixelFormat32bppRGBA and that GUID_WICPixelFormat80bppCMYKAlpha would convert to GUID_WICPixelFormat64bppRGBA, because the black channel (k) values would get readded / "swallowed" into into the CMY channels
      In the second case I would think that:
      GUID_WICPixelFormat40bppCMYKAlpha would convert to GUID_WICPixelFormat64bppRGBA and that GUID_WICPixelFormat80bppCMYKAlpha would convert to GUID_WICPixelFormat128bppRGBA, because the black channel (k) bits would get redistributed amongst the remaining 4 channels (CYMA) and those "new bits" added to those channels would fit in the GUID_WICPixelFormat64bppRGBA and GUID_WICPixelFormat128bppRGBA formats. But also seeing as there is no GUID_WICPixelFormat128bppRGBA format this case is kind of null and void
      I basically do not understand why Microsoft says GUID_WICPixelFormat40bppCMYKAlpha and GUID_WICPixelFormat80bppCMYKAlpha should convert to GUID_WICPixelFormat64bppRGBA in the end
       
    • By HD86
      As far as I know, the size of XMMATRIX must be 64 bytes, which is way too big to be returned by a function. However, DirectXMath functions do return this struct. I suppose this has something to do with the SIMD optimization. Should I return this huge struct from my own functions or should I pass it by a reference or pointer?
      This question will look silly to you if you know how SIMD works, but I don't.
    • By pristondev
      Hey, Im using directx allocate hierarchy from dx9 to use a skinned mesh system.
      one mesh will be only the skeleton with all animations others meshes will be armor, head etc, already skinned with skeleton above. No animation, idle position with skin, thats all I want to use the animation from skeleton to other meshes, so this way I can customize character with different head, armor etc. What I was thinking its copy bone matrices from skeleton mesh to others meshes, but Im a bit confused yet what way I can do this.
       
      Thanks.
    • By mister345
      Does buffer number matter in ID3D11DeviceContext::PSSetConstantBuffers()? I added 5 or six constant buffers to my framework, and later realized I had set the buffer number parameter to either 0 or 1 in all of them - but they still all worked! Curious why that is, and should they be set up to correspond to the number of constant buffers?
      Similarly, inside the buffer structs used to pass info into the hlsl shader, I added padding inside the c++ struct to make a struct containing a float3 be 16 bytes, but in the declaration of the same struct inside the hlsl shader file, it was missing the padding value - and it still worked! Do they need to be consistent or not? Thanks.
          struct CameraBufferType
          {
              XMFLOAT3 cameraPosition;
              float padding;
          };
    • By esenthel
      Just finished making latest WebGL demo for my game engine:
      http://esenthel.com/?id=live_demo
      Let me know what you think,
      as of now only Chrome and Firefox can run it.
      Edge, Safari, Opera have some unresolved bugs at the moment.
  • Popular Now