From 472950016c4828f7222dcecda85dffa633b9089b Mon Sep 17 00:00:00 2001 From: Jack Jackson Date: Tue, 24 Dec 2024 00:33:33 -0800 Subject: [PATCH] Solution to 5-2 --- NOTES.md | 60 +++++++++++++++++ scratch.zig | 30 ++++++--- solutions/05.zig | 170 ++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 250 insertions(+), 10 deletions(-) diff --git a/NOTES.md b/NOTES.md index ab06b8d..79a5a1e 100644 --- a/NOTES.md +++ b/NOTES.md @@ -1,5 +1,9 @@ Notes, thoughts, or questions that arose as I implemented the solutions. Hopefully I am able to go back and answer these questions as I keep learning! +# Useful references + +* [Zig Notes](https://github.com/david-vanderson/zig-notes) - particularly on Arrays vs. Slices, and Strings. + # Things I like * [Continue expressions](https://zig-by-example.com/while) @@ -136,3 +140,59 @@ scratch.zig:15:20: note: cast discards const qualifier ``` I _think_ this means that the pointer to the Iterator is a Const-pointer and `.next()` expects a mutable pointer. But, if so - how do we get a mutable pointer from a const? I tried `@ptrCast` but that gave a similar error. + +## How to return items accumulated into an ArrayList without causing a memory leak or a segementation fault? + +Trimming down the issues that I first saw in problem 05, here's some example code: + +```zig +const std = @import("std"); +const print = std.debug.print; + +test "Demo accumulation" { + const accumulated = try accumulate(); + print("DEBUG - accumulated values are {any}\n", .{accumulated}); +} + +fn accumulate() ![]u32 { + var gpa = std.heap.GeneralPurposeAllocator(.{}){}; + defer _ = gpa.deinit(); + const allocator = gpa.allocator(); + + var list = std.ArrayList(u32).init(allocator); + // defer list.deinit(); <-- this is the problem line + try list.append(1); + try list.append(2); + try list.append(3); + return list.items; +} +``` + +If the "problem line" is commented out, then I get warnings about a memory leak (unsurprisingly); but if it's left in, then I get a segmentation fault when trying to reference the response of the function. + +This is all, in some sense, "working as expected" (the compiler is correct to warn about the memory leak) - but it seems like a cumbersom way to work. I suspect that the response would be "_don't return a bare `[]u32`, then_", which feels pretty unsatisfying. + +You can't even work around this by creating a buffer (within `accumulate`), copying values into it, `deinit`-ing `list`, and returning the copy - because you can't create an array-buffer without pre-specifying how large it should be, and creating a slice has the same memory-leak issue - see below for example: + +``` +test "Demo accumulation" { + const accumulated = try accumulate(); + print("DEBUG - accumulated values are {any}\n", .{accumulated}); +} + +fn accumulate() ![]u32 { + var gpa = std.heap.GeneralPurposeAllocator(.{}){}; + defer _ = gpa.deinit(); + const allocator = gpa.allocator(); + + var list = std.ArrayList(u32).init(allocator); + defer list.deinit(); + try list.append(1); + try list.append(2); + try list.append(3); + + const response = try allocator.alloc(u32, list.items.len); + @memcpy(response, list.items); + return response; +} +``` \ No newline at end of file diff --git a/scratch.zig b/scratch.zig index a30cc14..6b00d7c 100644 --- a/scratch.zig +++ b/scratch.zig @@ -1,6 +1,27 @@ const std = @import("std"); const print = std.debug.print; +test "Demo accumulation" { + const accumulated = try accumulate(); + print("DEBUG - accumulated values are {any}\n", .{accumulated}); +} + +fn accumulate() ![]u32 { + var gpa = std.heap.GeneralPurposeAllocator(.{}){}; + defer _ = gpa.deinit(); + const allocator = gpa.allocator(); + + var list = std.ArrayList(u32).init(allocator); + defer list.deinit(); + try list.append(1); + try list.append(2); + try list.append(3); + + const response = try allocator.alloc(u32, list.items.len); + @memcpy(response, list.items); + return response; +} + pub fn main() !void { var gpa = std.heap.GeneralPurposeAllocator(.{}){}; defer _ = gpa.deinit(); @@ -16,7 +37,6 @@ pub fn main() !void { print("{c}", .{char}); } list.deinit(); - } fn doIt(string: *const [3:0]u8) *const [9:0]u8 { @@ -24,11 +44,3 @@ fn doIt(string: *const [3:0]u8) *const [9:0]u8 { } const expect = @import("std").testing.expect; - -test { - for (doIt("foo")) |char| {print("{c}", .{char});} - print("\n", .{}); - for ("prefixfoo") |char| {print("{c}", .{char});} - print("\n", .{}); - try expect(std.mem.eql(u8, doIt("foo"), "prefixfoo")); -} \ No newline at end of file diff --git a/solutions/05.zig b/solutions/05.zig index e574e4b..4f7853a 100644 --- a/solutions/05.zig +++ b/solutions/05.zig @@ -3,7 +3,7 @@ const print = std.debug.print; const util = @import("util.zig"); pub fn main() !void { - const output = try part_one(false); + const output = try part_two(false); print("{}\n", .{output}); } @@ -126,6 +126,87 @@ fn updateFromLine(line: []const u8, allocator: std.mem.Allocator) !Update { return return_update; } +pub fn part_two(is_test_case: bool) !u32 { + // I'm sure there's probably a better sorting logic here, but nothing's springing to mind, soooo we're going with... + // * Identify all the incorrect updates. For each of them: + // * For every rule, if the rule fails, swap the elements that made it fail + // * Repeat until the whole set of rules pass + // (I think this depends on Rules being consistent - i.e. if `a|b`, then there is no `b|a`. Which is a reasonable + // assumption otherwise the problems are insoluble!) + + var gpa = std.heap.GeneralPurposeAllocator(.{}){}; + defer _ = gpa.deinit(); + const allocator = gpa.allocator(); + + const input_file = try util.getInputFile("05", is_test_case); + const data = try util.readAllInputWithAllocator(input_file, allocator); + + // https://stackoverflow.com/a/79199470/1040915 + var it = std.mem.splitScalar(u8, data, '\n'); + var lines_list = std.ArrayList([]const u8).init(allocator); + defer lines_list.deinit(); + + var end_of_rules = false; + var rules = std.ArrayList(Rule).init(allocator); + defer rules.deinit(); + var updates = std.ArrayList(Update).init(allocator); + defer updates.deinit(); + + var total: u32 = 0; + while (it.next()) |line| { + if (line.len == 0) { + end_of_rules = true; + continue; + } + + if (!end_of_rules) { + try rules.append(try ruleFromLine(line)); + } else { + var update = try updateFromLine(line, allocator); + + if (!updatePassesAllRules(update, rules.items)) { + reorderUpdate(rules.items, &update); + total += getMiddleValueOfUpdate(update); + } + } + } + return total; +} + +fn reorderUpdate(rules: []Rule, update: *Update) void { + var number_of_reorderings: usize = 0; + while (reorderUpdateOnce(rules, update)) : (number_of_reorderings += 1) { + print("DEBUG - reordered {} times\n", .{number_of_reorderings}); + } +} + +// Ugh, I _hate_ modifying the argument to a function (rather than returning a new value) like some kind of C-using +// barbarian. But hey, when in Rome... +// +// Returns `true` if any changes were made +fn reorderUpdateOnce(rules: []Rule, update: *Update) bool { + var changes_made = false; + for (rules) |rule| { + var have_encountered_second_value = false; // Have to use this because checking for `!= undefined` is itself UB. + var index_of_second_value: usize = undefined; + var idx: usize = 0; + while (idx < update.values.len) : (idx += 1) { + if (update.values[idx] == rule.second) { + index_of_second_value = idx; + have_encountered_second_value = true; + } + if (update.values[idx] == rule.first and have_encountered_second_value == true) { + // Rule has been breached - swap the values, then continue with the next Rule + update.values[idx] = rule.second; + update.values[index_of_second_value] = rule.first; + changes_made = true; + break; + } + } + } + return changes_made; +} + const expect = std.testing.expect; test "part_one" { @@ -133,3 +214,90 @@ test "part_one" { print("DEBUG - part_one_value is {}\n", .{part_one_value}); try expect(part_one_value == 143); } + +test "reordering" { + var gpa = std.heap.GeneralPurposeAllocator(.{}){}; + defer _ = gpa.deinit(); + const allocator = gpa.allocator(); + + var values1 = try allocator.alloc(u32, 5); + defer allocator.free(values1); + values1[0] = 75; + values1[1] = 97; + values1[2] = 47; + values1[3] = 61; + values1[4] = 53; + var update1 = Update{ .values = values1 }; + + var values2 = try allocator.alloc(u32, 3); + defer allocator.free(values2); + values2[0] = 61; + values2[1] = 13; + values2[2] = 29; + var update2 = Update{ .values = values2 }; + + var values3 = try allocator.alloc(u32, 5); + defer allocator.free(values3); + values3[0] = 97; + values3[1] = 13; + values3[2] = 75; + values3[3] = 29; + values3[4] = 47; + var update3 = Update{ .values = values3 }; + + const ruleText = + \\47|53 + \\97|13 + \\97|61 + \\97|47 + \\75|29 + \\61|13 + \\75|53 + \\29|13 + \\97|29 + \\53|29 + \\61|53 + \\97|53 + \\61|29 + \\47|13 + \\75|47 + \\97|75 + \\47|61 + \\75|61 + \\47|29 + \\75|13 + \\53|13 + ; + + var it = std.mem.splitScalar(u8, ruleText, '\n'); + var rules = std.ArrayList(Rule).init(allocator); + defer rules.deinit(); + while (it.next()) |line| { + try rules.append(try ruleFromLine(line)); + } + + reorderUpdate(rules.items, &update1); + try expect(update1.values[0] == 97); + try expect(update1.values[1] == 75); + try expect(update1.values[2] == 47); + try expect(update1.values[3] == 61); + try expect(update1.values[4] == 53); + + reorderUpdate(rules.items, &update2); + try expect(update2.values[0] == 61); + try expect(update2.values[1] == 29); + try expect(update2.values[2] == 13); + + reorderUpdate(rules.items, &update3); + try expect(update3.values[0] == 97); + try expect(update3.values[1] == 75); + try expect(update3.values[2] == 47); + try expect(update3.values[3] == 29); + try expect(update3.values[4] == 13); +} + +test "part_two" { + const part_two_value = try part_two(true); + print("DEBUG - part_one_value is {}\n", .{part_two_value}); + try expect(part_two_value == 123); +}