[validation] validate switch statement

This CL adds validation for the following rules:
v-switch01: switch statement selector expression must be of a scalar integer type
v-0008: switch statement must have exactly one default clause
v-switch03: the case selector values must have the same type as the selector expression the case selectors for a switch statement
v-switch04: a literal value must not appear more than once in the case selectors for a switch statement
v-switch05: a fallthrough statement must not appear as the last statement in last clause of a switch

Bug: tint: 6
Change-Id: I264d5079cc6cb31075965c8721651dc76f3e2a24
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/28062
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: David Neto <dneto@google.com>

src/validator_control_block_test.cc

@@ -22,6 +22,7 @@
 #include "src/ast/scalar_constructor_expression.h"
 #include "src/ast/sint_literal.h"
 #include "src/ast/switch_statement.h"
+#include "src/ast/type/alias_type.h"
 #include "src/ast/type/f32_type.h"
 #include "src/ast/type/i32_type.h"
 #include "src/ast/type/u32_type.h"
@@ -37,8 +38,7 @@ namespace {
 class ValidateControlBlockTest : public ValidatorTestHelper,
                                  public testing::Test {};
 
-TEST_F(ValidateControlBlockTest,
+TEST_F(ValidateControlBlockTest, SwitchSelectorExpressionNoneIntegerType_Fail) {
-       DISABLED_SwitchSelectorExpressionNoneIntegerType_Fail) {
   // var a : f32 = 3.14;
   // switch (a) {
   //   default: {}
@@ -49,7 +49,7 @@ TEST_F(ValidateControlBlockTest,
   var->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
       std::make_unique<ast::SintLiteral>(&f32, 3.14f)));
 
-  auto cond = std::make_unique<ast::IdentifierExpression>("a");
+  auto cond = std::make_unique<ast::IdentifierExpression>(Source{12, 34}, "a");
   ast::CaseSelectorList default_csl;
   auto block_default = std::make_unique<ast::BlockStatement>();
   ast::CaseStatementList body;
@@ -58,8 +58,8 @@ TEST_F(ValidateControlBlockTest,
 
   auto block = std::make_unique<ast::BlockStatement>();
   block->append(std::make_unique<ast::VariableDeclStatement>(std::move(var)));
-  block->append(std::make_unique<ast::SwitchStatement>(
+  block->append(
-      Source{12, 34}, std::move(cond), std::move(body)));
+      std::make_unique<ast::SwitchStatement>(std::move(cond), std::move(body)));
 
   EXPECT_TRUE(td()->DetermineStatements(block.get())) << td()->error();
   EXPECT_FALSE(v()->ValidateStatements(block.get()));
@@ -68,7 +68,7 @@ TEST_F(ValidateControlBlockTest,
             "of a scalar integer type");
 }
 
-TEST_F(ValidateControlBlockTest, DISABLED_SwitchWithoutDefault_Fail) {
+TEST_F(ValidateControlBlockTest, SwitchWithoutDefault_Fail) {
   // var a : i32 = 2;
   // switch (a) {
   //   case 1: {}
@@ -94,11 +94,11 @@ TEST_F(ValidateControlBlockTest, DISABLED_SwitchWithoutDefault_Fail) {
   EXPECT_TRUE(td()->DetermineStatements(block.get())) << td()->error();
   EXPECT_FALSE(v()->ValidateStatements(block.get()));
   EXPECT_EQ(v()->error(),
-            "12:34: v-switch02: switch statement must have exactly one default "
+            "12:34: v-0008: switch statement must have exactly one default "
             "clause");
 }
 
-TEST_F(ValidateControlBlockTest, DISABLED_SwitchWithTwoDefault_Fail) {
+TEST_F(ValidateControlBlockTest, SwitchWithTwoDefault_Fail) {
   // var a : i32 = 2;
   // switch (a) {
   //   default: {}
@@ -128,7 +128,46 @@ TEST_F(ValidateControlBlockTest, DISABLED_SwitchWithTwoDefault_Fail) {
   ast::CaseSelectorList default_csl_2;
   auto block_default_2 = std::make_unique<ast::BlockStatement>();
   switch_body.push_back(std::make_unique<ast::CaseStatement>(
-      Source{12, 34}, std::move(default_csl_2), std::move(block_default_2)));
+      std::move(default_csl_2), std::move(block_default_2)));
+
+  auto block = std::make_unique<ast::BlockStatement>();
+  block->append(std::make_unique<ast::VariableDeclStatement>(std::move(var)));
+  block->append(std::make_unique<ast::SwitchStatement>(
+      Source{12, 34}, std::move(cond), std::move(switch_body)));
+
+  EXPECT_TRUE(td()->DetermineStatements(block.get())) << td()->error();
+  EXPECT_FALSE(v()->ValidateStatements(block.get()));
+  EXPECT_EQ(v()->error(),
+            "12:34: v-0008: switch statement must have exactly one default "
+            "clause");
+}
+
+TEST_F(ValidateControlBlockTest,
+       SwitchConditionTypeMustMatchSelectorType2_Fail) {
+  // var a : i32 = 2;
+  // switch (a) {
+  //   case 1: {}
+  //   default: {}
+  // }
+  ast::type::U32Type u32;
+  ast::type::I32Type i32;
+  auto var =
+      std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &i32);
+  var->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+      std::make_unique<ast::SintLiteral>(&i32, 2)));
+
+  ast::CaseStatementList switch_body;
+  auto cond = std::make_unique<ast::IdentifierExpression>("a");
+
+  ast::CaseSelectorList csl;
+  csl.push_back(std::make_unique<ast::UintLiteral>(&u32, 1));
+  switch_body.push_back(std::make_unique<ast::CaseStatement>(
+      Source{12, 34}, std::move(csl), std::make_unique<ast::BlockStatement>()));
+
+  ast::CaseSelectorList default_csl;
+  auto block_default = std::make_unique<ast::BlockStatement>();
+  switch_body.push_back(std::make_unique<ast::CaseStatement>(
+      std::move(default_csl), std::move(block_default)));
 
   auto block = std::make_unique<ast::BlockStatement>();
   block->append(std::make_unique<ast::VariableDeclStatement>(std::move(var)));
@@ -138,12 +177,12 @@ TEST_F(ValidateControlBlockTest, DISABLED_SwitchWithTwoDefault_Fail) {
   EXPECT_TRUE(td()->DetermineStatements(block.get())) << td()->error();
   EXPECT_FALSE(v()->ValidateStatements(block.get()));
   EXPECT_EQ(v()->error(),
-            "12:34: v-switch02: switch statement must have exactly one default "
+            "12:34: v-switch03: the case selector values must have the same "
-            "clause");
+            "type as the selector expression.");
 }
 
 TEST_F(ValidateControlBlockTest,
-       DISABLED_SwitchConditionTypeMustMatchSelectorType_Fail) {
+       SwitchConditionTypeMustMatchSelectorType_Fail) {
   // var a : u32 = 2;
   // switch (a) {
   //   case -1: {}
@@ -176,14 +215,57 @@ TEST_F(ValidateControlBlockTest,
 
   EXPECT_TRUE(td()->DetermineStatements(block.get())) << td()->error();
   EXPECT_FALSE(v()->ValidateStatements(block.get()));
-  EXPECT_EQ(
+  EXPECT_EQ(v()->error(),
-      v()->error(),
+            "12:34: v-switch03: the case selector values must have the same "
-      "12:34: v-switch03: the case selector values must have the same type "
+            "type as the selector expression.");
-      "as the selector expression.");
 }
 
-TEST_F(ValidateControlBlockTest,
+TEST_F(ValidateControlBlockTest, NonUniqueCaseSelectorValueUint_Fail) {
-       DISABLED_NonUniqueCaseSelectorLiteralValue_Fail) {
+  // var a : u32 = 3;
+  // switch (a) {
+  //   case 0: {}
+  //   case 2, 2: {}
+  //   default: {}
+  // }
+  ast::type::U32Type u32;
+  auto var =
+      std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &u32);
+  var->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+      std::make_unique<ast::UintLiteral>(&u32, 3)));
+
+  ast::CaseStatementList switch_body;
+  auto cond = std::make_unique<ast::IdentifierExpression>("a");
+
+  ast::CaseSelectorList csl_1;
+  csl_1.push_back(std::make_unique<ast::UintLiteral>(&u32, 0));
+  switch_body.push_back(std::make_unique<ast::CaseStatement>(
+      std::move(csl_1), std::make_unique<ast::BlockStatement>()));
+
+  ast::CaseSelectorList csl_2;
+  csl_2.push_back(std::make_unique<ast::UintLiteral>(&u32, 2));
+  csl_2.push_back(std::make_unique<ast::UintLiteral>(&u32, 2));
+  switch_body.push_back(std::make_unique<ast::CaseStatement>(
+      Source{12, 34}, std::move(csl_2),
+      std::make_unique<ast::BlockStatement>()));
+
+  ast::CaseSelectorList default_csl;
+  auto block_default = std::make_unique<ast::BlockStatement>();
+  switch_body.push_back(std::make_unique<ast::CaseStatement>(
+      std::move(default_csl), std::move(block_default)));
+
+  auto block = std::make_unique<ast::BlockStatement>();
+  block->append(std::make_unique<ast::VariableDeclStatement>(std::move(var)));
+  block->append(std::make_unique<ast::SwitchStatement>(std::move(cond),
+                                                       std::move(switch_body)));
+
+  EXPECT_TRUE(td()->DetermineStatements(block.get())) << td()->error();
+  EXPECT_FALSE(v()->ValidateStatements(block.get()));
+  EXPECT_EQ(v()->error(),
+            "12:34: v-switch04: a literal value must not appear more than once "
+            "in the case selectors for a switch statement: '2'");
+}
+
+TEST_F(ValidateControlBlockTest, NonUniqueCaseSelectorValueSint_Fail) {
   // var a : i32 = 2;
   // switch (a) {
   //   case 10: {}
@@ -210,7 +292,7 @@ TEST_F(ValidateControlBlockTest,
   csl_2.push_back(std::make_unique<ast::SintLiteral>(&i32, 2));
   csl_2.push_back(std::make_unique<ast::SintLiteral>(&i32, 10));
   switch_body.push_back(std::make_unique<ast::CaseStatement>(
-      Source{12, 34}, std::move(csl_1),
+      Source{12, 34}, std::move(csl_2),
       std::make_unique<ast::BlockStatement>()));
 
   ast::CaseSelectorList default_csl;
@@ -228,11 +310,10 @@ TEST_F(ValidateControlBlockTest,
   EXPECT_EQ(
       v()->error(),
       "12:34: v-switch04: a literal value must not appear more than once in "
-      "the case selectors for a switch statement");
+      "the case selectors for a switch statement: '10'");
 }
 
-TEST_F(ValidateControlBlockTest,
+TEST_F(ValidateControlBlockTest, LastClauseLastStatementIsFallthrough_Fail) {
-       DISABLED_LastClauseLastStatementIsFallthrough_Fail) {
   // var a : i32 = 2;
   // switch (a) {
   //   default: { fallthrough; }
@@ -246,10 +327,11 @@ TEST_F(ValidateControlBlockTest,
   auto cond = std::make_unique<ast::IdentifierExpression>("a");
   ast::CaseSelectorList default_csl;
   auto block_default = std::make_unique<ast::BlockStatement>();
-  block_default->append(std::make_unique<ast::FallthroughStatement>());
+  block_default->append(
+      std::make_unique<ast::FallthroughStatement>(Source{12, 34}));
   ast::CaseStatementList body;
   body.push_back(std::make_unique<ast::CaseStatement>(
-      Source{12, 34}, std::move(default_csl), std::move(block_default)));
+      std::move(default_csl), std::move(block_default)));
 
   auto block = std::make_unique<ast::BlockStatement>();
   block->append(std::make_unique<ast::VariableDeclStatement>(std::move(var)));
@@ -296,5 +378,50 @@ TEST_F(ValidateControlBlockTest, SwitchCase_Pass) {
   EXPECT_TRUE(v()->ValidateStatements(block.get())) << v()->error();
 }
 
+TEST_F(ValidateControlBlockTest, SwitchCaseAlias_Pass) {
+  // entry_point vertex = main
+  // type MyInt = u32;
+  // fn main()->void {
+  //   var v: MyInt;
+  //   switch(v){
+  //     default: {}
+  //   }
+  // }
+  ast::type::U32Type u32;
+  ast::type::AliasType my_int{"MyInt", &u32};
+
+  auto var =
+      std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &my_int);
+  var->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+      std::make_unique<ast::SintLiteral>(&u32, 2)));
+
+  auto cond = std::make_unique<ast::IdentifierExpression>("a");
+  ast::CaseSelectorList default_csl;
+  auto block_default = std::make_unique<ast::BlockStatement>();
+  ast::CaseStatementList body;
+  body.push_back(std::make_unique<ast::CaseStatement>(
+      Source{12, 34}, std::move(default_csl), std::move(block_default)));
+
+  auto block = std::make_unique<ast::BlockStatement>();
+  block->append(std::make_unique<ast::VariableDeclStatement>(std::move(var)));
+  block->append(
+      std::make_unique<ast::SwitchStatement>(std::move(cond), std::move(body)));
+  block->append(std::make_unique<ast::ReturnStatement>());
+
+  ast::type::VoidType void_type;
+  ast::VariableList params;
+  auto func =
+      std::make_unique<ast::Function>("main", std::move(params), &void_type);
+  func->set_body(std::move(block));
+  auto entry_point = std::make_unique<ast::EntryPoint>(
+      ast::PipelineStage::kVertex, "", "main");
+  mod()->AddFunction(std::move(func));
+  mod()->AddAliasType(&my_int);
+  mod()->AddEntryPoint(std::move(entry_point));
+
+  EXPECT_TRUE(td()->Determine()) << td()->error();
+  EXPECT_TRUE(v()->Validate(mod())) << v()->error();
+}
+
 }  // namespace
 }  // namespace tint

src/validator_impl.cc

@@ -14,10 +14,17 @@
 
 #include <cassert>
 #include "src/validator_impl.h"
+#include <unordered_set>
 #include "src/ast/call_statement.h"
 #include "src/ast/function.h"
+#include "src/ast/int_literal.h"
 #include "src/ast/intrinsic.h"
+#include "src/ast/sint_literal.h"
+#include "src/ast/switch_statement.h"
+#include "src/ast/type/i32_type.h"
+#include "src/ast/type/u32_type.h"
 #include "src/ast/type/void_type.h"
+#include "src/ast/uint_literal.h"
 #include "src/ast/variable_decl_statement.h"
 
 namespace tint {
@@ -239,13 +246,94 @@ bool ValidatorImpl::ValidateStatement(const ast::Statement* stmt) {
   if (stmt->IsCall()) {
     return ValidateCallExpr(stmt->AsCall()->expr());
   }
+  if (stmt->IsSwitch()) {
+    return ValidateSwitch(stmt->AsSwitch());
+  }
+  if (stmt->IsCase()) {
+    return ValidateCase(stmt->AsCase());
+  }
+  return true;
+}
+
+bool ValidatorImpl::ValidateSwitch(const ast::SwitchStatement* s) {
+  if (!ValidateExpression(s->condition())) {
+    return false;
+  }
+
+  auto* cond_type = s->condition()->result_type()->UnwrapAliasPtrAlias();
+  if (!(cond_type->IsI32() || cond_type->IsU32())) {
+    set_error(s->condition()->source(),
+              "v-switch01: switch statement selector expression must be of a "
+              "scalar integer type");
+    return false;
+  }
+
+  int default_counter = 0;
+  std::unordered_set<int32_t> selector_set;
+  for (const auto& case_stmt : s->body()) {
+    if (!ValidateStatement(case_stmt.get())) {
+      return false;
+    }
+
+    if (case_stmt.get()->IsDefault()) {
+      default_counter++;
+    }
+
+    for (const auto& selector : case_stmt.get()->selectors()) {
+      auto* selector_ptr = selector.get();
+      if (cond_type != selector_ptr->type()) {
+        set_error(case_stmt.get()->source(),
+                  "v-switch03: the case selector values must have the same "
+                  "type as the selector expression.");
+        return false;
+      }
+
+      auto v = static_cast<int32_t>(selector_ptr->type()->IsU32()
+                                        ? selector_ptr->AsUint()->value()
+                                        : selector_ptr->AsSint()->value());
+      if (selector_set.count(v)) {
+        auto v_str = selector_ptr->type()->IsU32()
+                         ? selector_ptr->AsUint()->to_str()
+                         : selector_ptr->AsSint()->to_str();
+        set_error(
+            case_stmt.get()->source(),
+            "v-switch04: a literal value must not appear more than once in "
+            "the case selectors for a switch statement: '" +
+                v_str + "'");
+        return false;
+      }
+      selector_set.emplace(v);
+    }
+  }
+
+  if (default_counter != 1) {
+    set_error(s->source(),
+              "v-0008: switch statement must have exactly one default clause");
+    return false;
+  }
+
+  auto* last_clause = s->body().back().get();
+  auto* last_stmt_of_last_clause = last_clause->AsCase()->body()->last();
+  if (last_stmt_of_last_clause && last_stmt_of_last_clause->IsFallthrough()) {
+    set_error(last_stmt_of_last_clause->source(),
+              "v-switch05: a fallthrough statement must not appear as "
+              "the last statement in last clause of a switch");
+    return false;
+  }
+  return true;
+}
+
+bool ValidatorImpl::ValidateCase(const ast::CaseStatement* c) {
+  if (!ValidateStatement(c->body())) {
+    return false;
+  }
   return true;
 }
 
 bool ValidatorImpl::ValidateCallExpr(const ast::CallExpression* expr) {
   if (!expr) {
-    // TODO(sarahM0): Here and other Validate.*: figure out whether return false
+    // TODO(sarahM0): Here and other Validate.*: figure out whether return
-    // or true
+    // false or true
     return false;
   }
 

src/validator_impl.h

@@ -115,6 +115,14 @@ class ValidatorImpl {
   /// @param eps the vector of entry points to check
   /// @return true if the validation was successful
   bool ValidateEntryPoints(const ast::EntryPointList& eps);
+  /// Validates switch statements
+  /// @param s the switch statement to check
+  /// @returns true if the valdiation was successful
+  bool ValidateSwitch(const ast::SwitchStatement* s);
+  /// Validates case statements
+  /// @param c the case statement to check
+  /// @returns true if the valdiation was successful
+  bool ValidateCase(const ast::CaseStatement* c);
 
  private:
   std::string error_;