Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected behavior when unwrapping optional #16690

Closed
geefuoco opened this issue Aug 5, 2023 · 1 comment
Closed

Unexpected behavior when unwrapping optional #16690

geefuoco opened this issue Aug 5, 2023 · 1 comment
Labels
question No questions on the issue tracker, please.

Comments

@geefuoco
Copy link

geefuoco commented Aug 5, 2023

Zig Version

0.11.0-dev.4217+e8fa19960

Steps to Reproduce and Observed Behavior

Optionals have a weird behavior when unwrapping. Unwrapping with an if statement doesn't work as expected, unless you manually unwrap with the .? operator in the same block.

The example is based off of the append method.

Currently the debug prints this:

Test [1/1] test.LinkedList... Tail(TMP): 42
Tail: -63390368
Tail: 16
expected -63390368, found 42
Test [1/1] test.LinkedList... FAIL (TestExpectedEqual)
/home/gianl/zig/lib/std/testing.zig:84:17: 0x214993 in expectEqual__anon_1124 (test)
                return error.TestExpectedEqual;
                ^
/home/gianl/projects/zig/data_structures/src/linked_list.zig:67:5: 0x214b87 in test.LinkedList (test)
    try std.testing.expectEqual(list.head.?.val, 42);
    ^
0 passed; 0 skipped; 1 failed.
error: the following test command failed with exit code 1:
/home/gianl/projects/zig/data_structures/zig-cache/o/dc83372edd4bf92a75e3cb2e9aad0d79/test

If the // std.debug.print("Tmp Again: {d}\n", .{tmp.?.val}); line is uncommented, its prints this:

Test [1/1] test.LinkedList... Tail(TMP): 42
Tail: 42
Tmp Again: 42
Tail: 16
All 1 tests passed.
const std = @import("std");
pub fn LinkedList(comptime T: type) type {
    return struct {
        //Variables
        const Self = @This();
        const Node = struct {
            next: ?*Node,
            prev: ?*Node,
            val: T,
        };
        //Fields
        head: ?*Node = null,
        tail: ?*Node = null,
        len: usize = 0,
        //Methods
        pub fn prepend(self: *Self, val: T) void {
            var node = Node{ .next = null, .prev = null, .val = val };
            var tmp = self.head;
            if (tmp) |head| {
                node.next = head;
                self.head = &node;
                self.len += 1;
            } else {
                self.head = &node;
                self.tail = &node;
                self.len = 1;
            }
        }

        pub fn append(self: *Self, val: T) void {
            var node = Node{.next = null, .prev = null, .val = val};
            var tmp = self.tail;
            std.debug.print("Tail(TMP): {d}\n", .{tmp.?.val});
            if (tmp) |tail| {
                std.debug.print("Tail: {d}\n", .{tail.val});
                // std.debug.print("Tmp Again: {d}\n", .{tmp.?.val});
                tail.next = &node;
                self.tail = &node;
                std.debug.print("Tail: {d}\n", .{self.tail.?.val});
                self.len += 1;
            } else {
                self.head = &node;
                self.tail = &node;
                self.len = 1;
            }
        }
    };
}

test "LinkedList" {
    //The type
    const Listi32 = LinkedList(i32);
    //The actual list
    var list = Listi32{};

    list.prepend(42);
    try std.testing.expectEqual(list.len, 1);
    try std.testing.expectEqual(list.head.?.val, 42);
    try std.testing.expectEqual(list.tail.?.val, 42);

    list.append(16);
    try std.testing.expectEqual(list.len, 2);
    try std.testing.expectEqual(list.head.?.val, 42);
    try std.testing.expectEqual(list.tail.?.val, 16);

}

Expected Behavior

That the unwrapping of the optional would work

        pub fn append(self: *Self, val: T) void {
            var node = Node{.next = null, .prev = null, .val = val};
            var tmp = self.tail;
            std.debug.print("Tail(TMP): {d}\n", .{tmp.?.val});
            // I should have access the the *Node inside of this block
            if (tmp) |tail| {
                // I would expect that tail.val should be 42
                std.debug.print("Tail: {d}\n", .{tail.val});
                // std.debug.print("Tmp Again: {d}\n", .{tmp.?.val});
                tail.next = &node;
                self.tail = &node;
                std.debug.print("Tail: {d}\n", .{self.tail.?.val});
                self.len += 1;
            } else {
                self.head = &node;
                self.tail = &node;
                self.len = 1;
            }
        }
@geefuoco geefuoco added the bug Observed behavior contradicts documented or intended behavior label Aug 5, 2023
@squeek502
Copy link
Collaborator

squeek502 commented Aug 5, 2023

You're storing pointers to locals (node is allocated on the stack of prepend/append), which go out-of-scope after their function returns. This is illegal behavior that is currently unchecked (see #3180).

From the language reference:

Once a function returns, any Pointers to variables in the function's stack frame become invalid references, and dereferencing them becomes unchecked Undefined Behavior.

You would need to heap allocate the Nodes if you want to them to live past the function. See also std.SinglyLinkedList for an idea of how a linked list can be implemented in a way that's agnostic about how Nodes are allocated.

Side note: this type of thing could be run past one of the community spaces first to double check that it's a bug.

@jacobly0 jacobly0 added question No questions on the issue tracker, please. and removed bug Observed behavior contradicts documented or intended behavior labels Aug 5, 2023
@jacobly0 jacobly0 closed this as completed Aug 5, 2023
@jacobly0 jacobly0 closed this as not planned Won't fix, can't repro, duplicate, stale Aug 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question No questions on the issue tracker, please.
Projects
None yet
Development

No branches or pull requests

3 participants