From 3ac4a0e0136ca6288edd61707a5fb86775f9b086 Mon Sep 17 00:00:00 2001
From: Ben Clayton <bclayton@google.com>
Date: Tue, 30 Mar 2021 20:20:28 +0000
Subject: [PATCH] writer/hlsl: Fix emission of EmitStorageBufferAccessor()

Fix number of parentheses emitted for all control flows.

Add a TINT_UNIMPLEMENTED() for types that are not currently handled.

Change-Id: I987776e4d3b6d7c4d237f44db8b0eebb1a9a7fbd
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46266
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
---
 src/writer/hlsl/generator_impl.cc | 42 +++++++++++++++++--------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc
index bd5a00c808..f02a66f46f 100644
--- a/src/writer/hlsl/generator_impl.cc
+++ b/src/writer/hlsl/generator_impl.cc
@@ -95,6 +95,13 @@ std::ostream& operator<<(std::ostream& s, const RegisterAndSpace& rs) {
   return s;
 }
 
+// Helper for writting a '(' on construction and a ')' destruction.
+struct ScopedParen {
+  std::ostream& s_;
+  explicit ScopedParen(std::ostream& s) : s_(s) { s << "("; }
+  ~ScopedParen() { s_ << ")"; }
+};
+
 }  // namespace
 
 GeneratorImpl::GeneratorImpl(const Program* program)
@@ -2185,6 +2192,10 @@ bool GeneratorImpl::EmitStorageBufferAccessor(std::ostream& pre,
       out << "asint(";
     } else if (result_type->is_unsigned_scalar_or_vector()) {
       out << "asuint(";
+    } else {
+      TINT_UNIMPLEMENTED(diagnostics_)
+          << result_type->FriendlyName(builder_.Symbols());
+      return false;
     }
   }
 
@@ -2229,8 +2240,8 @@ bool GeneratorImpl::EmitStorageBufferAccessor(std::ostream& pre,
       return true;
     }
 
-    out << "uint" << mat->rows() << "x" << mat->columns() << "(";
-
+    out << "uint" << mat->rows() << "x" << mat->columns();
+    ScopedParen p(out);
     for (uint32_t i = 0; i < mat->columns(); i++) {
       if (i != 0) {
         out << ", ";
@@ -2239,29 +2250,22 @@ bool GeneratorImpl::EmitStorageBufferAccessor(std::ostream& pre,
       out << buffer_name << "." << access_method << "(" << idx << " + "
           << (i * stride) << ")";
     }
-
-    // Close the matrix type and outer cast
-    out << "))";
-
-    return true;
-  }
-
-  out << buffer_name << "." << access_method << "(" << idx;
-  if (is_store) {
-    out << ", asuint(";
-    if (!EmitExpression(pre, out, rhs)) {
-      return false;
+  } else {
+    out << buffer_name << "." << access_method;
+    ScopedParen p(out);
+    out << idx;
+    if (is_store) {
+      out << ", asuint";
+      ScopedParen p2(out);
+      if (!EmitExpression(pre, out, rhs)) {
+        return false;
+      }
     }
-    out << ")";
   }
 
-  out << ")";
-
-  // Close the outer cast.
   if (!is_store) {
     out << ")";
   }
-
   return true;
 }