#Destructor deletes reference items after function scope ends

1 messages · Page 1 of 1 (latest)

swift abyss
#

I have a function that adds weighted binary tree leafs or complex trees ( that have a right & left side ) to a vector.
I'm adding the new binary tree to the vector depending on it's weight ( since the vector contains weighted binary trees).
The problem is that whenever the function scope ends the binary tree pointers gets deallocated for some reason.
I have declared a copy constructor, = operator, constructor, move constructor...
But still it didn't work out. It always deallocates nodes more than once just because it's leaving the scope of the function.
Here is the destructor:

~WeightedBinaryTree() {
                if (is_complex()){
                    std::pair<WeightedBinaryTree*, WeightedBinaryTree*> pair = std::get<std::pair<WeightedBinaryTree*, WeightedBinaryTree*>>(m_content); // m_content contains the binary trees.
                    delete pair.first; // left subtree
                    delete pair.second; // right subtree
                } 
            }

Here is the code that is breaking the execution:

    void insert_in_isorted(std::vector<D>& v, const D data){
        std::cout << "Into insert_in_isorted..." << std::endl;
        auto it = std::lower_bound(v.begin(), v.end(), data, 
                                [](const D& a, const D& b) {
                                    return a.get_weight() > b.get_weight();
                                });
        std::cout << "Comparaison done..." << std::endl;
        v.insert(it, data);
        std::cout << "Inserted complex tree : " << data.get_weight() << std::endl;
    }   // Right on leaving the function const D data which represents the binary tree would get destroyed with it's children.
      // In my case I have many binary trees in the vector that I'd like to merge into one single big tree.
      // exp: [w_b_t(6,'e'),w_b_t(4,'e'),w_b_t(2,'e'),w_b_t(2,'a')] I would end up with [6,4,2,2] > [6,4,4] > [6,8] > [14]

Error:
double free or corruption (out).
Abandon (Core dumped)

bitter prawnBOT
#

When your question is answered use !solved to mark the question as resolved.

Remember to ask specific questions, provide necessary details, and reduce your question to its simplest form. For tips on how to ask a good question use !howto ask.

idle isle
#

This doesn't look like it's related to the std::vector

#

it looks like there's a problem in your weighted binary tree implementation

#

but this isn't enough info to say where the problem is

#

address sanitizer is a great tool for finding out where things go wrong

#

it should give you an actual stacktrace

#

and also, you can try to create a failing testcase that's as simple as possible, then step through that with a debugger

swift abyss
#

I just removed the destructor

#

worked but not the right solution ig

idle isle
#

nah that might just leak everything

#

depends on how you're storing those binary trees

#

I think you're using something like a std::variant<std::pair<WeightedBinaryTree*, WeightedBinaryTree*>, SomethingElse>?

#

In that case, you definitely need to delete those pointers

#

if you were storing std::unique_ptrs, that would happen automatically

swift abyss
#

I get it

#

but I can't use that

#

I don't even know why I should delete the pointers

#

If you think of it

#

If I'm merging 2 leafs ( 2 pointers )

#

into one tree

#

or if I'm merging a tree and a tree into a new tree

#

I'm not deleting the pointers

#

I'm keeping them

#

and I'm using them as a result of the new tree

swift abyss
#

and whenever I put something in the destructor

#

It just breaks everything

idle isle
swift abyss
#

here is the impl

#

I have 2 issues if you could help me to understand how to resolve them It'd really help

#

whenever I drop something in the destructor it just breaks everything

#

and for some reason I have an add_dummy method that would basically take the right subtree

#

and make a new subtree with that right subtree as its left

#

and nullptr as its right

#

I'm just unable to figure it out because I'm not super advanced on cpp and pointer management is rly hard

swift abyss
#

Hello?

idle isle
#

ah yeah sorry, I was reading your code

swift abyss
#

It's terrible code

#

At first, I want to say to sorry for that

#

Still learning the basics. The language is rich but it's hard

idle isle
#

nah it's fine

swift abyss
#

Sooo I'm basically doing huffman encoding using that tree

#

and well that's an assignment but I really want to understand the language

#

The rest of the code is a huffman class that does the encoding of the data in the tree using 0s and 1s

#

and so the encoding uses the technique that I explained in the top

#
 static std::pair<utils::WeightedBinaryTree<std::size_t, D>, std::vector<bool>> encode(std::vector<D> const& source, bool add_dummy) {
        std::unordered_map<D, std::size_t> frequency_map;
        for (const auto& data : source) {
            ++frequency_map[data];
        }
#

It first gets a hashmap will values like this {'a':5,'b':6,'c':3} with letters representing the etiquettes of the nodes

#

and the values represent the weight

#

I would then get the hashmap and create the leafs inside a vector

   std::vector<utils::WeightedBinaryTree<std::size_t, D>> trees;
        trees.reserve(frequency_map.size());
        for (const auto& [data, freq] : frequency_map) {
            utils::insert_in_isorted(trees, utils::WeightedBinaryTree<std::size_t, D>::leaf(freq, data));
        }
idle isle
#

ok, so how do you create a binary tree?
Because I see there are two static methods leaf and complext, but they both return a tree by value. However, complex() expects pointers, and takes ownership of them

#

so I suspect that's your issue

swift abyss
#

Oh, so those signatures were given by my professor in the assignment

#

but he said there could be wrong things.

#

So complex shouldn't be using pointers?

#

but why though

idle isle
#

I would change leaf and complex to return std::unique_ptrs, and take std::unique_ptrs in complex() as arguments

#

I think your problem right now is that you create the nodes on the stack, which is why you can't delete them

swift abyss
#

I understand the problem kind of

#

if I can give you my thoughts

#

I'm not that good but maybe it could help

idle isle
#

sure

swift abyss
#

So let's say I have an input like this {'a':5,'b':6,'c':3}

#

I'd have a vector of trees like this right ?
[leaf(b,6),leaf(a,5),leaf(c,3)]

idle isle
#

I think so

swift abyss
#

sooo what is asked to do is to group 2 of from the end to the beginning

#

so I'd start of by combining leaf(a,5),leaf(c,3)
Like this:

            auto parent = utils::WeightedBinaryTree<std::size_t, D>::complex(&left, &right);
#

which would happen exactly in here

    void insert_in_isorted(std::vector<D>& v, const D data){
        std::cout << "Into insert_in_isorted..." << std::endl;
        auto it = std::lower_bound(v.begin(), v.end(), data, 
                                [](const D& a, const D& b) {
                                    return a.get_weight() > b.get_weight();
                                });
        std::cout << "Comparaison done..." << std::endl;
        v.insert(it, data);
        std::cout << "Inserted complex tree : " << data.get_weight() << std::endl;
    }
#

what would happen in general

#

is right after the v.insert

#

it would go to the destructor since the function just finished

#

and it would want to destroy the data node.

#

and it's children too

#

why because it's children are also WeightedBinaryTree

#

Sooooo it'd call the destructor other times

#

and it'd end up like hell no?

#

and soooo on the next iteration of that it'd try to destroy pointers that were already freeed

idle isle
#

yeah, the problem here is that you store the nodes in a std::vector, which means this std::vector actually owns them, and a node shouldn't delete its children.

#

However, there's another problem

#

you're storing pointers to children, but those pointers get invalidated when the vector reallocates

swift abyss
#

EXACTLY

#

That's why I did this

#
trees.reserve(frequency_map.size());
#

to force it to not reallocate

idle isle
#

ah ok

swift abyss
#

so the first problem remains

idle isle
#

another solution would have been to store indices instead of pointers

#

in that case, I think you're fine with just not using delete in the destructor

#

because a node doesn't actually own its children

#

all nodes are owned by the std::vector

swift abyss
#

I mean I don't know if I'm right

#

but there is another big problem

#

with the vector approach

#

let's say you merged 2 leafs into a tree

#

it'd mean that you have sthin like this for example.
4
/
2 2

#

but my issue is why does the destructor even get called

#

If we're leaving the function

#

shouldn't the destructor only be called

#

if I make a delete statement

#

like I'm passing a reference why would c++ touch that reference without asking me about it

#

Because at that point after 2 nodes and freeing them

#

the new tree would be malformed at that point

#

and when merging it the 2nd time with another tree

#

it'd break

#

because you're trying to free it's children which are already freed

idle isle
#

if you're creating a node as a local variable, then when you exit the scope where you declared that variable it gets destroyed

swift abyss
#

what about parameters?

#

function params I mean

idle isle
#

they're like local variables

#

if you pass them by reference nothing happens

#

because a reference isn't actually the object itself, it's just another name for it

#

ok so I still have couple of questions

swift abyss
#

that's weird

#

but like here

#
    void insert_in_isorted(std::vector<D>& v, const D data){
        std::cout << "Into insert_in_isorted..." << std::endl;
        auto it = std::lower_bound(v.begin(), v.end(), data, 
                                [](const D& a, const D& b) {
                                    return a.get_weight() > b.get_weight();
                                });
        std::cout << "Comparaison done..." << std::endl;
        v.insert(it, data);
        std::cout << "Inserted complex tree : " << data.get_weight() << std::endl;
    }
#

the vector is passed by reference

#

and I recall that the data was too

#

I'm not creating anything nor are the params passed by value

#

what makes the destructor get called though

idle isle
#

well data isn't passed by reference here, but also I'm concerned about the insert

#

because that moves the elements after it around, which means all pointers to them get invalidated

swift abyss
#

ooooh, hmmm

#

now I get it

#

a good idea would then be to create a new vector

#

but yeah still same issue

#

since I'm having a void being returned

#

I can't really return that

idle isle
#

sorry, I'm not quite sure what this code is doing, to be honest

#

it's a vector of D, not of nodes, right?

swift abyss
#

yes

idle isle
#

so there's no tree involved here?

swift abyss
#

but D is a generic

#

like a template* ( sorry confusing java with cpp)

idle isle
#

yeah

swift abyss
#

soo I'm passing complex and leaf there

idle isle
#

and you instantiate D with your WeightedBinaryTree<W, D>?

#

where the second D is a different type?

swift abyss
#

yes

#

so

#

if it's a leaf

#

W would be the weight

#

I guess we could as an example a size_t for that

#

the D in that case would be the data held

#

meaning for example a char char* or w/e

#

just some data

idle isle
#

right

swift abyss
#

in the case of complex

#

it's another story

#

W is the weight of the children

#

which are 2

#

and D would be the <std::pair<WeightedBinaryTree<W,D>,WeightedBinaryTree<W,D>>

#

it'd basically be 2 binary trees that would either be complex

#

or just leafs

#

it's basically a recursive definition of a binary tree ( well kind off :/)

idle isle
#

hmm

#

so you have a WeightedBinaryTree<size_t, std::pair<WeightedBinaryTree<size_t,D>,WeightedBinaryTree<size_t,D>>?

swift abyss
#

If I could share my screen just to show you the behavior of my code. It'd be understood. But yeah.

#

Generally

#

the D in the children would be same as parent

#

I mean let's consider it like that as of now

#

else you'd just have a tree with some heterogen data

idle isle
#

but like, it wouldn't actually be a WeightedBinaryTree<size_t, std::pair<WeightedBinaryTree<size_t,D>,WeightedBinaryTree<size_t,D>>, right?

swift abyss
#

it would

idle isle
#

it'd be a WeightedBinaryTree<size_t, char> or whatever

swift abyss
#

yeah D would be either another tree

#

or just some raw data

idle isle
#

but why would D be another tree? That would mean that each node can contain another tree

swift abyss
#

That's the professor idea

idle isle
#

or I guess i should take a step back

swift abyss
#

not mine hahaha

idle isle
#

do you store data in inner nodes?

#

or only in leaf nodes?

swift abyss
#

leaf nodes contain data only

#

yes

#

only in leaf nodes

#

complex nodes would only have a weight which is the sum of the weight of the children

idle isle
#

makes sense

#

and the data you store in a leaf node is whatever, but it's not another tree, right?

#

because that's what WeightedBinaryTree<size_t, std::pair<WeightedBinaryTree<size_t,D>,WeightedBinaryTree<size_t,D>> would mean

swift abyss
#

yes

idle isle
#

it would mean you literally store other trees (of different type) in leaf nodes

swift abyss
#

exactly

#

ah no no

#

leaf nodes are <W,D>

#

where W is the weight

#

and D is the data

#

and the data cannot be a tree

#

let's assume it's like that

idle isle
#

ok

swift abyss
#

But then still as you said

#

a node holds the definition for other nodes

#

so if I delete the children of a node

#

calling delete on those would call delete on their children

#

it'd basically end up as a stack of destructor calls ( I'm not sure about that )

idle isle
#

yeah

#

but I'm still trying to understand something else here, sorry

#

so in the insert_in_isorted function, is D the same data type that you store in leaf nodes?

swift abyss
#

because a child node cannot be detached from his parent

#

Ah I get what you mean now

#

Yes

#

well no actually

#

it depends

#

if it's a leaf node

#

you're just passing in it's D element

#

which is the data

#

but if it's a complex one

idle isle
swift abyss
#

D would basically be the tree ( either the complex one or the leaf )

#

it's the node yes

idle isle
#

yeah I'd rename that to make it clearer it's not the same as the D in the tree

#

because that confused me

swift abyss
#

I'll do that

idle isle
#

just use T if the type doesn't matter

#

ok so back to the actual problem

swift abyss
#

I'll do that

idle isle
#

you're storing pointers to nodes, but those nodes get moved around when you insert new nodes into the vector

#

which means the pointers end up pointing to other nodes

swift abyss
#

alright clear enough

#

so the insertion as you said

#

would invalidate the pointers since they are getting shifted

#

and reallocated

idle isle
#

it invalides all pointers after the insertion, yeah

#

if you only insert at the end, that's not a problem

#

otherwise, you'd need to use a different data structure

#

either store std::unique_ptr<Tree> in the std::vector or use a std::set

#

the benefit of the std::set is that it's always ordered and insertion is in O(log n) instead of O(n)

#

however, that still doesn't feel right to me

#

because a std::set is internally implemented as a tree, so you're basically storing your tree in another tree

swift abyss
#

so the trick is to use unique ptrs?

idle isle
#

why does it need to be sorted?

swift abyss
#

that's the algorithm given

#

It's not my idea either, it's about huffman's encoding

#

that's why he's sorting it's part of his algorithm

#

and when I use unique_ptrs I can basically end up having no issues anymore with that?

idle isle
#

but wait

#

if my understanding of huffman coding is correct (it's been a while), you're creating the tree once, then not modifying it, right?

#

and you're creating the tree by inserting one node after the other, ordered by weight?

#

that makes things way simpler

swift abyss
#

exactly that

idle isle
#

because it means you only need to insert at the end

#

so you never invalidate pointers in the actual tree

swift abyss
#

but for example

#

if I had this

#

[2,4,1,6]

idle isle
#

hmm I need to look up how exactly you construct huffman trees again

swift abyss
#

I can't insert those at the end

#

I'll give you the questions

#

give me a sec

idle isle
#

but anyway, you almost certainly want to use a priority queue

#

inserting at an arbitrary index of a std:;vector is an O(n) operation

#

so if you do that n times, that's O(n^2) in total

swift abyss
idle isle
#

ah I see

#

so the task is literally telling you to insert into sorted vectors

swift abyss
#

It is like that yes

#

It doesn't make any sense

idle isle
#

hmm

#

well then I guess using a std::vector<std::unique_ptr<WeightedBinaryTree<...>>> is the way to go

swift abyss
#

I guess I lost my entire day for nothing hahaha

#

that's sad

#

I tried alot to figure out a way, I refused to use unique_pointer

swift abyss
#

but at the end of the day there is no other way

#

This is what I mean

#

like you see

#

he's also giving the signatures

#

that's why I refused to use it

#

he made it with classic pointers so I just thought that I could possibly do it like that

#

but it just caused error after error

idle isle
#

well you can use pointers

#

but this pointer needs to point to an object that doesn't move around or get destroyed while you're still using the pointer

swift abyss
#

yeah so that doesn't make any sense to insert in the middle of the array

#

vector sorry*

idle isle
#

well you could have this vector store only pointers

#

and keep the actual objects somewhere else

#

like in another vector

swift abyss
#

in my example I'm passing a pointer on an object

#

but yet it still breaks

#

for the vector method

#

but as soon as I try to call the insert here or there it just goes directly to the destructor

#

even though it's an array of pointers

idle isle
#

you have one vector that stores the nodes such that they never move around

#

because that vector has sufficient capacity that it doesn't need to reallocate, and you're only inserting at the end, with push_back

#

and then the sorted vector just deals with pointers

swift abyss
#

I'd lose the order at that point no?

idle isle
#

yeah that's why you have another vector of pointers

swift abyss
#

I don't get one small thing in what you're saying

#

nodes are pointers no?

idle isle
#

with node, I mean a WeightedBinaryTree object

swift abyss
#

So one vector would hold the WeightedBinaryTree

#

like the value of those

#

and the other would reorder the pointers?

idle isle
#

yeah, and a WeightedBinaryTree itself can also hold pointers to other WeightedBinaryTrees

#

which is fine because they don't move around

swift abyss
#

but didn't we say that reordering the pointers breaks everything?

#

like for the other vector

idle isle
#

reordering the objects themselves

#

but if you reorder pointers to them noone cares

swift abyss
#

Oh I get what you mean

#

it'd be sthing like this then

idle isle
#

i'm like 80% certain that that's the intended solution by the way

swift abyss
#

One final problem that I would address

#

the function is a void that takes in a vector and the new inserted value

idle isle
#

yeah, that's why I think this is the intended solution

swift abyss
#

the reference vector would be the vector of pointers

idle isle
#

it's a vector of pointers and the inserted value is also a pointer

#

you also have another vector where you emplace_back the actual object first

swift abyss
#

and then I change pointer orders?

#

depending on that

#

so I'd get the pointer reference vector

#

get the content of the objects that are related to them

idle isle
#

you emplace_back the new node in this other vector, then you get a pointer to it and insert it in the sorted vector of pointers

swift abyss
#

add the new object to the new array

#

and then I just change the destination of each pointer

idle isle
#

nope

#

you never change where a pointer points to

swift abyss
#

I get it

idle isle
#

all you do is that you insert a new pointer in a vector of pointers

#

which moves some pointers around, but doesn't change where they point to

swift abyss
#

but like let's consider we have some ints for that

#

[1,2,3,5] that are represented by adresses [A,B,C,D]

#

if I'm inserting 4

#

that has address E

idle isle
#

yeah

swift abyss
#

as you said I can't insert it into the middle of the pointers array since that'll break everything

idle isle
#

you compare the pointers by calling the operator< on the nodes they point to

#

no you can

#

that only moves pointers around, which is fine

swift abyss
#

I add so the result is [A,B,C,E,D]?

idle isle
#

yeah

swift abyss
#

but that's what I did in my other work no?

#

I had a vector of pointers

idle isle
#

the vector of actual nodes would be [1, 2, 3, 5, 4], and the vector of pointers would be [A, B, C, E, D] because that's the sorted order

swift abyss
#

and I kept adding new pointers

#

using the vector insert

#

and that made it break?

#

I think I understood something incorrectly

idle isle
#

but in the old version you had a vector<Tree>, right?

#

and a Tree object can also have a pointer to another Tree

#

which means that moving a Tree object around is a bad idea, because all pointers that pointed to it are now garbage

#

but in this solution, you never move Trees around, because you only ever insert new Trees at the end of the vector

#

you can insert new elements into a vector of Tree*, which moves Tree*s around, but that's fine

#

you don't have pointers to pointers to trees

swift abyss
#

I understand now perfectly

#

final question

#

which is something I took as an axiom

#

why is moving a Tree object makes all pointers that pointed to it garbage

#

Like why is it like that

#

I just moved something in a vector

#

the pointer should remain intact logically no?

idle isle
#

garbage was the wrong word, yeah

#

but at runtime a pointer stores an address, right?
So if you dereference a pointer, you get whatever is at that address

swift abyss
#

oh yeah yeah sorry

idle isle
#

so let's say you have a vector vec of Trees

#

and you have a pointer to vec[0]

#

so if you insert something at vec.begin(), that means all elements of vec move to the right by one, and the new element is now at vec[0]

#

so this pointer now points to the new element, instead of what it pointed to previously

#

which would now be at vec[1]

#

instead of storing a pointer, you can also store an index

#

that's maybe easier to understand

swift abyss
#

I get what you mean

#

When I'm moving something

#

I'm mixing pointers

#

I'm shifting pointers

#

so I'm losing access to my trees

#

just by doing that

idle isle
#

you're changing the addresses, but the pointers still point to the old addresses

#

thinking about it in terms of indices in the vector is much easier

#

and it's also how you should implement this by the way

swift abyss
#

so instead of having 0 pointing to tree 0

idle isle
#

whenever I said "vector of pointers", you should instead use a vector of indices

swift abyss
#

I'd have 1 pointing to tree 0

#

if I'm pushing from the beginning

idle isle
#

yeah

#

let's say you're storing the index at which you've stored a tree in the vector

#

like, "this tree is at index 7"

#

now, when you insert something before index 7, the tree actually moves to index 8

#

but you're still storing that it's at index 7, because you didn't change that

#

so now when you try to access the tree again, by going to vec[7], you're actually accessing a different tree

swift abyss
#

holy moly that's interesting

#

but then why would the destructor get called

#

if the vector size already fits

swift abyss
#

yes

#

it always calls the destructor on insert_in_isorted

idle isle
#

yeah, because what that does is that it copies data, so you're inserting a copy of it in the vector, and then when you leave the function you're calling the destructor of data

#

it's not a problem in the version where you're just storing pointers in the vector

swift abyss
#

but the copy should get destroyed

#

not the real

#

object

idle isle
#

the copy is inserted into the vector

#

it gets destroyed when the vector gets destroyed

#

you'd want to move the object instead of copying it, but that doesn't matter now

#

because it's just not a problem with pointers

swift abyss
#

I get almost everything

#

but if I use the function

#

or method

#

and I create some pointers as you said

#

that would point over the right values

#

like the sorted ones

#

I'd need a copy of the object vector in the method too

#

and that would mean that I'll be ending up destroying the objects as soon as the function ends

idle isle
#

where would you need a copy?

swift abyss
#

oh well I don't think I need it

#

true

#

I already have the values from the pointers

idle isle
#

yeah

swift abyss
#

that's cool but hard

#

I'll see if I'll use that or just unique_pointers

#

but the idea is cool though

#

thank you for your help, I really appreciate it

idle isle
#

the unique_ptr idea doesn't really work with the functions you're supposed to use

swift abyss
#

There is still something that I couldn't figure out but I'm tired, like really tired

#

can I ask you about it tommorow?

idle isle
#

yeaha

swift abyss
#

Thank you, you helped me alot on this

idle isle
#

although it might be some time until I ansert

#

no problem

swift abyss
#

it's alright

#

I'll just send it and tag you if that doesn't bother you ofc

#

when you feel like you want to answer it there you go

#

thank you once again !

bitter prawnBOT
#

@swift abyss Has your question been resolved? If so, type !solved :)

swift abyss
#

!solved

bitter prawnBOT
#

Thank you and let us know if you have any more questions!

This thread is now set to auto-hide after an hour of inactivity

swift abyss
#

Hello

swift abyss
idle isle
#

what problem?

swift abyss
#

In the first place I'd have to make the insert_isorted method use a vector of pointers as a param right?

#

instead of a vector of trees

idle isle
#

yeah

swift abyss
#

I wouldn't need the other vector at that point right?

idle isle
#

well the pointers have to point somewhere

swift abyss
#

yes but in my insert_isorted

#

I can only pass one vector

#

while there are 2

#

one of them would stay in the main code and the vector of pointers would keep changing in the insert i_sorted?

#

because only one vector is allowed to get to the insert_isorted

idle isle
#

you first create the node in one vector, then you pass a pointer to it to inser_isorted

swift abyss
#

but when I would have to fusion 2 nodes of the tree vector

#

how would I do that

#

like for example I have 4 leafs

#

and I want to fusion 2 of them into a tree

#

that would basically force me to mutate the tree vector

idle isle
#

nope

swift abyss
#

assuming t[i] is a leaf where t[i] represents the weight

#

if I had [4,2,2,2]

#

I want to make it [4,4]

#

I'd be forced to pop two 2s and add a 4

idle isle
#

but you wouldn't actually remove the leafs, right?

#

you'd just insert a new node that has pointers to them

swift abyss
#

I have to remove them

#

I mean theoretically

#

they still exist

idle isle
#

you'd remove them from the vector of pointers

swift abyss
#

but I just remove them from the vector

idle isle
#

after you create a node, you insert a pointer to it into this vector of pointers

#

from them on, you only ever deal with the pointer

swift abyss
#

then I'd only need 1 vector

#

the pointers

idle isle
#

but the pointers have to point somewhere

#

the vector of nodes is just there so that it can store the nodes

#

it's kind of like storing them on the heap

#

except that you don't need to delete anything, you just automatically destroy this vector at the end

swift abyss
#

I'll code it now

#

thank you

swift abyss
#

there is something weird happening

#

it inserts them but I think that by shifting the pointers in the vector

#

it actually does something super weird

#

I mean for some reason it stored the same memory address 4 times lol

#
        for (const auto& [data, freq] : frequency_map) {
            std::cout << data << "," << freq << std::endl;
            auto leaf = utils::WeightedBinaryTree<std::size_t, D>::leaf(freq, data);
            std::cout << &leaf;
            trees.push_back(leaf);
            utils::insert_in_isorted(trees_pointers, &leaf);
        }
#

the auto leaf line is using the same address every time

idle isle
#

that's because you're inserting the address of the local variable leaf

#

which only lives for one loop iteration

#

and then the next loop iteration reuses the memory, so it will again have the same address

#

what you want to insert is the address of the object you inserted into the vector

swift abyss
#

shouldn't the variable get destroyed right after the iteration ends

idle isle
#

it gets destroyed after each loop iteration, yeah

#

so the pointer will be invalid

swift abyss
#

true

idle isle
#

this is how I'd do it: c++ auto leaf = utils::WeightedBinaryTree<std::size_t, D>::leaf(freq, data); trees.push_back(std::move(leaf)); utils::insert_in_isorted(trees_pointers, &trees.back());

swift abyss
#

I did that

#

yes I fixed it

#

I didn't do move I just moved the utils::Weighted... into the tree .push_back

#

but it's almost like you did

#

but you just stole the leaf attributes with std::move

#

which is cooler :p

#

and better

idle isle
#

don't forget to call reserve on the the tree vector

#

because if that ever reallocates all pointers will be invalid

idle isle
#

I don't know if you're already doing that

swift abyss
#

now I did that

#

but there is something weird happening

#
d,1
Into insert_in_isorted...
Comparison done...
1
Inserted complex tree: 1
b,2
Into insert_in_isorted...
Comparison done...
2
Inserted complex tree: 2
c,2
Into insert_in_isorted...
Comparison done...
2
Inserted complex tree: 2
a,5
Into insert_in_isorted...
Comparison done...
5
Inserted complex tree: 5
4
5 2 2 1 
Tree size : 4
STARTING TO FUSION TREES
Attempting to insert into vector...
Into insert_in_isorted...
Comparison done...
3
Inserted complex tree: 3
free(): invalid pointer
Abandon (core dumped)
#

weird stuff is happening

#

I think it's calling the destructor

#

Every single time I add something to the vector

#

it calls the destructor to destroy the copy?

swift abyss
#

something is off

idle isle
#

the destructor shouldn't delete the children

#

because all nodes are owned by the vector of nodes, which destroys them all at once when it goes out of scope

swift abyss
#
while (trees_pointers.size() > 1) {
            std::cout << "Tree size : " << trees.size() << std::endl;
            auto left = trees_pointers.back();
            trees_pointers.pop_back();
            auto right = trees_pointers.back();
            trees_pointers.pop_back();
            std::cout << std::endl;
            trees.push_back(utils::WeightedBinaryTree<std::size_t, D>::complex(left, right));
            std::cout << "Attempting to insert into vector..." << std::endl;
            utils::insert_in_isorted(trees_pointers, &trees.back());
            for (int i = 0 ; i < trees_pointers.size() ; i++)
                std::cout << trees_pointers[i]->get_weight() << " ";  
        }
#

what I do is I basically get the 2 last pointers since they refer to the smallest values

#

I pop them for the pointer vector

#

I create the new tree

#

and I push it to the trees vector

#

and then when I'm trying to work on inserti_sorted

#

it just breaks as per usual

#
void insert_in_isorted(std::vector<D*>& v, D* data) {
        std::cout << "Into insert_in_isorted..." << std::endl;

        // Find the correct position to insert the pointer based on weight comparison
        auto it = std::lower_bound(v.begin(), v.end(), data,
                                [](const D* a, const D* b) {
                                    return a->get_weight() > b->get_weight();
                                });

        std::cout << "Comparison done..." << std::endl;
        std::cout << data->get_weight() << std::endl;
        // Insert the pointer into the vector
        v.insert(it, data);

        std::cout << "Inserted complex tree: " << data->get_weight() << std::endl;
    }
idle isle
#

if you run it with address sanitizer (-fsanitize=address), you should get a stacktrace for when that happens

#

or you could look at the code, because you shouldn't be calling free, delete or anything like that at all here

swift abyss
#

I'm calling it at the destructor level

#

buuuuut

#

What's happening is that it's calling the destructor multiple times

#

on one single insertion

#

the vector might be getting reallocated as you said

#

even though I did give a bigger space just to try

idle isle
#

what does the destructor look like?

#

I don't think you need one at all to be honest

#

because a node does not manage the lifetime of its children

swift abyss
#
            ~WeightedBinaryTree() {
                std::cout << "Weight:" << get_weight() << " Complex: " << is_complex() << std::endl;
                if (is_complex()){
                    std::cout << "Weight:" << get_weight() << " Complex: " << is_complex() << std::endl;
                    auto& [left, right] = std::get<std::pair<WeightedBinaryTree<W, D>*, WeightedBinaryTree<W, D>*>>(m_content);
                    delete left;
                    delete right;
                } 
            } 
idle isle
#

yeah don't delete anything

#

just remove the entire destructor

swift abyss
#

the stacktrace looks like this:

d,1
Weight:1 Complex: 0
Into insert_in_isorted...
Comparison done...
1
Inserted complex tree: 1
b,2
Weight:2 Complex: 0
Into insert_in_isorted...
Comparison done...
2
Inserted complex tree: 2
c,2
Weight:2 Complex: 0
Into insert_in_isorted...
Comparison done...
2
Inserted complex tree: 2
a,5
Weight:5 Complex: 0
Into insert_in_isorted...
Comparison done...
5
Inserted complex tree: 5
4
5 2 2 1 
Tree size : 4

Weight:3 Complex: 1
Weight:3 Complex: 1
Weight:1 Complex: 0
Weight:2 Complex: 0
free(): invalid pointer
Abandon (core dumped)
idle isle
#

that's not a stacktrace

#

but yeah, don't call delete

swift abyss
#

that works like this

idle isle
#

just remove the destructor, the assignment operators, and the copy/move ctors

swift abyss
idle isle
#

they should all work automatically

swift abyss
#

Done

#

It almost works but at this point

#

Isn't using the other solution the same as using this one

#

I'm leaving pointers allocated

#

They won't be disallocating all by themselves

idle isle
#

yeah but you're not actually leaking them

#

because the nodes are stored in this other vector

#

which will destroy them when it is itself destroyed

#

and that happens when it goes out of scope, because it's a local variable

swift abyss
#

Ah well that's good

#

The final issue I want to address

#

Thank you first

#

But the final thing I'm struggling with

#

if I could show it

swift abyss
#
void add_dummy(){
                if (!is_complex()) {
                        throw std::logic_error("Cannot add dummy to a leaf node");
                    }

                auto& [left, right] = std::get<std::pair<WeightedBinaryTree<W, D>*, WeightedBinaryTree<W, D>*>>(m_content);

                WeightedBinaryTree<W, D>* new_dummy = new WeightedBinaryTree<W, D>(
                        right->get_weight(),  
                        std::make_pair(right, nullptr) 
                    );

                right = new_dummy;
            }
#

this function adds a dummy node

#

but it always makes a segmentation fault

#

That's basically my last issue

#

the definition of a dummy node is this:

swift abyss
idle isle
#

ah

#

don't use new

#

new and delete always come together: new creates something on the heap, and delete destroys it and gives back the memory

#

but you're storing nodes in this vector, so that's where you would need to create the new node

swift abyss
#

but if I can't affect it

#

how can I use it

#

the fact that it does a segmentation fault breaks my brain though

swift abyss
#

how am I going to use that method :/

idle isle
#

hmm that's a problem

swift abyss
#

I mean that's what the professor asks for

#

I only have access to the current object through this

#

but why does it not work though

#

like htat

#

that*

#

what's the magic that makes it not have the ability to change the value of the right pointer

idle isle
#

this shouldn't segfault, it should instead leak

#

which is also bad

#

so I guess you were actually meant to store the nodes individually on the heap

#

instead of in a vector

#

so sorry about that

#

it's not like storing them in a vector doesn't work, but it's not what your professor had in mind

#

the good news is that this is easy to fix

#

instead of using push_back to store the nodes in a vector, you're using new, and you're still storing the pointers in a vector

#

and then you need to re-add the destructor, copy / move constructor and assignment operator

#

(alternatively, if you want to continue with the current solution, you could also store a pointer to the vector of nodes in each node)

swift abyss
#

I don't get the new idea 🥲

#

so instead of using a vector of pointers

#

I'd store all the pointers using new into 1 single vector?

#

so I'd have a vector of allocated pointers that I would work on?

swift abyss
idle isle
#

nope, new creates an object on the heap and gives you a pointer to it

#

I have to go now but I'll be back in like 30 mins, then I'll explain

swift abyss
#

I'd have one vector

#

with all pointers

#

that'd make it quite logical

#

BUT

#

The constructor is private

#

atleast that's what he wanted the behavior to be

#

Then I guess the leaf and the complex nodes

#

should be pointers

#

returned using new

idle isle
#

that's why I initially thought you weren't supposed to write it like that

#

hmm yeah this is pretty weird, I'm not sure what's the intended solution

#

but didn't you say that you were allowed to change this?

#

if you can, then yeah just make leaf() and complex() return either C pointers or std::unique_ptrs

#

(std::unique_ptr would be the better interface, but I don't know if you're supposed to use that)

swift abyss
#

I'll change the return type to pointers

idle isle
#

I feel like what I'm struggling most here is trying to figure out what you're meant to do

#

because I can think of multiple ways to implement the huffman tree, but the instructions don't really fit with any of them

#

anyway, sorry for kind of going back and forth on the heap allocation

swift abyss
#

I guess I do with heaps

#

and return pointers

#

but still

swift abyss
#

So I'd return a pointer from the leaf and

#

complex

#

and it would be easier right?

idle isle
#

yeah

#

in that case, you need to make sure you delete the left and right pointers of a complex node in the destructor

swift abyss
#

alright I'll try that now

#

the only thing is that all of the signatures would have to change

swift abyss
idle isle
#

really?

#

what other methods would change, apart from leaf() and complex()?

swift abyss
#

the signature of the insert_in_isorted

#

would have to take a vector of pointers

#

But it seems logical to me that it returns pointers

#

Like really logical

idle isle
#

it takes a vector of D and returns a D, right? Or maybe T if you've changed the name

#

so that type could be pointer, there's no need to change the signature

swift abyss
#

yes D* it has to be

swift abyss
#
    template <typename D>
    void insert_in_isorted(std::vector<D>& v, const D data){
        std::cout << "Into insert_in_isorted..." << std::endl;
        auto it = std::lower_bound(v.begin(), v.end(), data, 
                                [](const D& a, const D& b) {
                                    return a.get_weight() > b.get_weight();
                                });
        std::cout << "Comparaison done..." << std::endl;
        v.insert(it, data);
        std::cout << "Inserted complex tree : " << data.get_weight() << std::endl;
    }   
#

I can't access to D& with . I need ->

idle isle
#

ah I see

#

you could just write a->get_weight() instead of a.get_weight()

#

but might as well change it to D*, if that's not a problem for the task

swift abyss
#

It works for the first part

#

still a segmentation error for add dummy

#

Big problem

swift abyss
idle isle
#

the pointers themselves don't matter

#

what matter is if the objects they point to get destroyed

#

and that only happens when you use delete

#

so if a destructor leads to a segfault, that probably means you're not copying correctly

#

what does your copy constructor look like?

swift abyss
#

I didn't make one I think

idle isle
#

ah that's a problem then

#

you don't need a copy constructor

swift abyss
#

rule of 3?

idle isle
#

but you need a move constructor

#

kind of

#

except you want moves, but not copies

#

and the move constructor and move assignment operator has to set the pointers of the moved-from object to nullptr

#

because otherwise they would point to the same object as the newly created node would, and that means that both destructors would try to delete the same object

#

which can cause a segfault

swift abyss
#

it works

#

but the add_dummy breaks the app

#

once again

idle isle
swift abyss
#
            void add_dummy(){
                if (!is_complex()) {
                        throw std::logic_error("Cannot add dummy to a leaf node");
                    }

                auto& [left, right] = std::get<std::pair<WeightedBinaryTree<W, D>*, WeightedBinaryTree<W, D>*>>(m_content);

                WeightedBinaryTree<W, D>* new_dummy = WeightedBinaryTree<W, D>::complex(
                    right, nullptr);

                right = new_dummy;
            }
swift abyss
#

it still breaks

idle isle
#

hmm

#

can you show more code? Like the destructor and move constructor

swift abyss
#
WeightedBinaryTree(const WeightedBinaryTree& other)
                : m_weight(other.m_weight), m_content(copy_content(other.m_content)) {}

            WeightedBinaryTree& operator=(const WeightedBinaryTree& other) {
                if (this != &other) {  
                    clear();

                    m_weight = other.m_weight;
                    m_content = copy_content(other.m_content);
                }
                return *this;
            }

            void clear() {
                if (std::holds_alternative<std::pair<WeightedBinaryTree<W, D>*, WeightedBinaryTree<W, D>*>>(m_content)) {
                    auto& [left, right] = std::get<std::pair<WeightedBinaryTree<W, D>*, WeightedBinaryTree<W, D>*>>(m_content);
                    delete left;
                    delete right;
                }
            }

            static WeightedBinaryTree<W,D>* leaf(W weight, D data){
                return new WeightedBinaryTree<W,D>(weight,data);
            }
swift abyss
#

I did a copy constructor

idle isle
#

but that's not correct

#

you're calling clear before calling copy_content

#

so at that point you're trying to copy children that have already been destroyed

swift abyss
#

true

swift abyss
#

but yet still

#

the problem occurs

#
            void add_dummy(){
                if (!is_complex()) {
                        throw std::logic_error("Cannot add dummy to a leaf node");
                    }

                auto& [left, right] = std::get<std::pair<WeightedBinaryTree<W, D>*, WeightedBinaryTree<W, D>*>>(m_content);

                WeightedBinaryTree<W, D>* new_dummy = WeightedBinaryTree<W, D>::complex(
                    right, nullptr); // THE PROBLEM OCCURS HERE WHEN I CREATE THIS


            }
#

As I try to create the new_dummy pointer

#

it does a segmentation fault

idle isle
#

that doesn't have to be where the actual bug is

#

because this looks correct to me

swift abyss
#

I mean if I remove that line

#

it works

#

I mean the segmentation fault doesn't happen

#

which is weird

#

I just created a pointer that is allocated dynamically ^^

idle isle
#

what does complex() look like?

#

and what does the copy assignment operator look like now?

#

and what does copy_content look like?

swift abyss
#
            static WeightedBinaryTree<W, D>* complex(WeightedBinaryTree<W, D>* left,WeightedBinaryTree<W, D>* right) {
      W weight = left->get_weight() + right->get_weight();
       return new WeightedBinaryTree<W,D>(weight, std::make_pair(left, right));
            }
copy_content(const std::variant<D, std::pair<WeightedBinaryTree<W, D>*, WeightedBinaryTree<W, D>*>>& content) {
                if (std::holds_alternative<D>(content)) {
                    return std::get<D>(content); 
                } else {
                    auto& [left, right] = std::get<std::pair<WeightedBinaryTree<W, D>*, WeightedBinaryTree<W, D>*>>(content);
                    return std::make_pair(new WeightedBinaryTree(*left), new WeightedBinaryTree(*right));
                }
            }
#

I got it

#

.......

idle isle
#

you found the bug?

swift abyss
#
W weight = left->get_weight() + right->get_weight();
#

passing nullptr and calling a method on it

#

rookie mistake

#

nullptr->get_weight() would ofc defaults to segmentation fault

idle isle
#

ah yeah

#

by the way

swift abyss
#

That's about it I think

swift abyss
idle isle
#

to debug things like this, you should use a debugger and step though it

swift abyss
#

Yes true

#

but I didn't know how to set it up for vscode

idle isle
#

and you can use address sanitizer, that would have also told you what happened

swift abyss
#

i added the -parameter=val

#

like the one you wrote before

#

it didn't show much

idle isle
swift abyss
#

what do you use

idle isle
#

clion

swift abyss
#

or what do you recommend

idle isle
#

vscode should be fine

swift abyss
#

WOOOOOOOOOOOOOOOOOOOOOOAT

#

JETBRAINS HAS A CPP IDE

idle isle
#

yeah, but unfortunately it's not free for everyone

swift abyss
#

Well, I'm a jetbrains fanboy

#

...

#

yeah, I have student membership

#

so all good for all jetbrain stuff

idle isle
#

yeah then it's free for you

idle isle
#

and then it should print a stacktrace when it segfault

swift abyss
#

I did that weirdly enough

#

It didn't print anything

idle isle
#

hmm maybe you need -fno-omit-frame-pointer

swift abyss
#

I'll try that

#

time to go to bed though

#

thank you for your help !

idle isle
#

npnp

swift abyss
#

Still a problem that I'm unable to figure out, is it okay if I show it to you?

#

I mean I thought I solved it but I guess I didn't

idle isle
#

it's for the same exercise?

swift abyss
#

yes

idle isle
#

ok yeah

swift abyss
#
            
            void add_dummy(){
                if (!is_complex()) {
                        throw std::logic_error("Cannot add dummy to a leaf node");
                    }

                auto& [left, right] = std::get<std::pair<WeightedBinaryTree<W, D>*, WeightedBinaryTree<W, D>*>>(m_content);

                WeightedBinaryTree<W, D>* new_dummy = WeightedBinaryTree<W, D>::complex(
                    right, nullptr);
                right = new_dummy; // HERE ( segmentation fault )
            }
#

this keep generating an error

#

segmentation fault

#

on the //HERE line

#

I was wondering if that was because of this

#
auto&/*Reference??*/ [left, right] = std::get<std::pair<WeightedBinaryTree<W, D>*, WeightedBinaryTree<W, D>*>>(m_content);
#

Since those are pointers I don't need to get the reference right?

idle isle
#

maybe

#

you could use a debugger to find out

#

by the way, don't compile with optimizations in debug mode

#

but yeah you don't need the reference, just remove it

swift abyss
#

Is it mandatory to have the copy constructor and the = operator?

idle isle
#

so you do need the reference, otherwise that's not actually changing the member

idle isle
#

I think there's a wheatly command that shows a table of what the compiler generates in what case, but I don't remember what it's called

#

but anyway, you shold follow the rule of 0/3/5

swift abyss
#

they should be public?

idle isle
#

I think in your case you don't need to copy trees at all

#

so you can just =delete them

swift abyss
#

it generates errors if I remove them

swift abyss
idle isle
#

but only on pointers, right?

#

I don't see where you're actually copying the tree

swift abyss
#

it's only on pointers

idle isle
#

that doesn't require the tree itself to be copyable

swift abyss
#

if I remove the copy constructor

#

well it breaks

idle isle
#

where?

swift abyss
#

= operator doesn't matter

idle isle
#

it should show you the line where you're trying to copy it

swift abyss
#

it breaks, I mean by that

#

segmentation fault :/

idle isle
#

hmm what

#

if you write = delete on the copy constructor, you get a segfault, but without that it works?

swift abyss
#

no no

#

if I just remove the copy constructor I've written

#

it breaks

idle isle
#

yeah of course that breaks

#

that's a violation of the rule of 3

swift abyss
#
WeightedBinaryTree(const WeightedBinaryTree& other)
  : m_weight(other.m_weight), m_content(copy_content(other.m_content)) {}
#

sorry for the indentiation

idle isle
#

yeah, so what the compiler-generated copy ctor does is to copy all the members

#

but your members are pointers

#

and copying pointers won't copy the thing they point to, just the pointers themselves

swift abyss
#

sooo it breaks

idle isle
#

which means you end up with two different pointers pointing to the same thing, and then you try to delete that thing twice

#

one way to fix this is by using std::unique_ptr instead of a C pointer

swift abyss
#

I'll keep it this way ig

idle isle
#

then you should write WeightedBinaryTree(const WeightedBinaryTree& other) = delete;

#

because I don't think you ever need to copy trees

swift abyss
#

I get you

idle isle
#

if it breaks right now, that means you are actually copying trees

swift abyss
#

I need to run this manually right to understand the subject

#

like get a piece of paper

#

to see what's happening

#

for the trees and the huffman tree

#

because as of now

#

there is something weird happening

#

my add_dummy doesn't change the huffman code

#

which is pretty crazy

#

like if you had for example

#

0
|
1 0

#

the add dummy would replace the 0 subtree

idle isle
#

yeah

#

so first things first

idle isle
#

which should mention a line where you're trying to copy a tree

#

can you show me where that happens?

swift abyss
#

or my issue with logic of the problem?

idle isle
swift abyss
#

let me try

#
error: use of deleted function ‘utils::WeightedBinaryTree<W, D>::WeightedBinaryTree(const utils::WeightedBinaryTree<W, D>&) [with W = long unsigned int; D = char]’
   78 |         auto root = *trees.front();
#

I guess it doesn't since I'm trying to put into a function in my code

#

the idea would probably be to use the pointer

idle isle
#

yeah, you don't want to copy the tree itself

#

where is this line?

swift abyss
#

it's here

#
 auto root = *trees.front();
        std::cout << "weight:" << root.get_weight() << std::endl;
        if (add_dummy) {
            root.add_dummy();
        }
        std::cout << "ERROR" << std::endl;
        std::unordered_map<D, std::vector<bool>> encoding_map;
        std::vector<bool> path;
        auto map_encoding = [&](auto& self, const utils::WeightedBinaryTree<std::size_t, D>& node, std::vector<bool> path) -> void {
            if (node.is_leaf()) {
                
                encoding_map[node.get_data()] = path;
            } else {
                        std::cout << node.get_weight() << std::endl;
                path.push_back(false);
                self(self, node.get_left(), path);
                path.back() = true;
                self(self, node.get_right(), path);
            }
#

after merging the trees into one tree

idle isle
#

but you don't need to copy the tree there, right?

#

root should just be a pointer

swift abyss
#

I'll change it

#

now it's good

#

but it's just the logic

#

that's entirely wrong

#

may I show you a video?

#

the output is sadly incorrect

idle isle
#

ok

#

but really the best way forward is to learn how to debug these things

#

I would try to create a simple testcase that fails

#

and then try to understand why it fails

#

the debugger is very helpful for that

swift abyss
#

I want to do that

#

but I don't know

#

how to use a debugger

#

on vs code

#

you probably know it

#

I tested on that case

#

it gave me something completely different

#

0010111100110

#

my output

#

I'll basically run the code on paper

#

just to see what's happening

idle isle
#

maybe try a simpler example first

swift abyss
#

that one is easy

#

it's about 6 characters

#

so the tree isn't huge

#

I want to write some unit tests

#

to deal with it

#

but

#

I'm not super familiar with that

#

coming from a js background 😂

#

and cpp seems to be really complex but rich too

idle isle
#

you should ideally use a framwork like googletest

#

but if that's too much to set up now, just writing a function that uses assert is good enough for now

#

start with really simple things, like a tree that consists of only 1 or 2 nodes

#

and assert that it looks like what you'd expect

swift abyss
#

a tree with 1 node

#

would be giving 1

idle isle
#

yeah, that's a very simple testcase

swift abyss
#

terminate called after throwing an instance of 'std::logic_error'
what(): Cannot add dummy to a leaf node

#

woops

idle isle
#

you want testcases to be as simple as possible so that they're easy to debug

#

well that's good

#

it means you've found a bug

#

which is the point of testing

swift abyss
#

yeah I have to do those cases true

swift abyss
#

it seems weird

#

it's doing nothing

idle isle
#

what does the code look like again?

swift abyss
#
            void add_dummy(){
                if (!is_complex()) {
                        throw std::logic_error("Cannot add dummy to a leaf node");
                    }

                auto [left, right] = std::get<std::pair<WeightedBinaryTree<W, D>*, WeightedBinaryTree<W, D>*>>(m_content);
                WeightedBinaryTree<W, D>* new_dummy = WeightedBinaryTree<W, D>::complex(
                    right, nullptr);
                right = new_dummy;
                go_dfs(this);
                std::cout << std::endl;
            }
#

godfs is just a method I made

#

to check the content of the tree

#

Before adding:
7-3-4-2-2-1-1
After adding:
7-3-4-2-2-1-1

swift abyss
#

can I assign it to a tree using the definition of the tree itself?

swift abyss
idle isle
#

ah sorry I didn't read this

swift abyss
#

even if I try to assign something constant

#

the dfs method keeps displaying the same tree

idle isle
#

yeah because you don't have the reference

swift abyss
#

I'm applying on a this object no?

idle isle
#

sorry, I later corrected myself but I guess you didn't see that

#

you need to use auto& [left, right]

swift abyss
#
   
  if (add_dummy) {
            root->add_dummy();
        }
idle isle
#

because otherwise right is a copy of the pointer

idle isle
swift abyss
#

from my huffman class

#

it does copy pointers

#

what

#

I thought it takes the reference

#

directly

swift abyss
idle isle
#

when you call a function and don't pass an argument by reference, it gets copied

#

when you pass a pointer, that pointer gets copied

#

which means that the new copy of this pointer will point to the same object as the original pointer

swift abyss
#

that's so abnormal

#

so changing the second pointer reference would never matter ig

idle isle
#

exactly

swift abyss
#

since the root would always keep pointing over it

#

But

#

Then

idle isle
#

right = new_dummy doen't matter

swift abyss
#

if I do what you said

#

it basically does a segmentation faul

#

t

idle isle
#

show the code pls

swift abyss
#
            void add_dummy(){
                if (!is_complex()) {
                        throw std::logic_error("Cannot add dummy to a leaf node");
                    }

                auto& right = std::get<std::pair<WeightedBinaryTree<W, D>*, WeightedBinaryTree<W, D>*>>(m_content).second;
                WeightedBinaryTree<W, D>* new_dummy = WeightedBinaryTree<W, D>::complex(
                    right,nullptr);
                right = new_dummy;
                go_dfs(this);
            }
#

on the right = new_dummy

#

the thing that I'm feeling is weird

#

I'm defining new_dummy through right

#

and right through new_dummy

swift abyss
# idle isle show the code pls
WeightedBinaryTree<W, D>* new_dummy = WeightedBinaryTree<W, D>::complex(
                    right,nullptr);
                right = new_dummy;

These lines could break right ?

idle isle
#

wait that doesn't compile

#

it should be auto& [right, left] = ..., no?

swift abyss
#

But I only got the second

idle isle
#

or [left, right], rather

swift abyss
#
auto& right = std::get
<std::pair<WeightedBinaryTree<W, D>*,
 WeightedBinaryTree<W, D>*>
>
(m_content).second // SECOND ONLY;
idle isle
#

oh wait I didn't see the .second, my bad

#

right

swift abyss
#

I thought about making a copy of right

#

using it inside the dummy

#

and then changing the right with the dummy

idle isle
#

but then you assign to the reference right, which means you assign to the member

#

you can add an assertion for that if you're unsure

swift abyss
#
            static WeightedBinaryTree<W, D>* complex(WeightedBinaryTree<W, D>* left,WeightedBinaryTree<W, D>* right) {
                W weight = 0;
                if(left && !right)
                    weight = left->get_weight();
                else if (right && !left)
                    weight = right->get_weight();
                else if (right && left)
                    weight = left->get_weight() + right->get_weight();
                return new WeightedBinaryTree<W,D>(weight, std::make_pair(left, right));
            }
idle isle
#

like, add assert(std::get<std::pair<WeightedBinaryTree<W, D>*, WeightedBinaryTree<W, D>*>>(m_content).second == new_dummy) before go_dfs

idle isle
#

so you're copying two pointers

swift abyss
#

so it creates a new object

idle isle
#

a new pointer

#

and then in the last line with the new you actually create a new tree object

swift abyss
#

that makes no sense that it's breaking then

#
utils.hpp:104:17: error: ‘assert’ was not declared in this scope
  104 |                 assert(std::get<std::pair<WeightedBinaryTree<W, D>*, WeightedBinaryTree<W, D>*>>(m_content).second== new_dummy && "OFF");
#

getting this

#

even though I declared it

#

macro "assert" passed 4 arguments, but takes just 1

swift abyss
idle isle
#

you need to #include <cassert> for assert

swift abyss
#

I did

idle isle
#

and also, assert is only active in debug mode

#

it won't do anything of NDEBUG is defined

#

which e.g. cmake defines by default in release mode

#

ah wait the error comes from the fact that assert is a macro

#

it doesn' understand <> as parantheses

#

so you have to put everything in an extra ()

#

assert((std::get<std::pair<WeightedBinaryTree<W, D>*, WeightedBinaryTree<W, D>*>>(m_content).second) == new_dummy

swift abyss
#

segmentation fault

#

even before the fact that it reaches the assert

#

t

swift abyss
idle isle
#

where does it segfault?

#

does address sanitizer give you a stacktrace now?

swift abyss
#

can you give me the param I have to add to the execution?

#

Forgot it

idle isle
#

add -fsanitize=address,undefined to compiler and linker options, and also -O0 to compiler options

#

maybe you have to add -fno-omit-frame-pointer too, although I think that should already be implied by -O0

#

oh and -g as well of course

#

so that's already much better

#

but it's still not printing line numbers

#

try again with -g if you didn't

swift abyss
#

utils.hpp:88:42: runtime error: member call on null pointer of type 'const struct WeightedBinaryTree'
utils.hpp:65:30: runtime error: member access within null pointer of type 'const struct WeightedBinaryTree'
AddressSanitizer:DEADLYSIGNAL
=================================================================
==3926946==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x55f00f674b73 bp 0x7ffd9617ed40 sp 0x7ffd9617ed30 T0)
==3926946==The signal is caused by a READ memory access.
==3926946==Hint: address points to the zero page.
    #0 0x55f00f674b73 in utils::WeightedBinaryTree<unsigned long, char>::get_weight() const /src/utils.hpp:65
    #1 0x55f00f67ba94 in utils::WeightedBinaryTree<unsigned long, char>::go_dfs(utils::WeightedBinaryTree<unsigned long, char> const*) /src/utils.hpp:88
    #2 0x55f00f67bb87 in utils::WeightedBinaryTree<unsigned long, char>::go_dfs(utils::WeightedBinaryTree<unsigned long, char> const*) /src/utils.hpp:92
    #3 0x55f00f67bb87 in utils::WeightedBinaryTree<unsigned long, char>::go_dfs(utils::WeightedBinaryTree<unsigned long, char> const*) /src/utils.hpp:92
    #4 0x55f00f676085 in utils::WeightedBinaryTree<unsigned long, char>::add_dummy() /src/utils.hpp:107
    #5 0x55f00f670357 in encoding::lossless::Huffman<char>::encode(std::vector<char, std::allocator<char> > const&, bool) /src/encoding_lossless.hpp:89
    #6 0x55f00f66996c in main /src/test_2.cpp:10
    #7 0x7f5c62246249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #8 0x7f5c62246304 in __libc_start_main_impl ../csu/libc-start.c:360
#
    #9 0x55f00f6694a0 in _start (/src/a.out+0x424a0)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /src/utils.hpp:65 in utils::WeightedBinaryTree<unsigned long, char>::get_weight() const
==3926946==ABORTING
#

it's probably because of the fact that the getter

#

is

swift abyss
idle isle
#

right

swift abyss
#

holy moly It's terrible I think

idle isle
#

you need to fix go_dfs

swift abyss
#

I removed it

#

yet another problem

idle isle
#

what was the description of the task that told you to add the dummy again?

#

did it say that the other pointer should be nullptr?

swift abyss
#

it told me to change the right subtree

#

into a new subtree that has it as it's left

#

and having the right

#

as NULL

idle isle
#

right

#

so that means all your functions have to deal with the fact that children can be null

#

you probably don't need to change that many functions

swift abyss
#

problem is

#

the complex nodes

#

I don't do a nullity check

idle isle
#

yeah, now you need to do that

swift abyss
#

I only iterate

#

ah well...

#

I have to define ==?

idle isle
#

no

#

you're only comparing pointers

#

I think maybe you should read about pointers a bit more

swift abyss
#

I'll do so

idle isle
#

I didn't actually read it right now but the info there is generally good

swift abyss
#

Still not to great at this

#

haven't read too many docs either

idle isle
#

well pointers are confusing to most people learning C and C++

#

but it's a very important concept to understand

swift abyss
#

true true

#

I understand it