• Advertisement

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
Advertisement

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


  • Advertisement
  • Advertisement
  • Popular Tags

  • Advertisement
  • Popular Now

  • Similar Content

    • By lonewolff
      Hi guys,
      I am having problems with trying to perform a basic 'shift left' on a char.
      char temp[1]; temp[0] = buffer[0] << 1; // buffer[0] is 0xff After this I have temp[0] writing to a file. Instead of being the expected 0x7F it is written as 0xF8.
      Any guidance on what I am doing wrong would be awesome.
      Thanks in advance
    • By sergio2k18
      Hi all
      this is my first post on this forum.
      First of all i want to say you that i've searched many posts on this forum about this specific argument, without success, so i write another one....
      Im a beginner.
      I want use GPU geometry clipmaps algorithm to visualize virtual inifinte terrains. 
      I already tried to use vertex texture fetch with a single sampler2D with success.
       
      Readed many papers about the argument and all speak about the fact that EVERY level of a geometry clipmap, has its own texture. What means this exactly? i have to 
      upload on graphic card a sampler2DArray?
      With a single sampler2D is conceptually simple. Creating a vbo and ibo on cpu (the vbo contains only the positions on X-Z plane, not the heights)
      and upload on GPU the texture containing the elevations. In vertex shader i sample, for every vertex, the relative height to te uv coordinate.
      But i can't imagine how can i reproduce various 2d footprint for every level of the clipmap. The only way i can imagine is follow:
      Upload the finer texture on GPU (entire heightmap). Create on CPU, and for each level of clipmap, the 2D footprints of entire clipmap.
      So in CPU i create all clipmap levels in terms of X-Z plane. In vertex shader sampling these values is simple using vertex texture fetch.
      So, how can i to sample a sampler2DArray in vertex shader, instead of upload a sampler2D of entire clipmap?
       
       
      Sorry for my VERY bad english, i hope i have been clear.
       
    • By mangine
      Hello. I am developing a civ 6 clone set in space and I have a few issues. I am using Lua for the logic and UI of the game and c++ directx 12 for the graphics. I need a way to send information between Lua and c++ occasionally and was wondering what is the best and most flexible (and hopefully fast) way to do this. Don't forget that I also need to send things from c++ back to Lua. I know of a lua extension called "LuaBridge" on github but it is a little old and I am worried that it will not work with directx 12. Has anybody done something similar and knows a good method of sending data back and forth? I am aware that Lua is used more and more in the industry and surely plenty of AAA game programmers know the answer to this. I want a good solution that will hopefully still be viable code in a couple of years...
    • By owenjr
      Hi there.
      I'm pretty new to this and I don't know if it has been asked before, but here I go.
      I'm developing a game using SFML and C++.
      I would like to use the "Tiled" tool to load maps into my game but I don't actually find any tutorial or guide on how to exaclty use it (I know that I have to read an XML file and stuff). I just step into diverse projects that make all a mess. 
      Anyone knows where can I find good information to make my map loader by myself?
      Thanks in advantage!!
    • By MHG OstryTM
      Hello guys,
      I've released my game for the first time. I'm very excited about it and I hope you'll enjoy the game - Beer Ranger. It's a retro-like puzzle-platfromer which makes you think a lot or die trying. You have a squad of skilled dwarfs with special powers and your goal is tasty beer. There is a lot of traps as well as many solutions how to endure them - it is up to your choice how to complete the level! 
      Link to the project: Project site
      Link to the Steam site with video: Beer Ranger
      Have fun and please write feedback if you feel up to.
      Some screens: 




  • Advertisement