commit b487c71e59d80835fd27973b0a3a2951e19e04c3
author James Price <jrprice@google.com> Tue Jun 01 12:18:10 2021 +0000
committer Tint LUCI CQ <tint-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Jun 01 12:18:10 2021 +0000
tree 5bd9531c26f8b06261f29810c2d05a8e542b1dc7
parent 42220ba1b2572521fb247eb86a8b1a93eef8509c

writer: Add parentheses for array accesses

The LHS should be wrapped in parentheses if it has lower precedence
than the access. This fixes issues with pointer dereferences followed
by array accesses, where we were previously generating *a[i].

Change-Id: I639f94b642f32119350b0a0d23f4ff2a5d6c3c25
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52843
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>

src/writer/msl/generator_impl.cc

diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc
index 81e06e5..6d0a523 100644
--- a/src/writer/msl/generator_impl.cc
+++ b/src/writer/msl/generator_impl.cc
@@ -165,9 +165,20 @@
 }
 
 bool GeneratorImpl::EmitArrayAccessor(ast::ArrayAccessorExpression* expr) {
+  bool paren_lhs =
+      !expr->array()
+           ->IsAnyOf<ast::ArrayAccessorExpression, ast::CallExpression,
+                     ast::IdentifierExpression, ast::MemberAccessorExpression,
+                     ast::TypeConstructorExpression>();
+  if (paren_lhs) {
+    out_ << "(";
+  }
   if (!EmitExpression(expr->array())) {
     return false;
   }
+  if (paren_lhs) {
+    out_ << ")";
+  }
   out_ << "[";
 
   if (!EmitExpression(expr->idx_expr())) {

src/writer/msl/generator_impl_array_accessor_test.cc

diff --git a/src/writer/msl/generator_impl_array_accessor_test.cc b/src/writer/msl/generator_impl_array_accessor_test.cc
index 0c3c67b..f5c2f13 100644
--- a/src/writer/msl/generator_impl_array_accessor_test.cc
+++ b/src/writer/msl/generator_impl_array_accessor_test.cc
@@ -32,6 +32,19 @@
   EXPECT_EQ(gen.result(), "ary[5]");
 }
 
+TEST_F(MslGeneratorImplTest, ArrayAccessor_OfDref) {
+  Global("ary", ty.array<i32, 10>(), ast::StorageClass::kPrivate);
+
+  auto* p = Const("p", nullptr, AddressOf("ary"));
+  auto* expr = IndexAccessor(Deref("p"), 5);
+  WrapInFunction(p, expr);
+
+  GeneratorImpl& gen = Build();
+
+  ASSERT_TRUE(gen.EmitExpression(expr)) << gen.error();
+  EXPECT_EQ(gen.result(), "(*(p))[5]");
+}
+
 }  // namespace
 }  // namespace msl
 }  // namespace writer

src/writer/wgsl/generator_impl.cc

diff --git a/src/writer/wgsl/generator_impl.cc b/src/writer/wgsl/generator_impl.cc
index 17b8359..9ed6527 100644
--- a/src/writer/wgsl/generator_impl.cc
+++ b/src/writer/wgsl/generator_impl.cc
@@ -141,9 +141,20 @@
 }
 
 bool GeneratorImpl::EmitArrayAccessor(ast::ArrayAccessorExpression* expr) {
+  bool paren_lhs =
+      !expr->array()
+           ->IsAnyOf<ast::ArrayAccessorExpression, ast::CallExpression,
+                     ast::IdentifierExpression, ast::MemberAccessorExpression,
+                     ast::TypeConstructorExpression>();
+  if (paren_lhs) {
+    out_ << "(";
+  }
   if (!EmitExpression(expr->array())) {
     return false;
   }
+  if (paren_lhs) {
+    out_ << ")";
+  }
   out_ << "[";
 
   if (!EmitExpression(expr->idx_expr())) {

src/writer/wgsl/generator_impl_array_accessor_test.cc

diff --git a/src/writer/wgsl/generator_impl_array_accessor_test.cc b/src/writer/wgsl/generator_impl_array_accessor_test.cc
index 77137ae..db18d16 100644
--- a/src/writer/wgsl/generator_impl_array_accessor_test.cc
+++ b/src/writer/wgsl/generator_impl_array_accessor_test.cc
@@ -32,6 +32,19 @@
   EXPECT_EQ(gen.result(), "ary[5]");
 }
 
+TEST_F(WgslGeneratorImplTest, ArrayAccessor_OfDref) {
+  Global("ary", ty.array<i32, 10>(), ast::StorageClass::kPrivate);
+
+  auto* p = Const("p", nullptr, AddressOf("ary"));
+  auto* expr = IndexAccessor(Deref("p"), 5);
+  WrapInFunction(p, expr);
+
+  GeneratorImpl& gen = Build();
+
+  ASSERT_TRUE(gen.EmitExpression(expr)) << gen.error();
+  EXPECT_EQ(gen.result(), "(*(p))[5]");
+}
+
 }  // namespace
 }  // namespace wgsl
 }  // namespace writer