diff --git a/mlir/test/mlir-tblgen/op-format-spec.td b/mlir/test/mlir-tblgen/op-format-spec.td --- a/mlir/test/mlir-tblgen/op-format-spec.td +++ b/mlir/test/mlir-tblgen/op-format-spec.td @@ -320,13 +320,22 @@ //===----------------------------------------------------------------------===// // Test all of the valid literals. -// CHECK: error: expected valid literal +// CHECK: error: expected valid literal but got 'a:': keywords should contain only alphanum, '_', '$', or '.' characters def LiteralInvalidA : TestFormat_Op<[{ + `a:` +}]>; +// CHECK: error: expected valid literal but got '1': single character literal must be a letter or one of '_:,=<>()[]{}?+*' +def LiteralInvalidB : TestFormat_Op<[{ `1` }]>; +// CHECK: error: expected valid literal but got ':abc': valid keyword starts with a letter or '_' +def LiteralInvalidC : TestFormat_Op<[{ + `:abc` +}]>; + // CHECK: error: unexpected end of file in literal // CHECK: error: expected directive, literal, variable, or optional group -def LiteralInvalidB : TestFormat_Op<[{ +def LiteralInvalidD : TestFormat_Op<[{ ` }]>; // CHECK-NOT: error diff --git a/mlir/tools/mlir-tblgen/AttrOrTypeFormatGen.cpp b/mlir/tools/mlir-tblgen/AttrOrTypeFormatGen.cpp --- a/mlir/tools/mlir-tblgen/AttrOrTypeFormatGen.cpp +++ b/mlir/tools/mlir-tblgen/AttrOrTypeFormatGen.cpp @@ -606,8 +606,11 @@ /// Get the literal spelling without the surrounding "`". auto value = curToken.getSpelling().drop_front().drop_back(); - if (!isValidLiteral(value)) - return emitError("literal '" + value + "' is not valid"); + if (!isValidLiteral(value, [&](Twine diag) { + (void)emitError("expected valid literal but got '" + value + + "': " + diag); + })) + return failure(); consumeToken(); return {std::make_unique(value)}; diff --git a/mlir/tools/mlir-tblgen/FormatGen.h b/mlir/tools/mlir-tblgen/FormatGen.h --- a/mlir/tools/mlir-tblgen/FormatGen.h +++ b/mlir/tools/mlir-tblgen/FormatGen.h @@ -147,10 +147,13 @@ bool shouldEmitSpaceBefore(StringRef value, bool lastWasPunctuation); /// Returns true if the given string can be formatted as a keyword. -bool canFormatStringAsKeyword(StringRef value); +bool canFormatStringAsKeyword(StringRef value, + function_ref emitError = nullptr); /// Returns true if the given string is valid format literal element. -bool isValidLiteral(StringRef value); +/// If `emitError` is provided, it is invoked with the reason for the failure. +bool isValidLiteral(StringRef value, + function_ref emitError = nullptr); /// Whether a failure in parsing the assembly format should be a fatal error. extern llvm::cl::opt formatErrorIsFatal; diff --git a/mlir/tools/mlir-tblgen/FormatGen.cpp b/mlir/tools/mlir-tblgen/FormatGen.cpp --- a/mlir/tools/mlir-tblgen/FormatGen.cpp +++ b/mlir/tools/mlir-tblgen/FormatGen.cpp @@ -189,30 +189,50 @@ return !StringRef("<>(){}[],").contains(value.front()); } -bool mlir::tblgen::canFormatStringAsKeyword(StringRef value) { - if (!isalpha(value.front()) && value.front() != '_') +bool mlir::tblgen::canFormatStringAsKeyword( + StringRef value, function_ref emitError) { + if (!isalpha(value.front()) && value.front() != '_') { + if (emitError) + emitError("valid keyword starts with a letter or '_'"); return false; - return llvm::all_of(value.drop_front(), [](char c) { - return isalnum(c) || c == '_' || c == '$' || c == '.'; - }); + } + if (!llvm::all_of(value.drop_front(), [](char c) { + return isalnum(c) || c == '_' || c == '$' || c == '.'; + })) { + if (emitError) + emitError( + "keywords should contain only alphanum, '_', '$', or '.' characters"); + return false; + } + return true; } -bool mlir::tblgen::isValidLiteral(StringRef value) { - if (value.empty()) +bool mlir::tblgen::isValidLiteral(StringRef value, + function_ref emitError) { + if (value.empty()) { + if (emitError) + emitError("literal can't be empty"); return false; + } char front = value.front(); // If there is only one character, this must either be punctuation or a // single character bare identifier. - if (value.size() == 1) - return isalpha(front) || StringRef("_:,=<>()[]{}?+*").contains(front); - + if (value.size() == 1) { + StringRef bare = "_:,=<>()[]{}?+*"; + if (isalpha(front) || bare.contains(front)) + return true; + if (emitError) + emitError("single character literal must be a letter or one of '" + bare + + "'"); + return false; + } // Check the punctuation that are larger than a single character. if (value == "->") return true; // Otherwise, this must be an identifier. - return canFormatStringAsKeyword(value); + return canFormatStringAsKeyword(value, emitError); } //===----------------------------------------------------------------------===// diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp --- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp +++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp @@ -2724,9 +2724,12 @@ } // Check that the parsed literal is valid. - if (!isValidLiteral(value)) - return emitError(literalTok.getLoc(), "expected valid literal"); - + if (!isValidLiteral(value, [&](Twine diag) { + (void)emitError(literalTok.getLoc(), + "expected valid literal but got '" + value + + "': " + diag); + })) + return failure(); element = std::make_unique(value); return ::mlir::success(); }