From aa9e7bd71d122da0bd84ccd78987cb014580074c Mon Sep 17 00:00:00 2001 From: Jared Ramirez Date: Sat, 20 Dec 2025 16:34:25 -0500 Subject: [PATCH] Add some comments --- src/check/unify.zig | 8 ++++++-- src/types/generalize.zig | 32 +++++++++++++++++++++----------- test.roc | 10 +++++++--- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/check/unify.zig b/src/check/unify.zig index 1fdb26407b..10017ddba3 100644 --- a/src/check/unify.zig +++ b/src/check/unify.zig @@ -650,7 +650,9 @@ const Unifier = struct { // then we redirect both a & b to the new alias. const fresh_alias_var = self.fresh(vars, .{ .alias = a_alias }) catch return Error.AllocatorError; - // TODO: Is it possible to loose rank information here? I suspect so... + // These redirects are safe because fresh_alias_var is created at min(a_rank, b_rank). + // Because of this, we do not loose any rank information. + // This is essentially a custom `self.merge` strategy self.types_store.dangerousSetVarRedirect(vars.a.var_, fresh_alias_var) catch return Error.AllocatorError; self.types_store.dangerousSetVarRedirect(vars.b.var_, fresh_alias_var) catch return Error.AllocatorError; }, @@ -739,7 +741,9 @@ const Unifier = struct { // then we redirect both a & b to the new alias. const fresh_alias_var = self.fresh(vars, .{ .alias = b_alias }) catch return Error.AllocatorError; - // TODO: Is it possible to loose rank information here? I suspect so... + // These redirects are safe because fresh_alias_var is created at min(a_rank, b_rank). + // Because of this, we do not loose any rank information. + // This is essentially a custom `self.merge` strategy self.types_store.dangerousSetVarRedirect(vars.a.var_, fresh_alias_var) catch return Error.AllocatorError; self.types_store.dangerousSetVarRedirect(vars.b.var_, fresh_alias_var) catch return Error.AllocatorError; }, diff --git a/src/types/generalize.zig b/src/types/generalize.zig index 51174aa53d..5fe83e2b59 100644 --- a/src/types/generalize.zig +++ b/src/types/generalize.zig @@ -328,9 +328,15 @@ pub const Generalizer = struct { return group_rank; }, .alias => |alias| { - // THEORY: Here, we don't need to recurse into the alias backing - // variable, because those variables are have been instantiated - // at the alias's rank. + // THEORY: Here, we don't need to recurse into the backing type because: + // 1. We visit the type arguments (args) + // 2. Anything in the RHS of the alias is either: + // - A reference to an arg (already visited via args) + // - A concrete type (doesn't need rank adjustment) + // So traversing the backing var would be redundant. + // + // We use top_level as a default, as the type container itself + // does not contribute to the rank calculation. var next_rank = Rank.top_level; var args_iter = self.store.iterAliasArgs(alias); while (args_iter.next()) |arg_var| { @@ -341,7 +347,7 @@ pub const Generalizer = struct { .structure => |flat_type| { switch (flat_type) { .empty_record, .empty_tag_union => { - // Empty records/tag unions never need to be generalized + // THEORY: Empty records/tag unions never need to be generalized return .top_level; }, .tuple => |tuple| { @@ -353,17 +359,21 @@ pub const Generalizer = struct { } return next_rank; } else { - // Empty tuples never need to be generalized + // THEORY: Empty tuples never need to be generalized return .top_level; } }, .nominal_type => |nominal| { - // TODO: Do we need to recurse into the nominal types backing - // var here? We do not for alias. But here, it may make sense - // to do so, because types can be unified directly against - // a nominal type's backing var. - - var next_rank = try self.adjustRank(self.store.getNominalBackingVar(nominal), group_rank, vars_to_generalize); + // THEORY: Here, we don't need to recurse into the backing type because: + // 1. We visit the type arguments (args) + // 2. Anything in the RHS of the nominal type is either: + // - A reference to an arg (already visited via args) + // - A concrete type (doesn't need rank adjustment) + // So traversing the backing var would be redundant. + // + // We use top_level as a default, as the type container itself + // does not contribute to the rank calculation. + var next_rank = Rank.top_level; var args_iter = self.store.iterNominalArgs(nominal); while (args_iter.next()) |arg_var| { next_rank = next_rank.max(try self.adjustRank(arg_var, group_rank, vars_to_generalize)); diff --git a/test.roc b/test.roc index 48b753a9eb..ce25434467 100644 --- a/test.roc +++ b/test.roc @@ -5,7 +5,11 @@ app [main!] { main! : List(Str) => Try({}, [Exit(I32)]) main! = |_args| { utf8 = [120, 121, 122] - _parsed = parse!(utf8, 0, [])? + _parsed = + match parse!(utf8, 0, []) { + Ok(v) => Ok(v) + Err(_) => Err(1) + } Ok({}) } @@ -91,12 +95,12 @@ TopLevel : [ TypeDefinition({name: Str, type: Type, position: U64}), ] -parse_type = |file, index| { +parse_type = |_file, index| { Ok((Name("T"), index)) } parse_block : List(U8), U64, List(Statement) -> Try((List(Statement), U64), Str) -parse_block = |file, index, acc| { +parse_block = |_file, index, acc| { Ok((acc, index)) }