From a59bf32eabfa6ec2787d91557c5e3e24bb23c2db Mon Sep 17 00:00:00 2001 From: Anton Trunov Date: Mon, 10 Jun 2024 11:26:28 +0530 Subject: [PATCH] fix: typechecking of conditional expressions (#394) in the presense of subtyping --- CHANGELOG.md | 1 + .../resolveStatements.spec.ts.snap | 129 ++++++++++++++++++ src/types/isAssignable.ts | 2 +- src/types/resolveExpression.ts | 20 ++- src/types/stmts-failed/case-54.tact | 12 ++ src/types/stmts/case-22.tact | 36 +++++ 6 files changed, 192 insertions(+), 8 deletions(-) create mode 100644 src/types/stmts-failed/case-54.tact create mode 100644 src/types/stmts/case-22.tact diff --git a/CHANGELOG.md b/CHANGELOG.md index 371b54033..fa97832cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Return type of `skipBits` now matches FunC and does not lead to compilation errors: PR [#388](https://github.com/tact-lang/tact/pull/388) +- Typechecking of conditional expressions when one branch's type is a subtype of another, i.e. for optionals and maps/`null`: PR [#394](https://github.com/tact-lang/tact/pull/394) ## [1.3.1] - 2024-06-08 diff --git a/src/types/__snapshots__/resolveStatements.spec.ts.snap b/src/types/__snapshots__/resolveStatements.spec.ts.snap index d597fbaae..0207f1fff 100644 --- a/src/types/__snapshots__/resolveStatements.spec.ts.snap +++ b/src/types/__snapshots__/resolveStatements.spec.ts.snap @@ -540,6 +540,16 @@ Line 5, col 12: " `; +exports[`resolveStatements should fail statements for case-54 1`] = ` +":6:5: Type mismatch: "Int?" is not assignable to "Int" +Line 6, col 5: + 5 | let cond: Bool = true; +> 6 | return cond ? 42 : x; + ^~~~~~~~~~~~~~~~~~~~~ + 7 | } +" +`; + exports[`resolveStatements should resolve statements for case-0 1`] = ` [ [ @@ -1417,3 +1427,122 @@ exports[`resolveStatements should resolve statements for case-21 1`] = ` ], ] `; + +exports[`resolveStatements should resolve statements for case-22 1`] = ` +[ + [ + "true", + "Bool", + ], + [ + "cond", + "Bool", + ], + [ + "42", + "Int", + ], + [ + "x", + "Int?", + ], + [ + "cond ? 42 : x", + "Int?", + ], + [ + "true", + "Bool", + ], + [ + "cond", + "Bool", + ], + [ + "x", + "Int?", + ], + [ + "42", + "Int", + ], + [ + "cond ? x : 42", + "Int?", + ], + [ + "true", + "Bool", + ], + [ + "cond", + "Bool", + ], + [ + "null", + "", + ], + [ + "m", + "map", + ], + [ + "cond ? null : m", + "map", + ], + [ + "true", + "Bool", + ], + [ + "cond", + "Bool", + ], + [ + "m", + "map", + ], + [ + "null", + "", + ], + [ + "cond ? m : null", + "map", + ], + [ + "true", + "Bool", + ], + [ + "cond", + "Bool", + ], + [ + "42", + "Int", + ], + [ + "Baz { b: 42 }", + "Baz", + ], + [ + "Baz { b: 42 }.toCell()", + "Cell", + ], + [ + "x", + "Cell?", + ], + [ + "cond ? Baz { b: 42 }.toCell() : x", + "Cell?", + ], + [ + "Bar { + a: cond ? Baz { b: 42 }.toCell() : x + }", + "Bar", + ], +] +`; diff --git a/src/types/isAssignable.ts b/src/types/isAssignable.ts index 6d2690050..49d40bd1f 100644 --- a/src/types/isAssignable.ts +++ b/src/types/isAssignable.ts @@ -3,7 +3,7 @@ import { TypeRef } from "./types"; export function isAssignable(src: TypeRef, to: TypeRef): boolean { // If both are refs if (src.kind === "ref" && to.kind === "ref") { - // Can assign optional to non-optional + // Cannot assign optional to non-optional if (!to.optional && src.optional) { return false; } diff --git a/src/types/resolveExpression.ts b/src/types/resolveExpression.ts index dc45a1ae6..99d72ac27 100644 --- a/src/types/resolveExpression.ts +++ b/src/types/resolveExpression.ts @@ -663,15 +663,21 @@ export function resolveConditional( ctx = resolveExpression(ast.elseBranch, sctx, ctx); const thenType = getExpType(ctx, ast.thenBranch); const elseType = getExpType(ctx, ast.elseBranch); - if (!typeRefEquals(thenType, elseType)) { - throwError( - `Non-matching types "${printTypeRef(thenType)}" and "${printTypeRef(elseType)}" for ternary branches`, - ast.elseBranch.ref, - ); + + // This takes care of sub-typing for optionals and maps/null + if (isAssignable(thenType, elseType)) { + // the type of the else branch is the more general one + return registerExpType(ctx, ast, elseType); + } + if (isAssignable(elseType, thenType)) { + // the type of the then branch is the more general one + return registerExpType(ctx, ast, thenType); } - // Register result - return registerExpType(ctx, ast, thenType); + throwError( + `Non-matching types "${printTypeRef(thenType)}" and "${printTypeRef(elseType)}" for ternary branches`, + ast.elseBranch.ref, + ); } export function resolveLValueRef( diff --git a/src/types/stmts-failed/case-54.tact b/src/types/stmts-failed/case-54.tact new file mode 100644 index 000000000..6b35f8ede --- /dev/null +++ b/src/types/stmts-failed/case-54.tact @@ -0,0 +1,12 @@ +primitive Int; +primitive Bool; + +fun foo1(x: Int?): Int { + let cond: Bool = true; + return cond ? 42 : x; +} + +fun foo2(x: Int?): Int { + let cond: Bool = true; + return cond ? x : 42; +} diff --git a/src/types/stmts/case-22.tact b/src/types/stmts/case-22.tact new file mode 100644 index 000000000..25a77c362 --- /dev/null +++ b/src/types/stmts/case-22.tact @@ -0,0 +1,36 @@ +primitive Int; +primitive Bool; +primitive Cell; + +fun foo1(x: Int?): Int? { + let cond: Bool = true; + return cond ? 42 : x; +} + +fun foo2(x: Int?): Int? { + let cond: Bool = true; + return cond ? x : 42; +} + +fun bar1(m: map): map { + let cond: Bool = true; + return cond ? null : m; +} + +fun bar2(m: map): map { + let cond: Bool = true; + return cond ? m : null; +} + +struct Bar { + a: Cell?; +} + +struct Baz { b: Int; } + +fun baz(x: Cell?): Bar { + let cond: Bool = true; + return Bar { + a: cond ? Baz { b: 42 }.toCell() : x + }; +}