Recommended Posts

irreversible    2862

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
frob    44970

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
irreversible    2862
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
Alberth    9525

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
irreversible    2862
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
irreversible    2862
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
Alberth    9525
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
irreversible    2862
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
Hodgman    51328
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
irreversible    2862
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
frob    44970

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
irreversible    2862
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


  • Similar Content

    • By Jan Haas
      Hello,
      I just found out about https://github.com/shader-slang/slang. It's a shader language and accompanied by a library that is supposed to make it easier to work with modular shaders.
      What do you think about that? 
      Here is the paper that describes the concept: http://graphics.cs.cmu.edu/projects/shadercomp/he17_shadercomp.pdf
      Thanks in advance
       
    • By Shaarigan
      So here again with some more opinion/discussion related topic about what features should/are normally be related into a game engine's core system from the implementation side of view. With a huge time I invested into writing and even more into research for my very own game engine project, there were as many differences as there are to coding guidelines. Especially open source engines (Urho3D, Lumberyard) and those that offer there source code (Unreal, CryEngine, Phyre) change during the centuries depending on current technical, coding and hardware standards. They all consist of third party code and libraries that are set apart from the core system bur rely on it.
      Now I'm on a point where I want restructure/cleanup my project(s), removing obsolete and integrate prototyping code as same as change the growing code base to have a better clean managed file structure. The question that has had to emerge for long or short is what parts of the code base should be a core feature in order to keep the core system clean and flexible. Again reading many cross references it pointed some similarity out but also huge differences (like Unreals hashing/encryption code beeing in core) so I made a list of important stuff any game needs (I left out platform specifics and audio/renderer here because they are platform dependent and should stay in an own module/system)
      Allocator (memory management) Math (Vector, Matrix, Quaternion ...) Threading (Threading, Locks) Container (apart from the STL, Queue, Buffers, certain types of Array ...) String (management, encoding, utils) Input (seen a game without input?) IO (reading/writing files and manage filesystem structure) Type Handling And a list that is optional but may/may not be inside the core code
      Logging (because it is a development tool) Profiler (see logging) Serialization Plugins Events In my opinion, logging and profiler code is a heavyly used feature in development but is stripped out mostly when deploying a release version, not all games rely on events rather than using other interaction (like polling, fixed function loops) and also the same for plugins and serializing/deserializing data; so what do you think should also be a must have in core system or should go from the list into core system as well as a must have? What else should go outside as an own module/system (into an own subfolder inside the engine project's code base)?
      Courious to read your opinions
    • By DKdowneR
      Hi! I made a tile map reading from a file. Almost everything works good, but when a player go out of map, program runs into an error and says that "vector subscript out of range". My question is how to make check for it
      Drawing map : 
      void GameplayScreen::DrawMap(SDL_Renderer *renderer) { for (int y = map.size() - 1; y >= 0; --y) { for (int x = getStartBlockX(), xEnd = getEndBlockX(); x < xEnd && x < map[y].size(); ++x) { if (map[y][x] != "0,0") { int tempX = atoi(map[y][x].substr(0, map[y][x].find(',')).c_str()); int tempY = atoi(map[y][x].substr(map[y][x].find(',') + 1).c_str()); srcRect.x = tempX * 32; srcRect.y = tempY * 32; srcRect.w = 32; srcRect.h = 32; destRect.x = x * 32 + posX; destRect.y = (y * 32 + posY); destRect.w = 32; destRect.h = 32; vBlock[Earth]->Draw(renderer, srcRect, destRect); } } } } getStartBlockX returns first map block, getEndBlockX returns last, so it's like render on screen only a piece of map, not all blocks.
      tempX returns x coordinate of tile image,  tempY y coordinate. So, for example, if map is like :

      0,0 0,0 0,0 0,0 0,0
      0,0 0,0 0,0 0,0 0,0
      1,0 2,0 0,3 1,0 1,0
      0,0 is first block image, 1,0 is one next to the first, 0,3 is 2 under the first block etc.
         
    • By Poprocks
      Hi everyone!
      I've managed to build zlib, tinyxml2 and tmxparser with cmake-gui & visual studio 2017, but when I try building the example program for tmxparser (https://github.com/sainteos/tmxparser/tree/master/test) I'm getting numerous unresolved externals for errors, and in the warnings they all state the library machine type 'x64' conflicts with target machine type 'x86'. (some of the unresolved symbols include __imp__UnhandledExceptionFilter@4, __imp__getCurrentProcess@0, __imp__HeapAlloc@12 and the file referenced in the error log is MSVCRTD.lib along with varying object files)
      Thing is, when I go back to the cmake configuration listings for all 3 of those libraries, i can't find anything to suggest i've explicitly configured anything as a 64bit build, and when I open the solutions cmake generated for each of the libraries, all of them (including the test project) have x86 as the current target.. what gives?
      Thanks in advance!
  • Popular Now