Patrick
it's a return statement
Patrick
and if some voodoo like copy elision is occurring, it's setting the field of another class
Patrick
Let's see if I can make a small example
I'm doing
new Cell{getValue()} which initialises a Value field in my Cell class. The return statement in getValue() is where the copy constructor generally copies into a fresh this object.
But on the second instance it's overwriting another Value object in memory
Patrick
that is, on the second return value; it's calling the copy constructor where *this is already instantiated
Naman
Return by reference, i think it ll solve the problem
Patrick
How am I meant to do that?
Patrick
the return is Value type
Patrick
Value EVM::eval (Cell* a, Cell* p) {
... lots of business logic ...
return a->value;
}
Patrick
Maybe I could force it to copy into a new object consistently, before returning?
Naman
Value& EVM::eval (Cell* a, Cell* p) {
... lots of business logic ...
return value;
}
Patrick
Ah, I must return by value unfortunately
Naman
You can you this way if Value is a user defined
Patrick
I really don't mind the copying, in fact I need it, but it's copying it into an already existing object that's the problem
Naman
Yes, try to create a temporary object
Patrick
Nope, I'm still getting the issue :/
Patrick
it's on the return statement that it's copying into the already existing object
Patrick
Value EVM::eval (Cell* a, Cell* p) {
... lots of business logic ...
Value ret = a->value;
return ret;
}
This just copies twice, the second time into the existing obj
Patrick
Where does C++ get its copy constructor this pointer from?
Patrick
I think RVO might be messing me up a little but I'm not experienced enough to say
olli
The copy constructor copies the content and constructs a new object, one that does not exist before. Can you provide a whole example showing your issue?
Patrick
I can't unfortunately, it's hundreds of lines through multiple files
Patrick
and it's definitely an object that already exists that it's copying into, in my case
olli
then it's not calling the copy constructor
Patrick
I watch it happen with GDB
olli
do you have a custom copy constructor?
Patrick
Yes
olli
it's really hard to say what's going on w/o code
Patrick
idk how to share it but I'll try
Patrick
//The Cell struct
struct Cell {
Value value;
Cell* next = nullptr;
~Cell ();
};
//Calling EVM::eval
Cell* head = new Cell{eval(a, p)};
//Returning from EVM::eval
return a->value;
//Value's copy constructor
Value::Value (const Value& obj)
: _ref(obj._ref), _data(obj._data), _type(obj._type) {
++refs[_ref];
}
A Value's _data contains a pointer, and in my erroneous situation the Value is being copied into that pointer, but knowing that probably isn't important.
olli
what's the type of _ref ?
Patrick
uint16_t
olli
and refs is a global array?
Patrick
Yes, it's a custom ARC implementation
Patrick
It's really really strange because I've never had an unrelated object get trashed by a copy constructor. It has been working fine until a strange edge case where the first return a->value; will copy into a blank Value, but the second time it will copy into an existing object
olli
how do you tell it's an existing object?
Patrick
Because a Value's _data is a pointer, and it's that pointer it's trashing
Patrick
which causes my business logic to infinitely loop as it tries to traverse something suddenly referencing itself
Naman
Value EVM::eval (Cell* a, Cell* p) {
... lots of business logic ...
Value ret = a->value;
return ret;
}
This just copies twice, the second time into the existing obj
You are saying copy constructor is called twice right?
Patrick
You are saying copy constructor is called twice right?
When I was creating that temporary object yes. It's the return copy that's causing an issue
Patrick
I created the temporary object to see if it would help and it didn't
Naman
Split the line
Value ret = a->val into two
Patrick
I'll try, but at this point I don't feel anything leading up to the return will mitigate it
Naman
Value ret;
ret = a->val;
Like this and let me know if it solves
Patrick
It doesn't seem to just running the whole thing, but I'll closely inspect everything
Naman
Value ret;
ret = a->val;
Like this and let me know if it solves
Actually it ll avoid copy constructor calling.
Patrick
Yes, it is now using the custom assignment operator I have, and it's assigning into an empty Value
Patrick
But then the return is calling the copy constructor
Patrick
and once more *this already exists
olli
yes, once you can access this it does exist, but
Value::Value(const Value& v) {
// this is not the same as v, as this has just been created
assert(this != &v);
}
Naman
//The Cell struct
struct Cell {
Value value;
Cell* next = nullptr;
~Cell ();
};
//Calling EVM::eval
Cell* head = new Cell{eval(a, p)};
//Returning from EVM::eval
return a->value;
//Value's copy constructor
Value::Value (const Value& obj)
: _ref(obj._ref), _data(obj._data), _type(obj._type) {
++refs[_ref];
}
A Value's _data contains a pointer, and in my erroneous situation the Value is being copied into that pointer, but knowing that probably isn't important.
In Value copy constructor your are accepting (const Value&) but passing by value in eval return....
Patrick
yes, once you can access this it does exist, but
Value::Value(const Value& v) {
// this is not the same as v, as this has just been created
assert(this != &v);
}
this isn't v but some other important object
Patrick
(gdb) p obj._data.cell
$13 = (Cell *) 0x56591c80
(gdb) p this
$14 = (Value * const) 0x56591c80
(gdb)
Patrick
which causes my code to go in circles, like I said
Naman
In Value copy constructor your are accepting (const Value&) but passing by value in eval return....
Make eval return by reference, i think it ll solve
olli
Make eval return by reference, i think it ll solve
don't return a reference to a temporary object (dangling reference)
Patrick
ooops, that was way too huge
olli
if you want to share larger amount of codes feel free to use pastebin (e.g. https://paste.ubuntu.com/)
Patrick
The first time
308 return a->value;
(gdb) p a->value
$1 = ... fID = 1448680576}, _type = T_Lamb}
(gdb) p *a->value.cell()
$2 = ... fID = 2}, _type = T_Op}, next = 0x56591ca0}
(gdb) n
309 }
(gdb) p *a->value.cell()
$3 = ... fID = 2}, _type = T_Op}, next = 0x56591ca0}
(gdb)
The second time
308 return a->value;
(gdb) p *a->value.cell()
$4 = ... fID = 2}, _type = T_Op}, next = 0x56591ca0}
(gdb) n
309 }
(gdb) p *a->value.cell()
$5 = ... fID = 1448680576}, _type = T_Lamb}, next = 0x56591ca0}
Patrick
if you want to share larger amount of codes feel free to use pastebin (e.g. https://paste.ubuntu.com/)
It's okay, it needed abbreviation anyway
Patrick
You can see the internal information of a->value.cell() changing after the return the second time
Patrick
And it just so happens that a->value.cell() is becoming a->value
Patrick
The first time
308 return a->value;
(gdb) p a->value
$1 = ... fID = 1448680576}, _type = T_Lamb}
(gdb) p *a->value.cell()
$2 = ... fID = 2}, _type = T_Op}, next = 0x56591ca0}
(gdb) n
309 }
(gdb) p *a->value.cell()
$3 = ... fID = 2}, _type = T_Op}, next = 0x56591ca0}
(gdb)
The second time
308 return a->value;
(gdb) p *a->value.cell()
$4 = ... fID = 2}, _type = T_Op}, next = 0x56591ca0}
(gdb) n
309 }
(gdb) p *a->value.cell()
$5 = ... fID = 1448680576}, _type = T_Lamb}, next = 0x56591ca0}
I'll see if I can make it even clearer with another attempt
Patrick
Actually I really can't, it's just... the copy constructor is picking an already existing object
I'm at a total loss
olli
do you have a custom allocator? do you use any pointer arithmetic?
Patrick
I even just tried rewriting my whole project so that
Value EVM::eval(Cell* a, Cell* p) becomes
void EVM::eval(Value& ret, Cell* a, Cell* p)
and it's still happening... even though I have to explicitly do
auto v = Value()
eval(v, a, p);
olli
given the information we have, unfortunately I can't repro your issue
Patrick
My gosh, I solved it
Because I'm managing memory all over the place I was giving ownership of that pointer to an object dutifully deleting it after, therefore causing the issue to only arise on the second instance.
Gaaaaaaaah, does everybody end up in these kinds of messes which using C++? Maybe I'm over-managing my memory.
That's 2h40m I'll never get back.
CappedCrusader
What's wrong with this code and why it doesn't work
try{
throw "123";
}
catch(string a){
cout << a << endl;
}
CappedCrusader
olli
as side note, it's considered bad practice to throw types that don't inherit std::exception
Anunay
or throw std::to_string(123) maybe 🤔
Artöm
Ludovic 'Archivist'
as side note, it's considered bad practice to throw types that don't inherit std::exception
Yep, generally you'd throw a std::runtime_exception if you don't want to make a dedicated class
Anonymous
I have a func like this:
size_t func(...) {
...
if (current_sum == sol_number) {
return sol_size;
}
..
return -1;
}