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

[rriddle]

Summary

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.

[mehdi_amini]

Thanks! That "expected non-function type" message annoyed me so many times :)
(I suspect it will in other contexts than the one you're fixing)

flang/test/Fir/invalid-types.fir

 // Test the FIR types parser diagnostics
 // RUN: fir-opt -split-input-file -verify-diagnostics %s
 
 // expected-error@+1 {{expected non-function type}}
 func private @box3() -> !fir.boxproc<>
 
 // -----
 
-// expected-error@+2 {{expected non-function type}}
+// expected-error@+2 {{expected attribute value}}
 // expected-error@+1 {{expected affine map}}
 func private @box1() -> !fir.box<!fir.array<?xf32>, >
 
 // -----
 
 // expected-error@+1 {{expected non-function type}}
 func private @box1() -> !fir.box<>
 

mlir/lib/Parser/AttributeParser.cpp

Attribute Parser::parseAttribute(Type type) {
   }
 
   // Parse a 'unit' attribute.
   case Token::kw_unit:
     consumeToken(Token::kw_unit);
     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);
   }
 }
 
 /// Parse an optional attribute with the provided type.
 OptionalParseResult Parser::parseOptionalAttribute(Attribute &attribute,
                                                    Type type) {
   switch (getToken().getKind()) {
   case Token::at_identifier:

mlir/test/Dialect/SPIRV/IR/composite-ops.mlir

 }
 
 // -----
 
 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
 }
 
 // -----
 
 func.func @composite_extract_invalid_index_type_2(%arg0 : !spv.array<4x!spv.array<4xf32>>) -> () {
   // expected-error @+1 {{attribute 'indices' failed to satisfy constraint: 32-bit integer array attribute}}
   %0 = spv.CompositeExtract %arg0[1] : !spv.array<4x!spv.array<4xf32>>
   return
 }
 
 // -----
 
 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
 }
 
 // -----
 
 func.func @composite_extract_2D_array_out_of_bounds_access_1(%arg0: !spv.array<4x!spv.array<4xf32>>) -> () {
   // expected-error @+1 {{index 4 out of bounds for '!spv.array<4 x !spv.array<4 x f32>>'}}

mlir/test/Dialect/SPIRV/IR/memory-ops.mlir

func.func @volatile_aligned_load() -> () {
   %1 = "spv.Load"(%0) {memory_access = 3 : i32, alignment = 4 : i32} : (!spv.ptr<f32, Function>) -> (f32)
   return
 }
 
 // -----
 
 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
 }
 
 // -----
 
 func.func @simple_load_missing_operand() -> () {
   %0 = spv.Variable : !spv.ptr<f32, Function>
func.func @aligned_store(%arg0 : f32) -> () {
   spv.Store  "Function" %0, %arg0 ["Aligned", 4] : f32
   return
 }
 
 // -----
 
 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
 }
 
 // -----
 
 func.func @simple_store_missing_operand(%arg0 : f32) -> () {
   %0 = spv.Variable : !spv.ptr<f32, Function>

mlir/test/IR/invalid-locations.mlir

 ^bb:
   return loc(fused<true [unknown]) // expected-error {{expected '>' after fused location metadata}}
 }
 
 // -----
 
 func.func @location_fused_missing_metadata() {
 ^bb:
-  // expected-error@+1 {{expected non-function type}}
+  // expected-error@+1 {{expected attribute value}}
   return loc(fused<) // expected-error {{expected valid attribute metadata}}
 }
 
 // -----
 
 func.func @location_fused_missing_l_square() {
 ^bb:
   return loc(fused<true>unknown]) // expected-error {{expected '[' in fused location}}

mlir/test/IR/invalid.mlir

func.func @reference_to_iv_in_bound() {
   // expected-note@+1 {{previously referenced here}}
   affine.for %i0 = #map0(%i0) to 10 {
   }
 }
 
 // -----
 
 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}}
 
 // -----
 
 func.func @for_negative_stride() {
   affine.for %i = 1 to 10 step -1
 }        // expected-error@-1 {{expected step to be representable as a positive signed integer}}
 
 // -----
func.func @large_bound() {
   // expected-error@-1 {{integer constant out of range for attribute}}
   }
   return
 }
 
 // -----
 
 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
 }
 
 // -----
 
 func.func @step_typo() {
   affine.for %i = 1 to 100 step -- 1 { //expected-error {{expected constant integer}}

mlir/test/mlir-tblgen/attr-or-type-format.mlir

func.func private @test_ugly_attr_parser_dispatch() -> () attributes {
   // expected-error@+1 {{expected 'begin'}}
   attr = #test<"attr_ugly">
 }
 
 // -----
 
 func.func private @test_ugly_attr_missing_parameter() -> () attributes {
   // expected-error@+2 {{failed to parse TestAttrUgly parameter 'attr'}}
-  // expected-error@+1 {{expected non-function type}}
+  // expected-error@+1 {{expected attribute value}}
   attr = #test<"attr_ugly begin">
 }
 
 // -----
 
 func.func private @test_ugly_attr_missing_literal() -> () attributes {
   // expected-error@+1 {{expected 'end'}}
   attr = #test<"attr_ugly begin \"string_attr\"">

 func.func private @test_struct_not_enough_values() -> () attributes {
   // expected-error@+1 {{expected ','}}
   attr = #test.attr_with_format<42 : two = "foo">
 }
 
 // -----
 
 func.func private @test_parse_param_after_struct() -> () attributes {
-  // expected-error@+2 {{expected non-function type}}
+  // expected-error@+2 {{expected attribute value}}
   // expected-error@+1 {{failed to parse TestAttrWithFormat parameter 'three'}}
   attr = #test.attr_with_format<42 : two = "foo", four = [1, 2, 3] : >
 }
 
 // -----
 
 // expected-error@+1 {{expected '<'}}
 func.func private @test_invalid_type() -> !test.type_with_format