Why is move() causing me trouble when constructing object?

[gedamial]

Hi. I have this Tree class

#ifndef GTREE_H
#define GTREE_H
#include <iostream>

template<typename T>
class Tree
{
	struct Node
	{
		friend class Tree<T>;
		Node(T const& key, Node* lhs = nullptr, Node* rhs = nullptr) : value{ key }, left{ lhs }, right{ rhs } {}
		Node(Node const& copy) : value{ copy.value }, left{ copy.left }, right{ copy.right } {}

		T getValue() const
		{
			return value;
		}

		T value;
		Node* left;
		Node* right;
 	} *root{ nullptr };

public:
	Tree() { std::cout << "ctor for gtree";  }

	Tree(const T& _Root) : root{ new Node{ _Root } } { std::cout << "one-value ctor for tree";  }

	Tree(T&& move)
	{
		std::cout << "move ctor for tree";

		root = move.root;

 		move.root = nullptr;
	}

	Tree(std::initializer_list<T> il)
	{
		std::cout << "init-list ctor for tree";
		for (auto elem = il.begin(); elem != il.end(); elem++)
		Insert(*elem);
	}

	~Tree()
	{
	        Clear();
	}

	void Insert(T const& value)
	{
		if (!root)
		{
			root = new Node{ value };
			return;
		}

		InsertPrivate(value, root);
	}

	void Clear()
	{
		if (!root)
		     return;

		ClearPrivate(root);
		root = nullptr;
	}

private:
 	void InsertPrivate(T const& key, Node* current)
	{
		if (key <= current->value)
		{
			if (current->left == nullptr)
			      current->left = new Node{ key };

			else
			      InsertPrivate(key, current->left);
		}

		else
		{
			if (current->right == nullptr)
				current->right = new Node{ key };

			else
				 InsertPrivate(key, current->right);
		}
	}

	void ClearPrivate(Node* current)
	{
		if (!current)
			return;

		ClearPrivate(current->left);
		ClearPrivate(current->right);

		delete current;
		current = nullptr;
	}
};

#endif 

And the main function:

int main()
{	
    Tree<int> tree = move(Tree<int>{1, 5, 3, 2});
    return 0;
}

I was expecting to see the output move ctor for tree ... instead it is printed init-list ctor for tree and a crash occurs


Exception thrown: read access violation.
current was 0xFFFFFFFFFFFFFFF7.


precisely on line 97 when ClearPrivate(current->left); is called.

Any idea about why this happens?

Removing the move() call is fine though...

Thanks in advance!

[ne555]

//Tree(T&& move) //T=int
Tree(Tree&& move)

[gedamial]

I'm so idiot! Thanks... but anyway, why was it crashing?

[dutch]

Node doesn't need to befriend Tree since everything in Node is public from Tree's point of view.

You may as well get rid of your copy ctor for Node since it's just the default behaviour anyway, and nobody's likely to want to copy a Node.

Tree's ctor that takes a single value seems pointless. The initializer_list version handles that, it seems.

Your two XyzPrivate functions should be static.

  void insert( T const& value )
  {
      insert_private( root, value );
  }
  
  static void insert_private( Node*& curr, T const& value )
  {
      if ( !curr )
          curr = new Node( value );
      else
          insert_private( value < curr->value ? curr->left : curr->right, value );
  }

[gedamial]

Thank you dutch for your tips. Could you argue the last one a bit more? Why static? What's wrong with the way I've done it?

[dutch]

A non-static member function has a hidden parameter when called, i.e., the 'this' pointer that represents the object that it was called on. obj.func(x) gets converted under-the-hood to something like Object::func(&obj, x)

A static member function does not have this hidden parameter and therefore has no 'this' pointer. It is basically just a regular function that is within the namespace of the class (and that therefore also has implied "friend" access). It can be called in the same way, like obj.func(x), but in this case it translates into something like Object::func(x), with no obj parameter. In fact you could've just called it that way since it doesn't actually need an object.

#include <iostream>
using namespace std;

class Object
{
    int n;

public:
    Object( int n ) : n{ n } { }

    void member_regular()
    {
        cout << "member_regular: " << n << '\n';
    }

    static void member_static()
    {
        cout << "member_static\n";
        //cout << n << '\n';  // error (No object, so n doesn't exist!)
    }

    static void member_static2( Object& o )
    {
        cout << "member_static2: ";
        cout << o.n << '\n';  // okay (even though n is private)
    }
};

void non_member( Object& o )
{
    cout << "non_member:\n";
    //cout << o.n << '\n';  // error (since n is private)
}

int main()
{
    Object obj( 3 );
    obj.member_regular();
    obj.member_static();
    Object::member_static();
    Object::member_static2( obj );
    non_member( obj );
}

[ne555]

> why was it crashing?
it was calling the copy-constructor, the default one provided by the compiler
so it was jus copying the pointers and you've got a double delete in the destructor