[mlir][AttrType] Emit unbalanced character errors using the last punctuation used

[rriddle]

Summary

This gives a better diagnostic in general, because it indicates that the user didn't
close out the last open puncutation range. For example:

foo.op {
  some.op -> !blah.pointer<
}

We want the error to hint about the unclosed <, not the } (which isn't really in the context of the type).

Depends on D140199

[jpienaar]

Nice, easier to read IMHO

[Mogball]

nuceu

mlir/lib/AsmParser/DialectSymbolParser.cpp

ParseResult Parser::parseDialectSymbolBody(StringRef &body,
   const char *curPtr = getTokenSpelling().data();
 
   // Scan over the nested punctuation, bailing out on error and consuming until
   // we find the end. We know that we're currently looking at the '<', so we can
   // go until we find the matching '>' character.
   assert(*curPtr == '<');
   SmallVector<char, 8> nestedPunctuation;
   const char *codeCompleteLoc = state.lex.getCodeCompleteLoc();
+
+  // Functor used to emit an unbalanced punctuation error.
+  auto emitPunctError = [&] {
+    return emitError() << "unbalanced '" << nestedPunctuation.back()
+                       << "' character in pretty dialect name";
+  };
+  // Functor used to check for unbalanced punctuation.
+  auto checkNestedPunctuation = [&](char expectedToken) -> ParseResult {
+    if (nestedPunctuation.back() != expectedToken)
+      return emitPunctError();
+    nestedPunctuation.pop_back();
+    return success();
+  };
   do {
     // Handle code completions, which may appear in the middle of the symbol
     // body.
     if (curPtr == codeCompleteLoc) {
       isCodeCompletion = true;
       nestedPunctuation.clear();
       break;
     }
 
     char c = *curPtr++;
     switch (c) {
     case '\0':
       // This also handles the EOF case.
-      if (!nestedPunctuation.empty()) {
-        return emitError() << "unbalanced '" << nestedPunctuation.back()
-                           << "' character in pretty dialect name";
-      }
+      if (!nestedPunctuation.empty())
+        return emitPunctError();
       return emitError("unexpected nul or EOF in pretty dialect name");
     case '<':
     case '[':
     case '(':
     case '{':
       nestedPunctuation.push_back(c);
       continue;
 
     case '-':
       // The sequence `->` is treated as special token.
       if (*curPtr == '>')
         ++curPtr;
       continue;
 
     case '>':
-      if (nestedPunctuation.pop_back_val() != '<')
-        return emitError("unbalanced '>' character in pretty dialect name");
+      if (failed(checkNestedPunctuation('<')))
+        return failure();
       break;
     case ']':
-      if (nestedPunctuation.pop_back_val() != '[')
-        return emitError("unbalanced ']' character in pretty dialect name");
+      if (failed(checkNestedPunctuation('[')))
+        return failure();
       break;
     case ')':
-      if (nestedPunctuation.pop_back_val() != '(')
-        return emitError("unbalanced ')' character in pretty dialect name");
+      if (failed(checkNestedPunctuation('(')))
+        return failure();
       break;
     case '}':
-      if (nestedPunctuation.pop_back_val() != '{')
-        return emitError("unbalanced '}' character in pretty dialect name");
+      if (failed(checkNestedPunctuation('{')))
+        return failure();
       break;
     case '"': {
       // Dispatch to the lexer to lex past strings.
       resetToken(curPtr - 1);
       curPtr = state.curToken.getEndLoc().getPointer();
 
       // Handle code completions, which may appear in the middle of the symbol
       // body.

mlir/test/Dialect/SPIRV/IR/types.mlir

 
 // -----
 
 // expected-error @+1 {{expected ')'}}
 func.func private @struct_type_missing_comma2(!spirv.struct<(f32 [0] i32)>) -> ()
 
 // -----
 
-//  expected-error @+1 {{unbalanced ')' character in pretty dialect name}}
+//  expected-error @+1 {{unbalanced '[' character in pretty dialect name}}
 func.func private @struct_type_neg_offset(!spirv.struct<(f32 [0)>) -> ()
 
 // -----
 
-//  expected-error @+1 {{unbalanced ']' character in pretty dialect name}}
+//  expected-error @+1 {{unbalanced '(' character in pretty dialect name}}
 func.func private @struct_type_neg_offset(!spirv.struct<(f32 0])>) -> ()
 
 // -----
 
 //  expected-error @+1 {{expected ']'}}
 func.func private @struct_type_neg_offset(!spirv.struct<(f32 [NonWritable 0])>) -> ()
 
 // -----

 
 // -----
 
 // expected-error @+1 {{expected '<'}}
 func.func private @matrix_invalid_format(!spirv.matrix 3 x vector<3xf32>>) -> ()
 
 // -----
 
-// expected-error @+1 {{unbalanced ')' character in pretty dialect name}}
+// expected-error @+1 {{unbalanced '<' character in pretty dialect name}}
 func.func private @matrix_invalid_format(!spirv.matrix< 3 x vector<3xf32>) -> ()
 
 // -----
 
 // expected-error @+1 {{expected 'x' in dimension list}}
 func.func private @matrix_invalid_format(!spirv.matrix<2 vector<3xi32>>) -> ()
 
 // -----

mlir/test/IR/invalid.mlir

 // -----
 
 func.func @dialect_type_empty_namespace(!<"">) -> () { // expected-error {{invalid type identifier}}
   return
 }
 
 // -----
 
-func.func @dialect_type_missing_greater(!foo<) -> () { // expected-error {{unbalanced ')' character in pretty dialect name}}
+func.func @dialect_type_missing_greater(!foo<) -> () { // expected-error {{unbalanced '<' character in pretty dialect name}}
   return
 
 // -----
 
 func.func @type_alias_unknown(!unknown_alias) -> () { // expected-error {{undefined symbol alias id 'unknown_alias'}}
   return
 }
 
^bb2:
     %1 = arith.constant 0 : i32
     "foo.yield" () : () -> ()
   }) : () -> ()
   return
 }
 
 // -----
 
-// expected-error @+1 {{unbalanced ']' character in pretty dialect name}}
+// expected-error @+1 {{unbalanced '<' character in pretty dialect name}}
 func.func @invalid_unknown_type_dialect_name() -> !invalid.dialect<!x@#]!@#>
 
 // -----
 
 // expected-error @+1 {{expected '<' in tuple type}}
 func.func @invalid_tuple_missing_less(tuple i32>)
 
 // -----
%def = "test.ssacfg_region"() ({
     "foo.use" (%def) : (i32) -> ()
     "foo.yield" () : () -> ()
   }) : () -> (i32)
   return
 }
 
 // -----
 
-// expected-error @+1 {{unbalanced ')' character in pretty dialect name}}
+// expected-error @+1 {{unbalanced '<' character in pretty dialect name}}
 func.func @bad_arrow(%arg : !unreg.ptr<(i32)->)
 
 // -----
 
 // expected-error @+1 {{attribute 'attr' occurs more than once in the attribute list}}
 test.format_symbol_name_attr_op @name { attr = "xx" }
 
 // -----