From 16ebc48c9d197fccbc57948215091f80519375d1 Mon Sep 17 00:00:00 2001
From: Geoffrey Martin-Noble <gcmn@google.com>
Date: Wed, 29 May 2019 13:46:41 -0700
Subject: [PATCH]     Change elements literal parsing to not rely on shaped
 type being a vector or tensor.

    This is in preparation for making MemRef a ShapedType. In general, a shaped type should be anything with shape, rank, and element type properties, so use sites shouldn't assume more than that.

    I also pulled the trailing comma parsing out the parseElementsLiteralType (new name) method. It seems weird to have the method parse the type + a trailing comma, even if all call sites currently need that. It's surprising behavior without looking at the implementation.

--

PiperOrigin-RevId: 250558363
---
 mlir/lib/Parser/Parser.cpp | 46 ++++++++++++++++++++++++++++------------------
 mlir/test/IR/invalid.mlir  |  4 ++--
 2 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/mlir/lib/Parser/Parser.cpp b/mlir/lib/Parser/Parser.cpp
index 801ec40..553aa92 100644
--- a/mlir/lib/Parser/Parser.cpp
+++ b/mlir/lib/Parser/Parser.cpp
@@ -189,7 +189,7 @@ public:
                                                   IntegerSet &set);
   DenseElementsAttr parseDenseElementsAttr(ShapedType type);
   DenseElementsAttr parseDenseElementsAttrAsTensor(Type eltType);
-  ShapedType parseShapedType();
+  ShapedType parseElementsLiteralType();
 
   // Location Parsing.
 
@@ -1203,10 +1203,13 @@ Attribute Parser::parseAttribute(Type type) {
     if (parseToken(Token::comma, "expected ','"))
       return nullptr;
 
-    auto type = parseShapedType();
+    auto type = parseElementsLiteralType();
     if (!type)
       return nullptr;
 
+    if (parseToken(Token::comma, "expected ',' after elements literal type"))
+      return nullptr;
+
     if (getToken().getKind() != Token::string)
       return (emitError("opaque string should start with '0x'"), nullptr);
     auto val = getToken().getStringValue();
@@ -1227,9 +1230,11 @@ Attribute Parser::parseAttribute(Type type) {
     if (parseToken(Token::less, "expected '<' after 'splat'"))
       return nullptr;
 
-    auto type = parseShapedType();
+    auto type = parseElementsLiteralType();
     if (!type)
       return nullptr;
+    if (parseToken(Token::comma, "expected ',' after elements literal type"))
+      return nullptr;
     switch (getToken().getKind()) {
     case Token::floatliteral:
     case Token::integer:
@@ -1253,10 +1258,13 @@ Attribute Parser::parseAttribute(Type type) {
     if (parseToken(Token::less, "expected '<' after 'dense'"))
       return nullptr;
 
-    auto type = parseShapedType();
+    auto type = parseElementsLiteralType();
     if (!type)
       return nullptr;
 
+    if (parseToken(Token::comma, "expected ',' after elements literal type"))
+      return nullptr;
+
     auto attr = parseDenseElementsAttr(type);
     if (!attr)
       return nullptr;
@@ -1271,10 +1279,13 @@ Attribute Parser::parseAttribute(Type type) {
     if (parseToken(Token::less, "Expected '<' after 'sparse'"))
       return nullptr;
 
-    auto type = parseShapedType();
+    auto type = parseElementsLiteralType();
     if (!type)
       return nullptr;
 
+    if (parseToken(Token::comma, "expected ',' after elements literal type"))
+      return nullptr;
+
     switch (getToken().getKind()) {
     case Token::l_square: {
       /// Parse indices
@@ -1381,26 +1392,25 @@ DenseElementsAttr Parser::parseDenseElementsAttr(ShapedType type) {
 
 /// Shaped type for elements attribute.
 ///
-///   shaped-type ::= vector-type | tensor-type
+///   elements-literal-type ::= vector-type | ranked-tensor-type
 ///
 /// This method also checks the type has static shape.
-ShapedType Parser::parseShapedType() {
-  auto elementType = parseType();
-  if (!elementType)
+ShapedType Parser::parseElementsLiteralType() {
+  auto type = parseType();
+  if (!type)
     return nullptr;
 
-  auto type = elementType.dyn_cast<ShapedType>();
-  if (!type) {
-    return (emitError("elements literal must be a shaped type"), nullptr);
+  if (!type.isa<RankedTensorType>() && !type.isa<VectorType>()) {
+    return (
+        emitError("elements literal must be a ranked tensor or vector type"),
+        nullptr);
   }
 
-  if (parseToken(Token::comma, "expected ','"))
-    return nullptr;
+  auto sType = type.cast<ShapedType>();
+  if (!sType.hasStaticShape())
+    return (emitError("elements literal type must have static shape"), nullptr);
 
-  if (!type.hasStaticShape()) {
-    return (emitError("shaped literal must have static shape"), nullptr);
-  }
-  return type;
+  return sType;
 }
 
 /// Debug Location.
diff --git a/mlir/test/IR/invalid.mlir b/mlir/test/IR/invalid.mlir
index bcdeab1..1e7fc3d 100644
--- a/mlir/test/IR/invalid.mlir
+++ b/mlir/test/IR/invalid.mlir
@@ -591,14 +591,14 @@ func@n(){^b(
 
 func @elementsattr_non_tensor_type() -> () {
 ^bb0:
-  "foo"(){bar: dense<i32, [4]>} : () -> () // expected-error {{elements literal must be a shaped type}}
+  "foo"(){bar: dense<i32, [4]>} : () -> () // expected-error {{elements literal must be a ranked tensor or vector type}}
 }
 
 // -----
 
 func @elementsattr_non_ranked() -> () {
 ^bb0:
-  "foo"(){bar: dense<tensor<?xi32>, [4]>} : () -> () // expected-error {{shaped literal must have static shape}}
+  "foo"(){bar: dense<tensor<?xi32>, [4]>} : () -> () // expected-error {{elements literal type must have static shape}}
 }
 
 // -----
-- 
2.7.4