From a4c1c238581e2416d11b333f668c3473bd77b115 Mon Sep 17 00:00:00 2001 From: Michal Majer Date: Thu, 16 May 2024 12:55:15 +0200 Subject: [PATCH 1/6] Fix wrong type of orelse expression Fixes #1330 --- src/analysis.zig | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/analysis.zig b/src/analysis.zig index 331003dd6..c74e2b200 100644 --- a/src/analysis.zig +++ b/src/analysis.zig @@ -1470,7 +1470,29 @@ fn resolveTypeOfNodeUncached(analyser: *Analyser, node_handle: NodeWithHandle) e .deref => try analyser.resolveDerefType(base_type), .unwrap_optional => try analyser.resolveOptionalUnwrap(base_type), .array_access => try analyser.resolveBracketAccessType(base_type, .Single), - .@"orelse" => try analyser.resolveOptionalUnwrap(base_type), + .@"orelse" => { + // If the rhs is noreturn, return the left side unwrapped + // Else return the right side + const base_right = .{ .node = datas[node].rhs, .handle = handle }; + const base_type_right = try analyser.resolveTypeOfNodeInternal(base_right) orelse return null; + const idx = switch (base_type_right.data) { + .ip_index => |payload| payload.index, + else => { + return base_type_right; + }, + }; + const ty = analyser.ip.typeOf(idx); + const simple = switch (analyser.ip.indexToKey(ty)) { + .simple_type => |simple| simple, + else => { + return null; + }, + }; + return switch (simple) { + .noreturn, .comptime_int, .comptime_float => analyser.resolveOptionalUnwrap(base_type), + else => base_type_right, + }; + }, .@"catch" => try analyser.resolveUnwrapErrorUnionType(base_type, .payload), .@"try" => try analyser.resolveUnwrapErrorUnionType(base_type, .payload), .address_of => try analyser.resolveAddressOf(base_type), From a6cf531ccfbae47f2adc09011a95400b9c554cf0 Mon Sep 17 00:00:00 2001 From: Michal Majer Date: Thu, 16 May 2024 12:56:32 +0200 Subject: [PATCH 2/6] Fix the type of `while (true) {}` --- src/analysis.zig | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/src/analysis.zig b/src/analysis.zig index c74e2b200..f25ff3472 100644 --- a/src/analysis.zig +++ b/src/analysis.zig @@ -1875,13 +1875,29 @@ fn resolveTypeOfNodeUncached(analyser: *Analyser, node_handle: NodeWithHandle) e label_token: ?Ast.TokenIndex, then_expr: Ast.Node.Index, else_expr: Ast.Node.Index, - } = if (ast.fullWhile(tree, node)) |while_node| - .{ + } = if (ast.fullWhile(tree, node)) |while_node| no_hang: { + + // while (true) {} is a noreturn type + if (try analyser.resolveTypeOfNodeInternal(.{ .node = while_node.ast.cond_expr, .handle = handle })) |cond_type| { + switch (cond_type.data) { + .ip_index => |payload| { + switch (payload.index) { + .bool_true => { + return try Type.typeValFromIP(analyser, .noreturn_type); + }, + else => {}, + } + }, + else => {}, + } + } + + break :no_hang .{ .label_token = while_node.label_token, .then_expr = while_node.ast.then_expr, .else_expr = while_node.ast.else_expr, - } - else if (ast.fullFor(tree, node)) |for_node| + }; + } else if (ast.fullFor(tree, node)) |for_node| .{ .label_token = for_node.label_token, .then_expr = for_node.ast.then_expr, @@ -2144,10 +2160,13 @@ fn resolveTypeOfNodeUncached(analyser: *Analyser, node_handle: NodeWithHandle) e .switch_case, .switch_case_inline, .switch_range, + => {}, .@"continue", .@"break", .@"return", - => {}, + => { + return try Type.typeValFromIP(analyser, .noreturn_type); + }, .@"await", .@"suspend", From 0d39ad3d15edaad9024bfde0036a9d0d2ab11acb Mon Sep 17 00:00:00 2001 From: Michal Majer Date: Thu, 16 May 2024 12:57:27 +0200 Subject: [PATCH 3/6] Added tests for orelse --- tests/lsp_features/hover.zig | 78 ++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/tests/lsp_features/hover.zig b/tests/lsp_features/hover.zig index e8d00618b..66cb2cfd3 100644 --- a/tests/lsp_features/hover.zig +++ b/tests/lsp_features/hover.zig @@ -554,6 +554,84 @@ test "optional" { \\ \\Go to [S](file:///test.zig#L1) ); + + try testHover( + \\const foo: ?i32 = 5; + \\const bar = foo orelse 0; + , + \\```zig + \\const bar = foo orelse 0 + \\``` + \\```zig + \\(i32) + \\``` + ); + + try testHover( + \\const foo: ?i32 = 5; + \\const bar = foo orelse foo; + , + \\```zig + \\const bar = foo orelse foo + \\``` + \\```zig + \\(?i32) + \\``` + ); + + try testHover( + \\const foo: ?i32 = 5; + \\const bar = foo orelse unreachable; + , + \\```zig + \\const bar = foo orelse unreachable + \\``` + \\```zig + \\(i32) + \\``` + ); + + try testHover( + \\fn foo(a: ?i32) void { + \\ const bar = a orelse return; + \\} + , + \\```zig + \\const bar = a orelse return + \\``` + \\```zig + \\(i32) + \\``` + ); + + try testHover( + \\fn foo() void { + \\ const array: [1]?i32 = [1]?i32{ 4 }; + \\ for (array) |elem| { + \\ const bar = elem orelse continue; + \\ } + \\} + , + \\```zig + \\const bar = elem orelse continue + \\``` + \\```zig + \\(i32) + \\``` + ); + + try testHover( + \\fn foo(foo: ?i32) void { + \\ const bar = foo orelse while (true) {}; + \\} + , + \\```zig + \\const bar = foo orelse while (true) {} + \\``` + \\```zig + \\(i32) + \\``` + ); } test "error union" { From 3d089f2f38fbd8e3995a2fcf890f302730d93a15 Mon Sep 17 00:00:00 2001 From: Michal Majer Date: Fri, 17 May 2024 22:49:16 +0200 Subject: [PATCH 4/6] Simplified solution of orelse type resolution --- src/analysis.zig | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/src/analysis.zig b/src/analysis.zig index f25ff3472..653908c19 100644 --- a/src/analysis.zig +++ b/src/analysis.zig @@ -1471,26 +1471,10 @@ fn resolveTypeOfNodeUncached(analyser: *Analyser, node_handle: NodeWithHandle) e .unwrap_optional => try analyser.resolveOptionalUnwrap(base_type), .array_access => try analyser.resolveBracketAccessType(base_type, .Single), .@"orelse" => { - // If the rhs is noreturn, return the left side unwrapped - // Else return the right side - const base_right = .{ .node = datas[node].rhs, .handle = handle }; - const base_type_right = try analyser.resolveTypeOfNodeInternal(base_right) orelse return null; - const idx = switch (base_type_right.data) { - .ip_index => |payload| payload.index, - else => { - return base_type_right; - }, - }; - const ty = analyser.ip.typeOf(idx); - const simple = switch (analyser.ip.indexToKey(ty)) { - .simple_type => |simple| simple, - else => { - return null; - }, - }; - return switch (simple) { - .noreturn, .comptime_int, .comptime_float => analyser.resolveOptionalUnwrap(base_type), - else => base_type_right, + const type_right = try analyser.resolveTypeOfNodeInternal(.{ .node = datas[node].rhs, .handle = handle }) orelse return null; + return switch (type_right.data) { + .optional => type_right, + else => try analyser.resolveOptionalUnwrap(base_type), }; }, .@"catch" => try analyser.resolveUnwrapErrorUnionType(base_type, .payload), From 3db29bf08f038218aad563c9a16be60684638992 Mon Sep 17 00:00:00 2001 From: Michal Majer Date: Mon, 20 May 2024 18:35:23 +0200 Subject: [PATCH 5/6] Separate logic into `resolveOrelseType` --- src/analysis.zig | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/analysis.zig b/src/analysis.zig index 653908c19..9ba8042c2 100644 --- a/src/analysis.zig +++ b/src/analysis.zig @@ -799,6 +799,13 @@ pub fn resolveOptionalUnwrap(analyser: *Analyser, optional: Type) error{OutOfMem } } +pub fn resolveOrelseType(analyser: *Analyser, lhs: Type, rhs: Type) error{OutOfMemory}!?Type { + return switch (rhs.data) { + .optional => rhs, + else => try analyser.resolveOptionalUnwrap(lhs), + }; +} + /// Resolves the child type of an optional type pub fn resolveOptionalChildType(analyser: *Analyser, optional_type: Type) error{OutOfMemory}!?Type { _ = analyser; @@ -1472,10 +1479,7 @@ fn resolveTypeOfNodeUncached(analyser: *Analyser, node_handle: NodeWithHandle) e .array_access => try analyser.resolveBracketAccessType(base_type, .Single), .@"orelse" => { const type_right = try analyser.resolveTypeOfNodeInternal(.{ .node = datas[node].rhs, .handle = handle }) orelse return null; - return switch (type_right.data) { - .optional => type_right, - else => try analyser.resolveOptionalUnwrap(base_type), - }; + return try analyser.resolveOrelseType(base_type, type_right); }, .@"catch" => try analyser.resolveUnwrapErrorUnionType(base_type, .payload), .@"try" => try analyser.resolveUnwrapErrorUnionType(base_type, .payload), From 402b2cd60726de3519d3ef8015dd114cdd22b4d8 Mon Sep 17 00:00:00 2001 From: Michal Majer Date: Mon, 20 May 2024 18:36:12 +0200 Subject: [PATCH 6/6] Revert while loop type resolution --- src/analysis.zig | 24 ++++-------------------- tests/lsp_features/hover.zig | 13 ------------- 2 files changed, 4 insertions(+), 33 deletions(-) diff --git a/src/analysis.zig b/src/analysis.zig index 9ba8042c2..531ce3f16 100644 --- a/src/analysis.zig +++ b/src/analysis.zig @@ -1863,29 +1863,13 @@ fn resolveTypeOfNodeUncached(analyser: *Analyser, node_handle: NodeWithHandle) e label_token: ?Ast.TokenIndex, then_expr: Ast.Node.Index, else_expr: Ast.Node.Index, - } = if (ast.fullWhile(tree, node)) |while_node| no_hang: { - - // while (true) {} is a noreturn type - if (try analyser.resolveTypeOfNodeInternal(.{ .node = while_node.ast.cond_expr, .handle = handle })) |cond_type| { - switch (cond_type.data) { - .ip_index => |payload| { - switch (payload.index) { - .bool_true => { - return try Type.typeValFromIP(analyser, .noreturn_type); - }, - else => {}, - } - }, - else => {}, - } - } - - break :no_hang .{ + } = if (ast.fullWhile(tree, node)) |while_node| + .{ .label_token = while_node.label_token, .then_expr = while_node.ast.then_expr, .else_expr = while_node.ast.else_expr, - }; - } else if (ast.fullFor(tree, node)) |for_node| + } + else if (ast.fullFor(tree, node)) |for_node| .{ .label_token = for_node.label_token, .then_expr = for_node.ast.then_expr, diff --git a/tests/lsp_features/hover.zig b/tests/lsp_features/hover.zig index 66cb2cfd3..d212121a4 100644 --- a/tests/lsp_features/hover.zig +++ b/tests/lsp_features/hover.zig @@ -619,19 +619,6 @@ test "optional" { \\(i32) \\``` ); - - try testHover( - \\fn foo(foo: ?i32) void { - \\ const bar = foo orelse while (true) {}; - \\} - , - \\```zig - \\const bar = foo orelse while (true) {} - \\``` - \\```zig - \\(i32) - \\``` - ); } test "error union" {