0

I'm getting the error:

taking address of rvalue [-fpermissive]
   31 |     ListNode l = ListNode(2, &ListNode(4));

when executing the following code:

#include<iostream>
class ListNode {
public:
    int val;
    ListNode *next;
    ListNode() : val(0), next(nullptr) {}
    ListNode(int x) : val(x), next(nullptr) {}
    ListNode(int x, ListNode *next) : val(x), next(next) {}
};


int main(){
    ListNode l = ListNode(2, &ListNode(4));
    return 0;
}

I don't really know how to use classes in C++ but I would like to use a Class here for the LinkedList instead of a struct.

Simon Rechermann
  • 467
  • 3
  • 18
  • 1
    `ListNode(4)` is a temporary. It will be destroyed at the end of the expression, leaving you with a dangling pointer. You need to give it a storage duration by either assigning it to a local variable, or by allocating it on the heap (e.g. with `new`/`std::unique_ptr`/etc.) – 0x5453 Jun 09 '22 at 19:34

1 Answers1

2

You would get exactly the same error with a struct. The problem is that you are taking the address of a temporary object, here &ListNode(4) and that's bad because the address will live longer than the object, and you end up with a pointer to an object which no longer exists.

To fix, turn the temporary object into a variable

ListNode m = ListNode(4);
ListNode l = ListNode(2, &m);

Now the ListNode(4) object is held by a variable, so it's safe to take the address of it.

But usually in a linked list class you solve this problem by using dynamic memory allocation with new.

john
  • 85,011
  • 4
  • 57
  • 81
  • Thanks! How would one init the List with new? – Simon Rechermann Jun 09 '22 at 19:38
  • At the moment you don't have a list, only nodes, it's important to understand the difference. But `new ListNode(4)` would allocate a new list node. – john Jun 09 '22 at 19:39
  • I tried that but I'm getting an error: palindrome.cpp:31:19: error: invalid conversion from ‘ListNode*’ to ‘int’ [-fpermissive] 31 | ListNode l1 = new ListNode(4); – Simon Rechermann Jun 09 '22 at 19:44
  • @SimonRechermann You need to use pointers. `ListNode* l1 = new ListNode(4);` This is a big topic, you really should be learning from a book. – john Jun 09 '22 at 19:46
  • What John is suggesting is more like: `ListNode l = ListNode(2, new ListNode(4));` or `ListNode * l = new ListNode(2, &ListNode(4));` The second version is safer because later when you're cleaning up the list and have to `delete` all of the nodes allocated with `new` there is no easy way to tell the nodes allocated dynamically (and need explicit clean up) from the nodes that were allocated automatically (and will free themselves, but if you accidentally `delete` one the program will break). Proper care and feeding of pointers can be tricky. – user4581301 Jun 09 '22 at 19:50
  • 1
    I think the only sensible way to implement a list is to allocate all the nodes with new. That's the purpose of having a separate `List` class, it will allocate (and free) `ListNode` objects as appropriate. – john Jun 09 '22 at 19:52
  • Thanks. I actually know this but I was somehow confused because of references. I thought pointers should be avoided and maybe l1 gets converted to a reference automatically. I don't know maybe a good C++ book would be a good starting point even if I actually know C and the basic concepts of pointers, stack, heap and so on – Simon Rechermann Jun 09 '22 at 19:53
  • Ok now I understand. I will try to implement a separate List class – Simon Rechermann Jun 09 '22 at 19:55
  • If you're already familiar with C, you might be able to skip to [an intermediate book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list), but your struggles with C-Style pointer management suggests you'd benefit from a bit of working through some of the later chapters of an introductory book. – user4581301 Jun 09 '22 at 19:58