commit 7166f6ba9358d3af996f0ea8526515033f3698ad
author James Price <jrprice@google.com> Thu Sep 30 17:29:50 2021 +0000
committer James Price <jrprice@google.com> Thu Sep 30 17:29:50 2021 +0000
tree 59242c6522964d8b293cef64190058af69c0849e
parent 1ca6fbad8f231ffe3d68eb88e939b5eafba119e2

validation: Fix continue-bypasses-decl check

An expression that is inside an if-statement condition does not have a
"current block", which is what we were using to see if the usage was
inside a continuing block. Use the current statement to find the
containing block instead.

Fixed: chromium:1251664
Change-Id: Icc808ca1cf6a1b51757da8303901fa5ecb693e83
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/65520
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>

src/resolver/resolver.cc

diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index ba66017..14f5abb 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -2971,12 +2971,12 @@
     var->users.push_back(expr);
     set_referenced_from_function_if_needed(var, true);
 
-    if (current_block_) {
+    if (current_statement_) {
       // If identifier is part of a loop continuing block, make sure it
       // doesn't refer to a variable that is bypassed by a continue statement
       // in the loop's body block.
       if (auto* continuing_block =
-              current_block_
+              current_statement_
                   ->FindFirstParent<sem::LoopContinuingBlockStatement>()) {
         auto* loop_block =
             continuing_block->FindFirstParent<sem::LoopBlockStatement>();

src/resolver/validation_test.cc

diff --git a/src/resolver/validation_test.cc b/src/resolver/validation_test.cc
index 06cc2bb..1ee8f6f 100644
--- a/src/resolver/validation_test.cc
+++ b/src/resolver/validation_test.cc
@@ -616,6 +616,36 @@
 }
 
 TEST_F(ResolverValidationTest,
+       Stmt_Loop_ContinueInLoopBodySubscopeBeforeDecl_UsageOutsideBlock) {
+  // loop  {
+  //     if (true) {
+  //         continue; // bypasses z decl (if we reach here)
+  //     }
+  //     var z : i32;
+  //     continuing {
+  //         // Must fail even if z is used in an expression that isn't
+  //         // directly contained inside a block.
+  //         if (z < 2) {
+  //         }
+  //     }
+  // }
+
+  auto error_loc = Source{Source::Location{12, 34}};
+  auto* body = Block(If(Expr(true), Block(create<ast::ContinueStatement>())),
+                     Decl(Var("z", ty.i32(), ast::StorageClass::kNone)));
+  auto* compare = create<ast::BinaryExpression>(ast::BinaryOp::kLessThan,
+                                                Expr(error_loc, "z"), Expr(2));
+  auto* continuing = Block(If(compare, Block()));
+  auto* loop_stmt = Loop(body, continuing);
+  WrapInFunction(loop_stmt);
+
+  EXPECT_FALSE(r()->Resolve()) << r()->error();
+  EXPECT_EQ(r()->error(),
+            "12:34 error: continue statement bypasses declaration of 'z' in "
+            "continuing block");
+}
+
+TEST_F(ResolverValidationTest,
        Stmt_Loop_ContinueInLoopBodySubscopeBeforeDecl_UsageInContinuingLoop) {
   // loop  {
   //     if (true) {