writer: avoid type breakage during AppendVector

When building a vector via tint::writer::AppendVector, and the
vector argument is already a vector constructor, expand that
vector constructor into its components only when those components
are all scalars.  This avoids a type breakage which can occur with cases
like this:

 vector argument is:
	  vec2<i32>(vec2<u32>(0u,1u))
 scalar argument is:
          2

Before this fix, the result was:
  vec2<i32>(0u, 1u, 2);

But should be this instead:
  vec3<i32>(vec2<u32>(0u,1u),2)

This was noticed in SPIR-V writer output when forming a coordinate
vector from a an unsigned WGSL coordinate vector with a signed array
vector.

Fixed: tint:1048
Change-Id: Id46665739cc23da0ca58b9baabf7b4531b86350b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/60040
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>

src/writer/append_vector.cc

@@ -70,12 +70,18 @@ ast::TypeConstructorExpression* AppendVector(ProgramBuilder* b,
   auto* packed_ty = b->create<ast::Vector>(packed_el_ty, packed_size);
   auto* packed_sem_ty = b->create<sem::Vector>(packed_el_sem_ty, packed_size);
 
-  // If the coordinates are already passed in a vector constructor, extract
-  // the elements into the new vector instead of nesting a vector-in-vector.
+  // If the coordinates are already passed in a vector constructor, with only
+  // scalar components supplied, extract the elements into the new vector
+  // instead of nesting a vector-in-vector.
+  // If the coordinates are a zero-constructor of the vector, then expand that
+  // to scalar zeros.
+  // The other cases for a nested vector constructor are when it is used
+  // to convert a vector of a different type, e.g. vec2<i32>(vec2<u32>()).
+  // In that case, preserve the original argument, or you'll get a type error.
   ast::ExpressionList packed;
   if (auto* vc = AsVectorConstructor(b, vector)) {
-    packed = vc->values();
-    if (packed.size() == 0) {
+    const auto num_supplied = vc->values().size();
+    if (num_supplied == 0) {
       // Zero-value vector constructor. Populate with zeros
       auto buildZero = [&]() -> ast::ScalarConstructorExpression* {
         if (packed_el_sem_ty->Is<sem::I32>()) {
@@ -101,8 +107,13 @@ ast::TypeConstructorExpression* AppendVector(ProgramBuilder* b,
                                              sem::Constant{}));
         packed.emplace_back(zero);
       }
+    } else if (num_supplied + 1 == packed_size) {
+      // All vector components were supplied as scalars.  Pass them through.
+      packed = vc->values();
     }
-  } else {
+  }
+  if (packed.empty()) {
+    // The special cases didn't occur. Use the vector argument as-is.
     packed.emplace_back(vector);
   }
   if (packed_el_sem_ty != b->TypeOf(scalar)->UnwrapRef()) {

src/writer/append_vector.h

@@ -27,6 +27,8 @@ class TypeConstructorExpression;
 namespace writer {
 
 /// A helper function used to append a vector with an additional scalar.
+/// If the scalar's type does not match the target vector element type,
+/// then it is value-converted (via TypeConstructor) before being added.
 /// All types must have been assigned to the expressions and their child nodes
 /// before calling.
 /// @param builder the program builder.

src/writer/append_vector_test.cc

@@ -43,6 +43,58 @@ TEST_F(AppendVectorTest, Vec2i32_i32) {
   EXPECT_EQ(vec_123->values()[2], scalar_3);
 }
 
+TEST_F(AppendVectorTest, Vec2i32_u32) {
+  auto* scalar_1 = Expr(1);
+  auto* scalar_2 = Expr(2);
+  auto* scalar_3 = Expr(3u);
+  auto* vec_12 = vec2<i32>(scalar_1, scalar_2);
+  WrapInFunction(vec_12, scalar_3);
+
+  resolver::Resolver resolver(this);
+  ASSERT_TRUE(resolver.Resolve()) << resolver.error();
+
+  auto* vec_123 = AppendVector(this, vec_12, scalar_3)
+                      ->As<ast::TypeConstructorExpression>();
+  ASSERT_NE(vec_123, nullptr);
+  ASSERT_EQ(vec_123->values().size(), 3u);
+  EXPECT_EQ(vec_123->values()[0], scalar_1);
+  EXPECT_EQ(vec_123->values()[1], scalar_2);
+  auto* u32_to_i32 = vec_123->values()[2]->As<ast::TypeConstructorExpression>();
+  ASSERT_NE(u32_to_i32, nullptr);
+  EXPECT_TRUE(u32_to_i32->type()->Is<ast::I32>());
+  ASSERT_EQ(u32_to_i32->values().size(), 1u);
+  EXPECT_EQ(u32_to_i32->values()[0], scalar_3);
+}
+
+TEST_F(AppendVectorTest, Vec2i32FromVec2u32_u32) {
+  auto* scalar_1 = Expr(1u);
+  auto* scalar_2 = Expr(2u);
+  auto* scalar_3 = Expr(3u);
+  auto* uvec_12 = vec2<u32>(scalar_1, scalar_2);
+  auto* vec_12 = vec2<i32>(uvec_12);
+  WrapInFunction(vec_12, scalar_3);
+
+  resolver::Resolver resolver(this);
+  ASSERT_TRUE(resolver.Resolve()) << resolver.error();
+
+  auto* vec_123 = AppendVector(this, vec_12, scalar_3)
+                      ->As<ast::TypeConstructorExpression>();
+  ASSERT_NE(vec_123, nullptr);
+  ASSERT_EQ(vec_123->values().size(), 2u);
+  auto* v2u32_to_v2i32 =
+      vec_123->values()[0]->As<ast::TypeConstructorExpression>();
+  ASSERT_NE(v2u32_to_v2i32, nullptr);
+  EXPECT_TRUE(v2u32_to_v2i32->type()->is_signed_integer_vector());
+  EXPECT_EQ(v2u32_to_v2i32->values().size(), 1u);
+  EXPECT_EQ(v2u32_to_v2i32->values()[0], uvec_12);
+
+  auto* u32_to_i32 = vec_123->values()[1]->As<ast::TypeConstructorExpression>();
+  ASSERT_NE(u32_to_i32, nullptr);
+  EXPECT_TRUE(u32_to_i32->type()->Is<ast::I32>());
+  ASSERT_EQ(u32_to_i32->values().size(), 1u);
+  EXPECT_EQ(u32_to_i32->values()[0], scalar_3);
+}
+
 TEST_F(AppendVectorTest, Vec2i32_f32) {
   auto* scalar_1 = Expr(1);
   auto* scalar_2 = Expr(2);
@@ -61,6 +113,7 @@ TEST_F(AppendVectorTest, Vec2i32_f32) {
   EXPECT_EQ(vec_123->values()[1], scalar_2);
   auto* f32_to_i32 = vec_123->values()[2]->As<ast::TypeConstructorExpression>();
   ASSERT_NE(f32_to_i32, nullptr);
+  EXPECT_TRUE(f32_to_i32->type()->Is<ast::I32>());
   ASSERT_EQ(f32_to_i32->values().size(), 1u);
   EXPECT_EQ(f32_to_i32->values()[0], scalar_3);
 }
@@ -158,6 +211,7 @@ TEST_F(AppendVectorTest, Vec2i32Var_f32Var) {
   EXPECT_EQ(vec_123->values()[0], vec_12);
   auto* f32_to_i32 = vec_123->values()[1]->As<ast::TypeConstructorExpression>();
   ASSERT_NE(f32_to_i32, nullptr);
+  EXPECT_TRUE(f32_to_i32->type()->Is<ast::I32>());
   ASSERT_EQ(f32_to_i32->values().size(), 1u);
   EXPECT_EQ(f32_to_i32->values()[0], scalar_3);
 }