[Mlir-commits] [mlir] 6609c1c - [mlir] Add a better error message when failing to parse an attribute

River Riddle llvmlistbot at llvm.org
Thu May 5 15:07:00 PDT 2022


Author: River Riddle
Date: 2022-05-05T15:06:11-07:00
New Revision: 6609c1cc5997baa75c45283fd559897334e5d1a2

URL: https://github.com/llvm/llvm-project/commit/6609c1cc5997baa75c45283fd559897334e5d1a2
DIFF: https://github.com/llvm/llvm-project/commit/6609c1cc5997baa75c45283fd559897334e5d1a2.diff

LOG: [mlir] Add a better error message when failing to parse an attribute

The fallback attribute parse path is parsing a Type attribute, but this results
in a really unintuitive error message: `expected non-function type`, which
doesn't really hint at tall that we were trying to parse an attribute. This
commit fixes this by trying to optionally parse a type, and on failure
emitting an error that we were expecting an attribute.

Differential Revision: https://reviews.llvm.org/D124870

Added: 
    

Modified: 
    flang/test/Fir/invalid-types.fir
    mlir/lib/Parser/AttributeParser.cpp
    mlir/test/Dialect/SPIRV/IR/composite-ops.mlir
    mlir/test/Dialect/SPIRV/IR/memory-ops.mlir
    mlir/test/IR/invalid-locations.mlir
    mlir/test/IR/invalid.mlir
    mlir/test/mlir-tblgen/attr-or-type-format.mlir

Removed: 
    


################################################################################
diff  --git a/flang/test/Fir/invalid-types.fir b/flang/test/Fir/invalid-types.fir
index 929ef68558b8c..6de12936f45ff 100644
--- a/flang/test/Fir/invalid-types.fir
+++ b/flang/test/Fir/invalid-types.fir
@@ -6,7 +6,7 @@ func private @box3() -> !fir.boxproc<>
 
 // -----
 
-// expected-error at +2 {{expected non-function type}}
+// expected-error at +2 {{expected attribute value}}
 // expected-error at +1 {{expected affine map}}
 func private @box1() -> !fir.box<!fir.array<?xf32>, >
 

diff  --git a/mlir/lib/Parser/AttributeParser.cpp b/mlir/lib/Parser/AttributeParser.cpp
index 7a8b2431c4152..d891595658039 100644
--- a/mlir/lib/Parser/AttributeParser.cpp
+++ b/mlir/lib/Parser/AttributeParser.cpp
@@ -206,10 +206,13 @@ Attribute Parser::parseAttribute(Type type) {
     return builder.getUnitAttr();
 
   default:
-    // Parse a type attribute.
-    if (Type type = parseType())
-      return TypeAttr::get(type);
-    return nullptr;
+    // Parse a type attribute. We parse `Optional` here to allow for providing a
+    // better error message.
+    Type type;
+    OptionalParseResult result = parseOptionalType(type);
+    if (!result.hasValue())
+      return emitError("expected attribute value"), Attribute();
+    return failed(*result) ? Attribute() : TypeAttr::get(type);
   }
 }
 

diff  --git a/mlir/test/Dialect/SPIRV/IR/composite-ops.mlir b/mlir/test/Dialect/SPIRV/IR/composite-ops.mlir
index 8e96aacac6f65..a4ccd2412a192 100644
--- a/mlir/test/Dialect/SPIRV/IR/composite-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/composite-ops.mlir
@@ -116,7 +116,7 @@ func.func @composite_extract_invalid_index_type_1() -> () {
   %0 = spv.Constant 10 : i32
   %1 = spv.Variable : !spv.ptr<!spv.array<4x!spv.array<4xf32>>, Function>
   %2 = spv.Load "Function" %1 ["Volatile"] : !spv.array<4x!spv.array<4xf32>>
-  // expected-error @+1 {{expected non-function type}}
+  // expected-error @+1 {{expected attribute value}}
   %3 = spv.CompositeExtract %2[%0] : !spv.array<4x!spv.array<4xf32>>
   return
 }
@@ -132,7 +132,7 @@ func.func @composite_extract_invalid_index_type_2(%arg0 : !spv.array<4x!spv.arra
 // -----
 
 func.func @composite_extract_invalid_index_identifier(%arg0 : !spv.array<4x!spv.array<4xf32>>) -> () {
-  // expected-error @+1 {{expected non-function type}}
+  // expected-error @+1 {{expected attribute value}}
   %0 = spv.CompositeExtract %arg0 ]1 : i32) : !spv.array<4x!spv.array<4xf32>>
   return
 }

diff  --git a/mlir/test/Dialect/SPIRV/IR/memory-ops.mlir b/mlir/test/Dialect/SPIRV/IR/memory-ops.mlir
index c5f62805a3084..5bc438c450aa6 100644
--- a/mlir/test/Dialect/SPIRV/IR/memory-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/memory-ops.mlir
@@ -233,7 +233,7 @@ func.func @volatile_aligned_load() -> () {
 
 func.func @simple_load_missing_storageclass() -> () {
   %0 = spv.Variable : !spv.ptr<f32, Function>
-  // expected-error @+1 {{expected non-function type}}
+  // expected-error @+1 {{expected attribute value}}
   %1 = spv.Load %0 : f32
   return
 }
@@ -387,7 +387,7 @@ func.func @aligned_store(%arg0 : f32) -> () {
 
 func.func @simple_store_missing_ptr_type(%arg0 : f32) -> () {
   %0 = spv.Variable : !spv.ptr<f32, Function>
-  // expected-error @+1 {{expected non-function type}}
+  // expected-error @+1 {{expected attribute value}}
   spv.Store  %0, %arg0 : f32
   return
 }

diff  --git a/mlir/test/IR/invalid-locations.mlir b/mlir/test/IR/invalid-locations.mlir
index b72a751cc7862..59cf0fe25537c 100644
--- a/mlir/test/IR/invalid-locations.mlir
+++ b/mlir/test/IR/invalid-locations.mlir
@@ -74,7 +74,7 @@ func.func @location_fused_missing_greater() {
 
 func.func @location_fused_missing_metadata() {
 ^bb:
-  // expected-error at +1 {{expected non-function type}}
+  // expected-error at +1 {{expected attribute value}}
   return loc(fused<) // expected-error {{expected valid attribute metadata}}
 }
 

diff  --git a/mlir/test/IR/invalid.mlir b/mlir/test/IR/invalid.mlir
index ebe3683270c14..6d5bd57c26a08 100644
--- a/mlir/test/IR/invalid.mlir
+++ b/mlir/test/IR/invalid.mlir
@@ -261,7 +261,7 @@ func.func @reference_to_iv_in_bound() {
 // -----
 
 func.func @nonconstant_step(%1 : i32) {
-  affine.for %2 = 1 to 5 step %1 { // expected-error {{expected non-function type}}
+  affine.for %2 = 1 to 5 step %1 { // expected-error {{expected attribute value}}
 
 // -----
 
@@ -618,7 +618,7 @@ func.func @large_bound() {
 // -----
 
 func.func @max_in_upper_bound(%N : index) {
-  affine.for %i = 1 to max affine_map<(i)->(N, 100)> { //expected-error {{expected non-function type}}
+  affine.for %i = 1 to max affine_map<(i)->(N, 100)> { //expected-error {{expected attribute value}}
   }
   return
 }

diff  --git a/mlir/test/mlir-tblgen/attr-or-type-format.mlir b/mlir/test/mlir-tblgen/attr-or-type-format.mlir
index da4d6cbc111c2..8a26a5b1405ff 100644
--- a/mlir/test/mlir-tblgen/attr-or-type-format.mlir
+++ b/mlir/test/mlir-tblgen/attr-or-type-format.mlir
@@ -23,7 +23,7 @@ func.func private @test_ugly_attr_parser_dispatch() -> () attributes {
 
 func.func private @test_ugly_attr_missing_parameter() -> () attributes {
   // expected-error at +2 {{failed to parse TestAttrUgly parameter 'attr'}}
-  // expected-error at +1 {{expected non-function type}}
+  // expected-error at +1 {{expected attribute value}}
   attr = #test<"attr_ugly begin">
 }
 
@@ -97,7 +97,7 @@ func.func private @test_struct_not_enough_values() -> () attributes {
 // -----
 
 func.func private @test_parse_param_after_struct() -> () attributes {
-  // expected-error at +2 {{expected non-function type}}
+  // expected-error at +2 {{expected attribute value}}
   // expected-error at +1 {{failed to parse TestAttrWithFormat parameter 'three'}}
   attr = #test.attr_with_format<42 : two = "foo", four = [1, 2, 3] : >
 }


        


More information about the Mlir-commits mailing list