0

I am trying to experiment a bit with binary trees. I ran the following code:

#include<iostream>

using namespace std;

struct tree{
   char data;
   tree *left=NULL;
   tree *right=NULL;
   tree *parent=NULL;
};   


tree newNode(char d)
{
   struct tree node;
   node.data = d;
   return node;
}
int main()
{

   struct tree currentNode = newNode(' ');
  currentNode.left = &newNode(' ');
         return 0;
}

I get this error:

error: taking address of temporary [-fpermissive]

currentNode.left = &newNode(' ');

what is causing this error and how do i rectify this?

mybrave
  • 1,662
  • 3
  • 20
  • 37
Naveen
  • 1,363
  • 7
  • 17
  • 28
  • 2
    You must allocate nodes using `new` or something similar. `struct tree node;` is deleted when you exit from the function, and the copy of it you return is deleted at the end of the line (roughly) where you call the function, so even if the compiler allowed you to take the address here, you'd get a dangling pointer. – HolyBlackCat Apr 05 '20 at 10:09
  • 1
    You can't bind a temporary to a pointer. For this kind if structure the simplest thing to do is dynamically allocate your nodes. Also, if you're wanting to learn C then cool, but I'd use a C compiler. Learning the mythical c/c++ language is a mistake. If you're actually learning c++, then burn the resource you're using and find a better one :). – George Apr 05 '20 at 10:11
  • @HolyBlackCat what should i change in this code? – Naveen Apr 05 '20 at 10:12

2 Answers2

2

The return value from a function is called a temporary. Temporaries only exist for a short time, so if you want to do something with a temporary then you have to do it immediately, before it is destroyed. But you are taking the address of a temporary, which means your program is going to have a pointer to an object which has been destroyed. And so your program is likely to crash.

The way to do this is dynamic memory allocation. Now clearly you are at school or university, so they must be trying to teach you about this topic. It's far too big a topic to explain here, so I suggest you go back to your notes or your favourite C++ book and research dynamic memory allocation.

But quickly here's how to fix your code

tree* newNode(char d)
{
   tree* node = new tree; // dynamically allocate a node
   node->data = d;
   return node;
}

int main()
{
    tree* currentNode = newNode(' ');
    currentNode->left = newNode(' ');
}

Also your code (and your thinking) would be greatly improved if you sorted out the difference between a node and a tree. They are not the same thing. Your struct tree is actually a node, so it has completely the wrong name.

BTW, good job paying attention to compiler warning messages. Most newbies don't do that, they're just happy the code compiled. In this case the warning message showed up a serious problem in your code.

john
  • 85,011
  • 4
  • 57
  • 81
  • 3
    It is not a warning. It is a hard error. Taking the address of rvalues is not allowed by the language. – walnut Apr 05 '20 at 10:23
2

The expression newNode(' ') creates a temporary tree object, as suggested by the error message. This is an object which lifetime is limited to the statement it appears in. That means, past that semi-colon the object will not exist any more.

For the type of structure you are doing you probably want dynamic memory allocation. One (bad) way of doing this would be tree* ptr = new tree; (in newNode). This is bad because it will probably leak resources if you are not careful to write a proper destructor. It would be better to use a unique_ptr for your pointers in your tree class:

#include <memory>

struct tree{
   char data;
   std::unique_ptr<tree> left = nullptr;
   std::unique_ptr<tree> right = nullptr;
};

And then:

currentNode.left = std::make_unique<tree>(' ');

If you must have a parent pointer, you have to use a combination of std::shared_ptr and std::weak_ptr to avoid shared_ptr loops.

struct tree {
   char data;
   std::shared_ptr<tree> left = nullptr;
   std::shared_ptr<tree> right = nullptr;
   std::weak_ptr<tree> parent = nullptr;
};

Also note: Why is "using namespace std;" considered bad practice?

bitmask
  • 32,434
  • 14
  • 99
  • 159