Falling off the end of a function returns undefined

tslang/lib/TypeScript/MLIRGenFunctions.cpp

@@ -7,6 +7,188 @@ namespace typescript
 namespace mlirgen
 {
 
+    // A `break` that leaves `statement` (the body of a loop or the clauses of a switch): an unlabeled one not
+    // inside a nested loop or switch, or any labeled one - it may target this statement or one around it, and
+    // either way control leaves. Nested functions and classes have breaks of their own.
+    class BreakOutVisitor : public ts::FilterVisitorSkipFuncsAST<ts::BreakStatement>
+    {
+      public:
+        bool found = false;
+        bool labeledOnly;
+
+        BreakOutVisitor(bool labeledOnly)
+            : ts::FilterVisitorSkipFuncsAST<ts::BreakStatement>(SyntaxKind::BreakStatement, [&](ts::BreakStatement breakStatement) {
+                  if (!this->labeledOnly || breakStatement->label)
+                  {
+                      found = true;
+                  }
+              }),
+              labeledOnly(labeledOnly)
+        {
+        }
+
+      protected:
+        virtual bool isFiltered(ts::Node node) override
+        {
+            if (ts::FilterVisitorSkipFuncsAST<ts::BreakStatement>::isFiltered(node))
+            {
+                return true;
+            }
+
+            // inside a nested loop or switch only a labeled break can leave this statement
+            if (!labeledOnly)
+            {
+                SyntaxKind kind = node;
+                switch (kind)
+                {
+                case SyntaxKind::WhileStatement:
+                case SyntaxKind::DoStatement:
+                case SyntaxKind::ForStatement:
+                case SyntaxKind::ForInStatement:
+                case SyntaxKind::ForOfStatement:
+                case SyntaxKind::SwitchStatement:
+                    return true;
+                default:
+                    break;
+                }
+            }
+
+            return false;
+        }
+    };
+
+    static bool hasBreakOut(ts::Node statement)
+    {
+        if (statement == SyntaxKind::BreakStatement)
+        {
+            return true;
+        }
+
+        // unlabeled breaks of this statement
+        BreakOutVisitor ownBreaks(false);
+        ownBreaks.visit(statement);
+        if (ownBreaks.found)
+        {
+            return true;
+        }
+
+        // labeled breaks anywhere in it
+        BreakOutVisitor labeledBreaks(true);
+        labeledBreaks.visit(statement);
+        return labeledBreaks.found;
+    }
+
+    static bool isTrueCondition(ts::Expression expression)
+    {
+        return !expression || expression == SyntaxKind::TrueKeyword;
+    }
+
+    static bool canCompleteNormally(ts::Statement statement);
+
+    static bool canCompleteNormally(ts::NodeArray<ts::Statement> statements)
+    {
+        for (auto statement : statements)
+        {
+            if (!canCompleteNormally(statement))
+            {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    // Whether control can reach the point after `statement`: after a function's body, that is falling off its end,
+    // which returns undefined. Exact for return, throw, blocks, if/else, try/catch/finally, a switch with a
+    // default, and `while (true)` / `for (;;)` / `do .. while (true)`; any other statement is taken to complete.
+    static bool canCompleteNormally(ts::Statement statement)
+    {
+        if (!statement)
+        {
+            return true;
+        }
+
+        SyntaxKind kind = statement;
+        switch (kind)
+        {
+        case SyntaxKind::ReturnStatement:
+        case SyntaxKind::ThrowStatement:
+            return false;
+        case SyntaxKind::Block:
+            return canCompleteNormally(statement.as<ts::Block>()->statements);
+        case SyntaxKind::IfStatement:
+        {
+            auto ifStatement = statement.as<ts::IfStatement>();
+            return !ifStatement->elseStatement || canCompleteNormally(ifStatement->thenStatement)
+                || canCompleteNormally(ifStatement->elseStatement);
+        }
+        case SyntaxKind::TryStatement:
+        {
+            auto tryStatement = statement.as<ts::TryStatement>();
+            if (tryStatement->finallyBlock && !canCompleteNormally(tryStatement->finallyBlock.as<ts::Statement>()))
+            {
+                return false;
+            }
+
+            return canCompleteNormally(tryStatement->tryBlock.as<ts::Statement>())
+                || (tryStatement->catchClause && canCompleteNormally(tryStatement->catchClause->block.as<ts::Statement>()));
+        }
+        case SyntaxKind::WhileStatement:
+        {
+            auto whileStatement = statement.as<ts::WhileStatement>();
+            return !isTrueCondition(whileStatement->expression) || hasBreakOut(whileStatement->statement);
+        }
+        case SyntaxKind::DoStatement:
+        {
+            auto doStatement = statement.as<ts::DoStatement>();
+            return !isTrueCondition(doStatement->expression) || hasBreakOut(doStatement->statement);
+        }
+        case SyntaxKind::ForStatement:
+        {
+            auto forStatement = statement.as<ts::ForStatement>();
+            return !isTrueCondition(forStatement->condition) || hasBreakOut(forStatement->statement);
+        }
+        case SyntaxKind::SwitchStatement:
+        {
+            // a clause falls through to the next one, so only the end of the last clause - or a break - leaves it;
+            // without a default no clause may run at all
+            auto clauses = statement.as<ts::SwitchStatement>()->caseBlock->clauses;
+            auto hasDefault = false;
+            for (auto clause : clauses)
+            {
+                hasDefault |= clause == SyntaxKind::DefaultClause;
+            }
+
+            if (!hasDefault || clauses.size() == 0)
+            {
+                return true;
+            }
+
+            for (auto clause : clauses)
+            {
+                for (auto clauseStatement : clause->statements)
+                {
+                    if (hasBreakOut(clauseStatement))
+                    {
+                        return true;
+                    }
+                }
+            }
+
+            return canCompleteNormally(clauses[clauses.size() - 1]->statements);
+        }
+        default:
+            return true;
+        }
+    }
+
+    // a function body whose end is reachable: falling off it returns undefined
+    static bool bodyFallsOffEnd(ts::Node body)
+    {
+        return body == SyntaxKind::Block && canCompleteNormally(body.as<ts::Statement>());
+    }
+
+
     std::tuple<mlir::LogicalResult, bool, std::vector<std::shared_ptr<FunctionParamDOM>>> MLIRGenImpl::mlirGenParameters(
         SignatureDeclarationBase parametersContextAST, const GenContext &genContext)
     {
@@ -528,8 +710,9 @@ namespace mlirgen
                 funcProto->setDiscovered(true);
                 auto discoveredType = passResult->functionReturnType;
 
-                // a bare `return;` beside value returns: T | undefined
-                if (passResult->hasBareReturn && !hasExplicitReturnType && discoveredType && !mth.isNoneType(discoveredType)
+                // a bare `return;` beside value returns, or an end of the body that is reachable: T | undefined
+                if ((passResult->hasBareReturn || bodyFallsOffEnd(functionLikeDeclarationBaseAST->body))
+                    && !hasExplicitReturnType && discoveredType && !mth.isNoneType(discoveredType)
                     && !isa<mlir_ts::VoidType>(discoveredType))
                 {
                     discoveredType = getUnionType(loc(functionLikeDeclarationBaseAST), discoveredType, getUndefinedType());
@@ -1435,6 +1618,25 @@ namespace mlirgen
             {
                 return mlir::failure();
             }
+
+            // falling off the end of a function returning T | undefined (inferred so, or declared) returns undefined;
+            // the return slot would otherwise be read as it was left
+            if (auto returnType = getExplicitReturnTypeOfCurrentFunction(funcGenContext))
+            {
+                if ((isa<mlir_ts::OptionalType>(returnType) || isa<mlir_ts::AnyType>(returnType)
+                     || isa<mlir_ts::UnknownType>(returnType))
+                    && bodyFallsOffEnd(functionLikeDeclarationBaseAST->body))
+                {
+                    NodeFactory nf(NodeFactoryFlags::None);
+                    auto undefinedReturn = nf.createReturnStatement(nf.createIdentifier(S(UNDEFINED_NAME)));
+                    undefinedReturn->pos = functionLikeDeclarationBaseAST->body->_end - 1;
+                    undefinedReturn->_end = functionLikeDeclarationBaseAST->body->_end;
+                    if (failed(mlirGen(undefinedReturn, funcGenContext)))
+                    {
+                        return mlir::failure();
+                    }
+                }
+            }
         }
 
         // Falling off the end of the body reaches here rather than through a `return`, and the

tslang/test/tester/CMakeLists.txt

@@ -396,6 +396,7 @@ tslang_add_test(NAME test-compile-00-length-signed-arithmetic COMMAND test-runne
 tslang_add_test(NAME test-compile-00-optional-fields COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00optional_fields.ts")
 tslang_add_test(NAME test-compile-00-generator-return COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00generator_return.ts")
 tslang_add_test(NAME test-compile-00-return-inference-undefined COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00return_inference_undefined.ts")
+tslang_add_test(NAME test-compile-00-return-falls-off-end COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00return_falls_off_end.ts")
 tslang_add_test(NAME test-compile-00-intersection-type COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00intersection_type.ts")
 tslang_add_test(NAME test-compile-00-intersection-type-generic COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00intersection_type_generic.ts")
 tslang_add_test(NAME test-compile-00-length COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00length.ts")
@@ -875,6 +876,7 @@ tslang_add_test(NAME test-jit-00-length-signed-arithmetic COMMAND test-runner -j
 tslang_add_test(NAME test-jit-00-optional-fields COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00optional_fields.ts")
 tslang_add_test(NAME test-jit-00-generator-return COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00generator_return.ts")
 tslang_add_test(NAME test-jit-00-return-inference-undefined COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00return_inference_undefined.ts")
+tslang_add_test(NAME test-jit-00-return-falls-off-end COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00return_falls_off_end.ts")
 tslang_add_test(NAME test-jit-00-union-global-init COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00union_global_init.ts")
 tslang_add_test(NAME test-jit-00-intersection-type COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00intersection_type.ts")
 tslang_add_test(NAME test-jit-00-intersection-type-generic COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00intersection_type_generic.ts")

tslang/test/tester/tests/00return_falls_off_end.ts

@@ -0,0 +1,90 @@
+// Falling off the end of a function returns undefined, so a function that can do so beside value returns infers
+// `T | undefined`; one whose every path returns or throws keeps `T`
+function ifWithoutElse(c: boolean) {
+    if (c) return "a";
+}
+
+function ifElse(c: boolean) {
+    if (c) return "a";
+    else return "b";
+}
+
+function switchWithDefault(n: number) {
+    switch (n) {
+        case 1: return "one";
+        default: return "many";
+    }
+}
+
+function switchWithoutDefault(n: number) {
+    switch (n) {
+        case 1: return "one";
+        case 2: return "two";
+    }
+}
+
+function infiniteLoop(n: number) {
+    while (true) {
+        if (n > 0) return n;
+        n++;
+    }
+}
+
+function loopWithBreak(n: number) {
+    for (;;) {
+        if (n > 2.5) break;
+        n++;
+    }
+
+    return n;
+}
+
+function tryCatch(c: boolean) {
+    try {
+        if (c) return 1;
+        throw 1;
+    } catch {
+        return 2;
+    }
+}
+
+function returnOrThrow(c: boolean) {
+    if (c) {
+        return "x";
+    }
+
+    throw "e";
+}
+
+function loopReturn(n: number) {
+    for (const k of [1, 2]) {
+        if (k == n) return k;
+    }
+}
+
+function declared(n: number): number | undefined {
+    if (n > 0) return n;
+}
+
+function main() {
+    assert(ifWithoutElse(false) === undefined, "if without else: falls off");
+    assert(ifWithoutElse(true) == "a", "if without else: value");
+    assert(ifElse(false) == "b", "if/else");
+    assert(switchWithDefault(5) == "many", "switch with default");
+    assert(switchWithoutDefault(3) === undefined, "switch without default: falls off");
+    assert(switchWithoutDefault(2) == "two", "switch without default: value");
+    assert(infiniteLoop(1) == 1, "while (true)");
+    assert(loopWithBreak(0) == 3, "for (;;) with break");
+    assert(tryCatch(false) == 2, "try/catch");
+    assert(returnOrThrow(true) == "x", "return or throw");
+    assert(loopReturn(5) === undefined, "loop: falls off");
+    assert(loopReturn(2) == 2, "loop: value");
+    assert(declared(-1) === undefined, "declared T | undefined: falls off");
+    assert(declared(2) == 2, "declared: value");
+
+    // these keep T: a value of a type that cannot be undefined
+    const s: string = ifElse(true) + switchWithDefault(1) + returnOrThrow(true);
+    assert(s == "aonex", "always-returning functions keep T");
+
+    print("done.");
+}