Add parseBareIdentifier to AsmParser

[j2kun]

Removed the parseCaret after determining I can't make it work

[j2kun]

Revert ParserTest.cpp formatting change

[j2kun]

Still getting the hang of the arc CLI tool, sorry for the noise

[j2kun]

Remove caret token def

[j2kun]

Add optional variant

[rriddle]

I'm not really sure we need to expose this, why does parseKeyword not work for you?

[makslevental]

Seem eminently reasonable to me.

[j2kun]

In D156356#4540606, @rriddle wrote:

I'm not really sure we need to expose this, why does parseKeyword not work for you?

My mistake. I thought that parseKeyword only included reserved words as defined by TOK_KEYWORD, but it includes bare identifiers as well. I'll make sure that works for my needs and then abandon this patch if so.

[rriddle]

In D156356#4540639, @j2kun wrote:

In D156356#4540606, @rriddle wrote:

I'm not really sure we need to expose this, why does parseKeyword not work for you?

My mistake. I thought that parseKeyword only included reserved words as defined by TOK_KEYWORD, but it includes bare identifiers as well. I'll make sure that works for my needs and then abandon this patch if so.

Yeah, likely chose the wrong name a long time ago, but there is a separation between the lexer and what's exposed in the public API (we try not to expose many "lexer" specific things, like what is a keyword or bare identifier -> they are all keywords)

[j2kun]

Ah yes, now I see why parseKeyword isn't sufficient. I would like to have the bare_identifier be arbitrary, similar to how AffineMap allows for an arbitrary variable name.

The use case is for a polynomial dialect. I would like to have an attribute polynomial like #poly.polynomial<1 + x**1024> (note I can't use caret for the exponentiation op because it's handled specially in the lexer), but without requiring the specific variable name x. I suppose that not forcing the user to use x is a nice-to-have, but that's the discrepancy here.

[rriddle]

In D156356#4540730, @j2kun wrote:

Ah yes, now I see why parseKeyword isn't sufficient. I would like to have the bare_identifier be arbitrary, similar to how AffineMap allows for an arbitrary variable name.

The use case is for a polynomial dialect. I would like to have an attribute polynomial like #poly.polynomial<1 + x^1024>, but without requiring the specific variable name x. I suppose that not forcing the user to use x is a nice-to-have, but that's the discrepancy here.

What do you mean by arbitrary? There are several overloads, one takes a StringRef (i.e. an expected keyword), and one takes a StringRef * that parses any keyword.

[j2kun]

one takes a StringRef * that parses any keyword.

That was the trick, thanks! Abandoning this revision

mlir/include/mlir/IR/OpImplementation.h

public:
   virtual ParseResult parseOptionalStar() = 0;
 
   /// Parse a '|' token.
   virtual ParseResult parseVerticalBar() = 0;
 
   /// Parse a '|' token if present.
   virtual ParseResult parseOptionalVerticalBar() = 0;
 
+  /// Parse a bare identifier.
+  virtual ParseResult parseBareIdentifier(std::string *string) = 0;
+
+  /// Parse an optional bare identifier if present.
+  virtual OptionalParseResult
+  parseOptionalBareIdentifier(std::string *string) = 0;
+
   /// Parse a quoted string token.
   ParseResult parseString(std::string *string) {
     auto loc = getCurrentLocation();
     if (parseOptionalString(string))
       return emitError(loc, "expected string");
     return success();
   }
 
parseAffineExprOfSSAIds(SmallVectorImpl<UnresolvedOperand> &dimOperands,
                           SmallVectorImpl<UnresolvedOperand> &symbOperands,
                           AffineExpr &expr) = 0;
 
   //===--------------------------------------------------------------------===//
   // Argument Parsing
   //===--------------------------------------------------------------------===//
 
   struct Argument {
-    UnresolvedOperand ssaName;    // SourceLoc, SSA name, result #.
-    Type type;                    // Type.
-    DictionaryAttr attrs;         // Attributes if present.
+    UnresolvedOperand ssaName;         // SourceLoc, SSA name, result #.
+    Type type;                         // Type.
+    DictionaryAttr attrs;              // Attributes if present.
     std::optional<Location> sourceLoc; // Source location specifier if present.
   };
 
   /// Parse a single argument with the following syntax:
   ///
   ///   `%ssaName : !type { optionalAttrDict} loc(optionalSourceLoc)`
   ///
   /// If `allowType` is false or `allowAttrs` are false then the respective

mlir/lib/AsmParser/AsmParserImpl.h

ParseResult parseVerticalBar() override {
     return parser.parseToken(Token::vertical_bar, "expected '|'");
   }
 
   /// Parse a '|' token if present.
   ParseResult parseOptionalVerticalBar() override {
     return success(parser.consumeIf(Token::vertical_bar));
   }
 
+  /// Parse a bare identifier
+  ParseResult parseBareIdentifier(std::string *string) override {
+    if (!parser.getToken().is(Token::bare_identifier))
+      return emitError(getCurrentLocation(), "expected bare identifier");
+
+    if (string)
+      *string = parser.getToken().getBareIdentifier();
+    parser.consumeToken();
+    return success();
+  }
+
+  /// Parse an optional bare identifier, if present
+  OptionalParseResult
+  parseOptionalBareIdentifier(std::string *string) override {
+    if (parser.getToken().is(Token::bare_identifier))
+      return parseBareIdentifier(string);
+    return std::nullopt;
+  }
+
   /// Parses a quoted string token if present.
   ParseResult parseOptionalString(std::string *string) override {
     if (!parser.getToken().is(Token::string))
       return failure();
 
     if (string)
       *string = parser.getToken().getStringValue();
     parser.consumeToken();

mlir/lib/AsmParser/Token.h

#include "TokenKinds.def"
   /// string literal is a string starting with `0x` and only containing hex
   /// digits.
   std::optional<std::string> getHexStringValue() const;
 
   /// Given a token containing a symbol reference, return the unescaped string
   /// value.
   std::string getSymbolReference() const;
 
+  /// Given a token containing a bare identifier, return the unescaped string
+  /// value.
+  std::string getBareIdentifier() const;
+
   // Location processing.
   SMLoc getLoc() const;
   SMLoc getEndLoc() const;
   SMRange getLocRange() const;
 
   /// Given a punctuation or keyword token kind, return the spelling of the
   /// token as a string.  Warning: This will abort on markers, identifiers and
   /// literal tokens since they have no fixed spelling.

mlir/lib/AsmParser/Token.cpp

std::string Token::getSymbolReference() const {
   StringRef nameStr = getSpelling().drop_front();
 
   // Check to see if the reference is a string literal, or a bare identifier.
   if (nameStr.front() == '"')
     return getStringValue();
   return std::string(nameStr);
 }
 
+/// Given a token containing a bare identifier, return the unescaped string
+/// value.
+std::string Token::getBareIdentifier() const {
+  assert(is(Token::bare_identifier) && "expected bare identifier");
+  return std::string(getSpelling());
+}
+
 /// Given a hash_identifier token like #123, try to parse the number out of
 /// the identifier, returning std::nullopt if it is a named identifier like #x
 /// or if the integer doesn't fit.
 std::optional<unsigned> Token::getHashIdentifierNumber() const {
   assert(getKind() == hash_identifier);
   unsigned result = 0;
   if (spelling.drop_front().getAsInteger(10, result))
     return std::nullopt;

mlir/test/IR/parser.mlir

 
 // GENERIC-LABEL: parse_base64_test
 func.func @parse_base64_test() {
   // GENERIC: "test.parse_b64"() <{b64 = "hello world"}>
   test.parse_b64 "aGVsbG8gd29ybGQ="
   return
 }
 
+// GENERIC-LABEL: parse_bare_id
+func.func @parse_bare_id_test() {
+  // CHECK: test.parse_bare_id<foo>
+  test.parse_bare_id<foo>
+  return
+}
+
 // CHECK-LABEL: func @"\22_string_symbol_reference\22"
 func.func @"\"_string_symbol_reference\""() {
   // CHECK: ref = @"\22_string_symbol_reference\22"
   "foo.symbol_reference"() {ref = @"\"_string_symbol_reference\""} : () -> ()
   return
 }
 
 // CHECK-LABEL: func private @parse_opaque_attr_escape

mlir/test/lib/Dialect/Test/TestDialect.cpp

result.addAttribute("b64", parser.getBuilder().getStringAttr(
                                  StringRef(&bytes.front(), bytes.size())));
   return success();
 }
 
 void ParseB64BytesOp::print(OpAsmPrinter &p) {
   p << " \"" << llvm::encodeBase64(getB64()) << "\"";
 }
 
+ParseResult ParseBareIdOp::parse(OpAsmParser &parser, OperationState &result) {
+  if (parser.parseLess())
+    return failure();
+
+  std::string identifier;
+  if (parser.parseBareIdentifier(&identifier))
+    return failure();
+  if (parser.parseGreater())
+    return failure();
+
+  result.addAttribute("id", parser.getBuilder().getStringAttr(identifier));
+  return success();
+}
+
+void ParseBareIdOp::print(OpAsmPrinter &p) { p << "<" << getId() << ">"; }
+
 //===----------------------------------------------------------------------===//
 // Test WrapRegionOp - wrapping op exercising `parseGenericOperation()`.
 
 ParseResult WrappingRegionOp::parse(OpAsmParser &parser,
                                     OperationState &result) {
   if (parser.parseKeyword("wraps"))
     return failure();
 

mlir/test/lib/Dialect/Test/TestOps.td

 }
 
 def OpWithShapedTypeInferTypeInterfaceOp : TEST_Op<"op_with_shaped_type_infer_type_if",
       [InferTensorTypeWithReify]> {
   let arguments = (ins AnyTensor, AnyTensor);
   let results = (outs AnyTensor);
 }
 
-def OpWithShapedTypeInferTypeAdaptorInterfaceOp : 
+def OpWithShapedTypeInferTypeAdaptorInterfaceOp :
       TEST_Op<"op_with_shaped_type_infer_type_adaptor_if",
               [InferTensorTypeAdaptorWithReify]> {
   let arguments = (ins AnyTensor:$operand1, AnyTensor:$operand2);
   let results = (outs AnyTensor:$result);
 }
 
 def OpWithResultShapeInterfaceOp : TEST_Op<"op_with_result_shape_interface",
       [DeclareOpInterfaceMethods<InferShapedTypeOpInterface,
def ParseWrappedKeywordOp : TEST_Op<"parse_wrapped_keyword"> {
   let hasCustomAssemblyFormat = 1;
 }
 
 def ParseB64BytesOp : TEST_Op<"parse_b64"> {
   let arguments = (ins StrAttr:$b64);
   let hasCustomAssemblyFormat = 1;
 }
 
+def ParseBareIdOp : TEST_Op<"parse_bare_id"> {
+  let arguments = (ins StrAttr:$id);
+  let hasCustomAssemblyFormat = 1;
+}
+
 //===----------------------------------------------------------------------===//
 // Test region argument list parsing.
 
 def IsolatedRegionOp : TEST_Op<"isolated_region", [IsolatedFromAbove]> {
   let summary =  "isolated region operation";
   let description = [{
     Test op with an isolated region, to test passthrough region arguments. Each
     argument is of index type.
def TestCallAndStoreOp : TEST_Op<"call_and_store",
   let assemblyFormat =
     "$callee `(` $callee_operands `)` `,` $address attr-dict "
     "`:` functional-type(operands, results)";
 }
 
 def TestStoreWithARegion : TEST_Op<"store_with_a_region",
     [DeclareOpInterfaceMethods<RegionBranchOpInterface>,
      SingleBlock]> {
-  let arguments = (ins 
+  let arguments = (ins
     Arg<AnyMemRef, "", [MemWrite]>:$address,
     BoolAttr:$store_before_region
   );
   let regions = (region AnyRegion:$body);
   let assemblyFormat =
     "$address attr-dict-with-keyword regions `:` type($address)";
 }
 
 def TestStoreWithARegionTerminator : TEST_Op<"store_with_a_region_terminator",
     [DeclareOpInterfaceMethods<RegionBranchTerminatorOpInterface>, Terminator, NoMemoryEffect]> {
   let assemblyFormat = "attr-dict";
 }
 
 #endif // TEST_OPS