Reject continue placed directly in a for update region

Compiler/Core/Tests/LoopLoweringTests.cpp

@@ -7,6 +7,7 @@
 #include <optional>
 #include <ranges>
 #include <string>
+#include <string_view>
 #include <utility>
 #include <vector>
 
@@ -451,3 +452,55 @@ TEST_CASE("do-while continue targets the trailing condition block",
     for (const auto &block : function.blocks)
         CHECK(ReachesReturn(function, block.id));
 }
+
+TEST_CASE("Core verifier rejects continue in a for update region",
+          "[core][verifier][loop]")
+{
+    const auto codes = [](std::vector<Core::Statement> body,
+                          std::vector<Core::Statement> update) {
+        std::vector<std::string> result;
+        for (const auto &issue : Core::Verify(LoopModule(Core::Statement::For(
+                 Compare(Core::Primitive::LessThan, kIndex, 3),
+                 std::move(body),
+                 std::move(update)))))
+            result.push_back(issue.code);
+        return result;
+    };
+    const auto has
+        = [](const std::vector<std::string> &found, std::string_view code) {
+              return std::ranges::find(found, code) != found.end();
+          };
+
+    // The update region is the loop's continuation point: a `continue`
+    // there would re-enter the update and never test the condition again.
+    CHECK(has(codes({}, { Core::Statement::Continue() }), "VXC1066"));
+    CHECK(has(
+        codes({},
+              { Core::Statement::If(Compare(Core::Primitive::Equal, kIndex, 1),
+                                    { Core::Statement::Continue() },
+                                    {}) }),
+        "VXC1066"));
+
+    // `continue` in the body, `break` in the update, and `continue` in a
+    // loop nested inside the update all keep a defined target.
+    CHECK(
+        codes({ Core::Statement::Continue() }, { Increment(kIndex, U"index") })
+            .empty());
+    CHECK(codes({ Accumulate() },
+                { Increment(kIndex, U"index"), Core::Statement::Break() })
+              .empty());
+    CHECK(codes({ Accumulate() },
+                { Increment(kIndex, U"index"),
+                  Core::Statement::While(
+                      Compare(Core::Primitive::LessThan, kTotal, 0),
+                      { Core::Statement::Continue() }) })
+              .empty());
+
+    // Outside any loop the existing diagnostic applies, not the new one.
+    auto module = LoopModule(Core::Statement::Continue());
+    std::vector<std::string> outside;
+    for (const auto &issue : Core::Verify(module))
+        outside.push_back(issue.code);
+    CHECK(has(outside, "VXC1065"));
+    CHECK_FALSE(has(outside, "VXC1066"));
+}

Compiler/Core/Verifier.cpp

@@ -178,23 +178,37 @@ namespace Visual::XSharp::Core
                 }
                 return false;
             }
+            /// Where a `break` or `continue` would transfer to. A `for`
+            /// update region is inside its loop for `break`, which leaves the
+            /// loop, but it is the loop's continuation point itself: a
+            /// `continue` there has no later point of the same iteration to
+            /// reach and would re-enter the update without testing the
+            /// condition. A loop nested in an update opens a body scope again.
+            enum class TransferScope : std::uint8_t
+            {
+                OutsideLoop,
+                InLoopBody,
+                InForUpdate
+            };
+
             void
             VerifyStatements(const llvm::ArrayRef<Statement> statements,
                              Environment &environment,
                              const Type &expectedReturnType,
-                             const std::size_t loopDepth = 0U)
+                             const TransferScope scope
+                             = TransferScope::OutsideLoop)
             {
                 for (const auto &statement : statements)
                     VerifyStatement(statement,
                                     environment,
                                     expectedReturnType,
-                                    loopDepth);
+                                    scope);
             }
             void
             VerifyStatement(const Statement &statement,
                             Environment &environment,
                             const Type &expectedReturnType,
-                            const std::size_t loopDepth)
+                            const TransferScope scope)
             {
                 switch (statement.kind)
                 {
@@ -269,11 +283,11 @@ namespace Visual::XSharp::Core
                         VerifyStatements(statement.trueBranch,
                                          trueEnvironment,
                                          expectedReturnType,
-                                         loopDepth);
+                                         scope);
                         VerifyStatements(statement.falseBranch,
                                          falseEnvironment,
                                          expectedReturnType,
-                                         loopDepth);
+                                         scope);
                         return;
                     }
                     case Statement::Kind::Evaluate:
@@ -291,25 +305,29 @@ namespace Visual::XSharp::Core
                         VerifyStatements(statement.loopBody,
                                          loopEnvironment,
                                          expectedReturnType,
-                                         loopDepth + 1U);
+                                         TransferScope::InLoopBody);
                         if (statement.kind == Statement::Kind::For)
                         {
                             auto updateEnvironment = environment;
                             VerifyStatements(statement.loopUpdate,
                                              updateEnvironment,
                                              expectedReturnType,
-                                             loopDepth + 1U);
+                                             TransferScope::InForUpdate);
                         }
                         return;
                     }
                     case Statement::Kind::Break:
-                        if (loopDepth == 0U)
+                        if (scope == TransferScope::OutsideLoop)
                             Add("VXC1064", "Core break appears outside a loop");
                         return;
                     case Statement::Kind::Continue:
-                        if (loopDepth == 0U)
+                        if (scope == TransferScope::OutsideLoop)
                             Add("VXC1065",
                                 "Core continue appears outside a loop");
+                        if (scope == TransferScope::InForUpdate)
+                            Add("VXC1066",
+                                "Core continue appears in a for update "
+                                "region");
                         return;
                 }
             }
@@ -673,7 +691,7 @@ namespace Visual::XSharp::Core
                 VerifyStatements(*expression.closureBody,
                                  closureEnvironment,
                                  expression.closureReturnType,
-                                 0U);
+                                 TransferScope::OutsideLoop);
                 if (expression.closureReturnType != Type::unit()
                     && !AlwaysReturns(*expression.closureBody))
                     Add("VXC1042",

Compiler/Haskell/Core/src/Visual/XSharp/Core/Verifier.hs

@@ -64,7 +64,7 @@ verifyFunction functionEnvironment function =
         ++ unresolvedType "VXC1003" "Core function has an unresolved return type" (coreFunctionReturnType function)
         ++ duplicates "VXC1004" "duplicate Core parameter symbol" parameterSymbols
         ++ concatMap (uncurry verifyParameter) (coreFunctionParameters function)
-        ++ fst (verifyStatements initialEnvironment (coreFunctionReturnType function) 0 (coreFunctionBody function))
+        ++ fst (verifyStatements initialEnvironment (coreFunctionReturnType function) OutsideLoop (coreFunctionBody function))
         ++ missingReturn
     where
         parameterSymbols = map (resolvedSymbol . fst) (coreFunctionParameters function)
@@ -82,15 +82,24 @@ verifyParameter name valueType =
     invalidSymbol "VXC1006" "Core parameter symbol must be positive" name
         ++ unresolvedType "VXC1007" "Core parameter has an unresolved type" valueType
 
-verifyStatements :: Environment -> Type -> Int -> [CoreStatement] -> ([Diagnostic], Environment)
+{- | Where a @break@ or @continue@ would transfer to. A @for@ update region is
+inside its loop for @break@, which leaves the loop, but it is the loop's
+continuation point itself: a @continue@ there has no later point of the same
+iteration to reach and would re-enter the update without testing the
+condition. A loop nested in an update opens an ordinary body scope again.
+-}
+data TransferScope = OutsideLoop | InLoopBody | InForUpdate
+    deriving (Eq)
+
+verifyStatements :: Environment -> Type -> TransferScope -> [CoreStatement] -> ([Diagnostic], Environment)
 verifyStatements environment _ _ [] = ([], environment)
-verifyStatements environment returnType loopDepth (statement : remaining) =
-    let (currentProblems, nextEnvironment) = verifyStatement environment returnType loopDepth statement
-        (remainingProblems, finalEnvironment) = verifyStatements nextEnvironment returnType loopDepth remaining
+verifyStatements environment returnType scope (statement : remaining) =
+    let (currentProblems, nextEnvironment) = verifyStatement environment returnType scope statement
+        (remainingProblems, finalEnvironment) = verifyStatements nextEnvironment returnType scope remaining
      in (currentProblems ++ remainingProblems, finalEnvironment)
 
-verifyStatement :: Environment -> Type -> Int -> CoreStatement -> ([Diagnostic], Environment)
-verifyStatement environment returnType loopDepth statement = case statement of
+verifyStatement :: Environment -> Type -> TransferScope -> CoreStatement -> ([Diagnostic], Environment)
+verifyStatement environment returnType scope statement = case statement of
     CoreBind binding ->
         let name = coreBindingName binding
             symbol = resolvedSymbol name
@@ -135,29 +144,30 @@ verifyStatement environment returnType loopDepth statement = case statement of
                     ++ [ problem "VXC1017" "Core condition must be bool or numeric"
                        | expressionType condition /= boolType && not (isCoreNumericType (expressionType condition))
                        ]
-            (trueProblems, _) = verifyStatements environment returnType loopDepth trueBranch
-            (falseProblems, _) = verifyStatements environment returnType loopDepth falseBranch
+            (trueProblems, _) = verifyStatements environment returnType scope trueBranch
+            (falseProblems, _) = verifyStatements environment returnType scope falseBranch
          in (conditionProblems ++ trueProblems ++ falseProblems, environment)
     CoreEvaluate value -> (verifyExpression environment value, environment)
     CoreWhile condition body ->
         let conditionProblems = verifyLoopCondition environment "while" condition
-            (bodyProblems, _) = verifyStatements environment returnType (loopDepth + 1) body
+            (bodyProblems, _) = verifyStatements environment returnType InLoopBody body
          in (conditionProblems ++ bodyProblems, environment)
     CoreDoWhile body condition ->
-        let (bodyProblems, _) = verifyStatements environment returnType (loopDepth + 1) body
+        let (bodyProblems, _) = verifyStatements environment returnType InLoopBody body
             conditionProblems = verifyLoopCondition environment "do/while" condition
          in (bodyProblems ++ conditionProblems, environment)
     CoreFor condition body update ->
         let conditionProblems = verifyLoopCondition environment "for" condition
-            (bodyProblems, _) = verifyStatements environment returnType (loopDepth + 1) body
-            (updateProblems, _) = verifyStatements environment returnType (loopDepth + 1) update
+            (bodyProblems, _) = verifyStatements environment returnType InLoopBody body
+            (updateProblems, _) = verifyStatements environment returnType InForUpdate update
          in (conditionProblems ++ bodyProblems ++ updateProblems, environment)
     CoreBreak ->
-        ( [problem "VXC1045" "Core break is not nested in a loop" | loopDepth == 0]
+        ( [problem "VXC1045" "Core break is not nested in a loop" | scope == OutsideLoop]
         , environment
         )
     CoreContinue ->
-        ( [problem "VXC1046" "Core continue is not nested in a loop" | loopDepth == 0]
+        ( [problem "VXC1046" "Core continue is not nested in a loop" | scope == OutsideLoop]
+            ++ [problem "VXC1066" "Core continue appears in a for update region" | scope == InForUpdate]
         , environment
         )
 
@@ -229,7 +239,7 @@ verifyClosure environment captures parameters returnType body valueType =
         ++ concatMap verifyCapture captures
         ++ concatMap (uncurry verifyParameter) parameters
         ++ callableTypeProblems
-        ++ fst (verifyStatements closureEnvironment returnType 0 body)
+        ++ fst (verifyStatements closureEnvironment returnType OutsideLoop body)
         ++ [ problem "VXC1032" "non-void Core closure may complete without returning"
            | returnType /= unitType && not (statementsAlwaysReturn body)
            ]

Compiler/Haskell/Driver/test/CoreVerifierTests.hs

@@ -75,6 +75,23 @@ coreVerifierTests =
         , rejectedWith "VXC1028" (primitiveModule CoreEqual [boolValue, boolValue] intType)
         )
     , ("Core verifier accepts all-returning branches", accepted allReturningBranch)
+    , ("Core verifier rejects break outside a loop", rejectedWith "VXC1045" (loopModule [CoreBreak]))
+    , ("Core verifier rejects continue outside a loop", rejectedWith "VXC1046" (loopModule [CoreContinue]))
+    , ("Core verifier accepts continue in a for body", accepted (forModule [CoreContinue] []))
+    , ("Core verifier accepts break in a for update region", accepted (forModule [] [CoreBreak]))
+    , ("Core verifier rejects continue in a for update region", rejectedWith "VXC1066" (forModule [] [CoreContinue]))
+    ,
+        ( "Core verifier rejects continue nested in a branch of a for update region"
+        , rejectedWith "VXC1066" (forModule [] [CoreIf boolValue [CoreContinue] []])
+        )
+    ,
+        ( "Core verifier accepts continue in a loop nested in a for update region"
+        , accepted (forModule [] [CoreWhile boolValue [CoreContinue]])
+        )
+    ,
+        ( "Core verifier does not report an outside-loop continue as an update continue"
+        , not (rejectedWith "VXC1066" (loopModule [CoreContinue]))
+        )
     , ("Core verifier accepts a well-typed direct call", accepted validDirectCall)
     , ("Core verifier accepts a well-typed closure", accepted validClosure)
     ]
@@ -383,3 +400,9 @@ validDirectCall =
 
 validClosure :: CoreModule
 validClosure = coreModule [unitFunction mainName [CoreEvaluate validClosureValue]]
+
+loopModule :: [CoreStatement] -> CoreModule
+loopModule body = coreModule [unitFunction mainName body]
+
+forModule :: [CoreStatement] -> [CoreStatement] -> CoreModule
+forModule body update = loopModule [CoreFor boolValue body update]

Documents/CORE-IR.md

@@ -234,6 +234,17 @@ tracks loop nesting independently for each function and closure body and rejects
 either statement outside a loop. Nested loops push a new target pair, so an
 inner transfer cannot accidentally jump to an outer loop.
 
+The update list of a classic `for` is that loop's continuation point, so the
+two transfers differ there. `CoreBreak` in an update list is valid and exits
+the loop after the statements before it. `CoreContinue` placed directly in an
+update list, including inside a `CoreIf` there, is rejected with `VXC1066`:
+it has no later point of the same iteration to reach, and lowering it would
+jump back to the start of the update without testing the condition. A loop
+nested inside an update list has its own body and continuation point, so
+`CoreContinue` is valid again inside it. Source code cannot produce this form
+because a `for` update is a list of expressions; the rule protects Core built
+or transformed by other means.
+
 Optimizer passes preserve the explicit loop form unless their rewrite proves
 the replacement semantics, including effects and transfer edges. In
 particular, a constant condition does not permit deleting an effectful body or