thread 5628 panic: switch on corrupt value

[cerulean schooner]

Is it not possible to switch in unions?

const std = @import("std");
const pow = std.math.pow;
const parseFloat = std.fmt.parseFloat;
const Expression = @import("./types/Expression.zig");
const AstNode = Expression.AstNode;
const AstNodeKind = Expression.AstNodeKind;
const toOperator = Expression.toValue;


pub fn evaluate(node: *AstNode) f64 {
    switch (node.*) {
        AstNodeKind.value => return node.*.value,
        AstNodeKind.binaryOperation => {
            
            const left = evaluate(node.*.binaryOperation.left);
            const right = evaluate(node.*.binaryOperation.right);

            switch (node.binaryOperation.operator) {
                .@"+" => return left + right,
                .@"-" => return left - right,
                .@"*" => return left * right,
                .@"/" => return left / right,
                .@"**" => return pow(f64, left, right),
            }
        }
    }
}

[pure dew]

you can absolutely switch on unions
the error implies that node doesnt point to a valid AstNode value

make sure you arent doing anything weird
it could be a memory management error or a ptrCast of null pointer
so look for those

[cerulean schooner]

I could be Memory management error indeed

when try to print the AST gives an error

but I dont know what it means

[pure dew]

I assume you know the basics right?
if you return anything by reference (including stuff like slices), you need to allocate that memory in the function
that’s the thing people usually get wrong and would absolutely lead to issues like this

[cerulean schooner]

I'm learning

not sure if I got the basic right tbh

[pure dew]

can you send the code witha. zigbin.io url? (it’s like a pastebin)
I can look at it in a bit just to make sure theres nothing obvious but I dont have my computer to test stuff right now

[cerulean schooner]

my first zig project and before it I only did JS and TS

[cerulean schooner]

How can I do this?

I have it in github if it works for you

[pure dew]

yeah github works too

though zigbin is really useful, id recommend looking at it
you can just paste your code in there and share it with people if it’s a single file

[cerulean schooner]

oh I just found the zigbin.io

wow

it's live right?

[pure dew]

wdym?

[cerulean schooner]

like If I paste it there you can see it in realtime

[pure dew]

maybe? not sure actually
ive always just used it for one time pastes

I guess you can test it lol

[cerulean schooner]

https://zigbin.io/6b90f8

[pure dew]

id need to see the code that calls that though
if it’s all split into multiple files, github would be easier

[cerulean schooner]

https://github.com/coding-agent/cli-calculator-zig

[pure dew]

thanks

[cerulean schooner]

I thought it could be the case

but still very useful

Sorry if the code is messy, I still trying to learn how to make things clearer

[pure dew]

So multiple issues
Left_node and right_node are invalidated after that scope, so anything pointing to them will be pointing to garbage, you'll need to allocate those on the heap if you want to keep this pattern

Also, since the function that builds the AST will sometimes return the very AST it was passed, the default AST has to be a valid value
So setting it to undefined is VERY bad

Also, don't implicitly use std.mem.page_allocator for everything
If your function needs to allocate memory, then have the caller pass a std.mem.Allocator to the function

[cerulean schooner]

understood

the reason I used the ast as undefined was to declare it and then I could keep adding stuff to the same variable and the return it at the end

but as you explained then was the ast is pointing to get invalidated so I need to allocate it, which Im not sure how to do it

that might also be the reason I get a segfault when I try to print the whole ast

I understood now, but how to I allocate it in memory?

I tried before to add it to the ast instead of referencing it but then it would be adding something the same size of AstNode to itself which is basically infinite size. So how can I reference to a slice of memory that is actually not garbage at the end of the scope?

and can I do it locally(inside the function)?

[pure dew]

you would do allocator.create(AstNode)
Which returns a pointer to an uninitialized AstNode, and then yoj can fill jt in

@cerulean schooner

[cerulean schooner]

hmm ok

Thank you

[cerulean schooner]

struggling to creat the allocator

src\parser.zig:29:49: error: expected type 'mem.Allocator', found 'type'

[pure dew]

You'll probably want to use a GeneralPurposeAllocator so I'll show you how to initialize that

pub fn main() !void {
  var gpa = std.heap.GeneralPurposeAllocator(.{}){};
  defer _ = gpa.deinit();
  const allocator = gpa.allocator();
}

allocator is what you pass to other functions
And those functions should ask for a std.mem.Allocator

[cerulean schooner]

Oh ok

so the allocator.create is not the allocator itself but telling the allocator the type of memory to allocate right?

[pure dew]

Yeah, allocator.create() is a method that allocates a type

same as allocator.alloc() but for a single item, not a slice

You should look at the std docs

[cerulean schooner]

okok will have a look again

now I understand it better thank you

error: access of union field 'Pointer' while field 'Type' is active
    const Slice = @typeInfo(@TypeOf(memory)).Pointer;
                  ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~

[pure dew]

use allocator.destroy() instead of allocator.free()
I think that's the issue

Hard to tell from just that

[cerulean schooner]

destry would require to pass the ptr as paramenter but Im passing many ptr that would only be freed in other part of the code

[cerulean schooner]

tried adding it to the evaluate part of the code ```c++
const left = evaluate(node..binaryOperation.left, allocator);
const right = evaluate(node..binaryOperation.right, allocator);
defer allocator.destroy(left);
defer allocator.destroy(right);

but still the same error

[pure dew]

yeah
Unfortunately it's structured really weirdly so it's not very easy
You'll need to deinit the pointers in the binaryOperators themselves

There's probably a way around this, or you could just leak since there's no infinite loop in your program
Or you could use an arena for similar reasons

But the best way to go about it is restructuring everything

Ill try writing my own mini calculator thing when I get home, it's hard for me to imagine how to structure it so I'll just tell you if i make it work lol

[cerulean schooner]

I heard about arena. I think its a good option but I dont know how to use it for sure

Thanks mate!

[silver moat]

I don't think that's right; evaluate returns a f64 - so there's no reason to free it, right?

For tree-like structures, yes, an arena can be useful.
Especially if you're going to construct the whole tree; maybe adjust a couple of nodes; and then only care about the result at the end, and all the nodes can just go away at that point.
(If you need to keep one or two around or whatever, you can always duplicate just those at the end.)
Arenas are great for when you want to just allocate a bunch, and then free everything in one fell swoop in a quick and cheap way.

Even if you're reusing a bunch of stuff, you can still allocate the things inside an arena, and then use a freelist (list of 'freed' things).
That way, when you would free something, you instead just push it onto the freelist.
And attempt to pop one off instead of allocating again.
This way, you can still free all the things with arena.deinit(), but yet can "alloc/free" a bunch, without making it harder to free them in the end. 😄

[cerulean schooner]

Using arena and correcting this actually solved most other issues before but came back to the initial problem ```c
thread 15752 panic: switch on corrupt value

[cerulean schooner]

I think I was doing a very stupid thing

I changed this tagged union from this: ```c
pub const AstNode = union (AstNodeKind) {
value: f64,
binaryOperation: BinaryOperation,
};

pub const AstNodeKind = enum(u16) {
value,
binaryOperation,
};
to this:c
pub const AstNode = union (AstNodeKind) {
value: f64,
binaryOperation: BinaryOperation,
};

pub const AstNodeKind = enum {
value,
binaryOperation,
};

which now moves to other error: ```c
thread 1740 panic: access of union field 'binaryOperation' while field 'value' is active
cli-calculator-zig\src\evaluate.zig:15:39: 0x7ff7e5054fc9 in evaluate (calculator.exe.obj)
const left = evaluate(node.binaryOperation.left.*, allocator);

But the switch only select this in the case its active right? ```c
switch (node) {
.value => return node.value,
.binaryOperation => {
const left = evaluate(node.binaryOperation.left., allocator);
const right = evaluate(node.binaryOperation.right., allocator);

        switch (node.binaryOperation.operator) {
            .@"+" => return left + right,
            .@"-" => return left - right,
            .@"*" => return left * right,
            .@"/" => return left / right,
            .@"**" => return pow(f64, left, right),
        }
    }
}

[vale kelp]

you should capture the value instead of accessing the fields like that

iirc it's like
.value => |value| return value,
same thing for the binary operation one

[cerulean schooner]

thank you

ok I think now we got to the bottom of the issue

Segmentation fault at address 0xffffffffffffffff

I have been having so many of these that I deleted my code more than any mini project I made before

🫠

I printed the ast and got this

types.Expression.AstNode{ .value =  }Segmentation fault at address 0xffffffffffffffff

I should have tested the code before

I think I didnt get the concept of the arena allocator properly

[cerulean schooner]

Is it possible to allocate a union (as it is not a single type)?

[pure dew]

a union is still a type, just allocate it like anything else

[cerulean schooner]

the problem is when I do it says ```c
access of union field 'value' while field 'value' is active

[pure dew]

weird

are you sure it was defined and not left uninitialized

[cerulean schooner]

Im not sure tbh

I made the create which return the pointer to undefined and after that I try to assign the values

which causes this error

I think I might be messing up something but I cant figure it out

oh. maybe because I'm using the same allocator in a loop with the create?

[pure dew]

make sure youre doing it like this
foo = .{.value = bar}

[cerulean schooner]

let me try

src\parser.zig:31:26: error: type '*types.Expression.AstNode' does not support struct initialization syntax

ah wait ```c
foo.* = .{.value = bar}

still getting the segfault in the evaluate

I will review all over again the allocations

zig build run --summary failures -- "(2.5+1)-2"
types.Expression.AstNode{ .binaryOperation = types.Expression.BinaryOperation{ .operator = types.Expression.Operator.+, .left = types.Expression.AstNode{ .value = 2.5e+00 }, .right = types.Expression.AstNode{ .value = 2.5e+00 } } }Segmentation fault at address 0xffffffffffffffff 

so the "2.5+1" gets stored and then the last value get segfault

IT WORKED

I'm not sure what I did and the result is wrong but it worked

result: 5

zig build run --summary failures -- "1+1"      
result: 2
zig build run --summary failures -- "1+1+3"
result: 2
zig build run --summary failures -- "1+4+3"
result: 2
zig build run --summary failures -- "5+4" 
result: 10
zig build run --summary failures -- "5/2"
result: 1

[cerulean schooner]

Resolved

.\zig-out\bin\calculator.exe (2/2.56842+3)
Result: 3.77868884372494

.\zig-out\bin\calculator.exe 2.97*5.7+.1
Result: 17.029000000000003

.\zig-out\bin\calculator.exe (1+11+1)   
Result: 13