[Mlir-commits] [mlir] 86445e8 - [AsmParser] Adopt emitWrongTokenError more, improving QoI
Chris Lattner
llvmlistbot at llvm.org
Wed May 11 12:43:14 PDT 2022
Author: Chris Lattner
Date: 2022-05-11T20:41:12+01:00
New Revision: 86445e8c63c7a15456149c88b99af03933268c5d
URL: https://github.com/llvm/llvm-project/commit/86445e8c63c7a15456149c88b99af03933268c5d
DIFF: https://github.com/llvm/llvm-project/commit/86445e8c63c7a15456149c88b99af03933268c5d.diff
LOG: [AsmParser] Adopt emitWrongTokenError more, improving QoI
This is a full audit of emitError calls, I took the opportunity
to remove extranous parens and fix a couple cases where we'd
generate multiple diagnostics for the same error.
Differential Revision: https://reviews.llvm.org/D125355
Added:
Modified:
mlir/lib/Parser/AffineParser.cpp
mlir/lib/Parser/AttributeParser.cpp
mlir/lib/Parser/DialectSymbolParser.cpp
mlir/lib/Parser/LocationParser.cpp
mlir/lib/Parser/TypeParser.cpp
mlir/test/Dialect/SPIRV/IR/glsl-ops.mlir
mlir/test/Dialect/SPIRV/IR/logical-ops.mlir
mlir/test/Dialect/SPIRV/IR/misc-ops.mlir
mlir/test/Dialect/SPIRV/IR/ocl-ops.mlir
mlir/test/IR/invalid-locations.mlir
mlir/test/IR/invalid.mlir
Removed:
################################################################################
diff --git a/mlir/lib/Parser/AffineParser.cpp b/mlir/lib/Parser/AffineParser.cpp
index 57a2fc296d5fe..f8181cc39bb62 100644
--- a/mlir/lib/Parser/AffineParser.cpp
+++ b/mlir/lib/Parser/AffineParser.cpp
@@ -236,12 +236,10 @@ AffineExpr AffineParser::parseParentheticalExpr() {
if (parseToken(Token::l_paren, "expected '('"))
return nullptr;
if (getToken().is(Token::r_paren))
- return (emitError("no expression inside parentheses"), nullptr);
+ return emitError("no expression inside parentheses"), nullptr;
auto expr = parseAffineExpr();
- if (!expr)
- return nullptr;
- if (parseToken(Token::r_paren, "expected ')'"))
+ if (!expr || parseToken(Token::r_paren, "expected ')'"))
return nullptr;
return expr;
@@ -261,7 +259,7 @@ AffineExpr AffineParser::parseNegateExpression(AffineExpr lhs) {
if (!operand)
// Extra error message although parseAffineOperandExpr would have
// complained. Leads to a better diagnostic.
- return (emitError("missing operand of negation"), nullptr);
+ return emitError("missing operand of negation"), nullptr;
return (-1) * operand;
}
@@ -270,7 +268,7 @@ AffineExpr AffineParser::parseNegateExpression(AffineExpr lhs) {
/// affine-expr ::= bare-id
AffineExpr AffineParser::parseBareIdExpr() {
if (getToken().isNot(Token::bare_identifier))
- return (emitError("expected bare identifier"), nullptr);
+ return emitWrongTokenError("expected bare identifier"), nullptr;
StringRef sRef = getTokenSpelling();
for (auto entry : dimsAndSymbols) {
@@ -280,15 +278,15 @@ AffineExpr AffineParser::parseBareIdExpr() {
}
}
- return (emitError("use of undeclared identifier"), nullptr);
+ return emitWrongTokenError("use of undeclared identifier"), nullptr;
}
/// Parse an SSA id which may appear in an affine expression.
AffineExpr AffineParser::parseSSAIdExpr(bool isSymbol) {
if (!allowParsingSSAIds)
- return (emitError("unexpected ssa identifier"), nullptr);
+ return emitWrongTokenError("unexpected ssa identifier"), nullptr;
if (getToken().isNot(Token::percent_identifier))
- return (emitError("expected ssa identifier"), nullptr);
+ return emitWrongTokenError("expected ssa identifier"), nullptr;
auto name = getTokenSpelling();
// Check if we already parsed this SSA id.
for (auto entry : dimsAndSymbols) {
@@ -299,7 +297,7 @@ AffineExpr AffineParser::parseSSAIdExpr(bool isSymbol) {
}
// Parse the SSA id and add an AffineDim/SymbolExpr to represent it.
if (parseElement(isSymbol))
- return (emitError("failed to parse ssa identifier"), nullptr);
+ return nullptr;
auto idExpr = isSymbol
? getAffineSymbolExpr(numSymbolOperands++, getContext())
: getAffineDimExpr(numDimOperands++, getContext());
@@ -325,7 +323,7 @@ AffineExpr AffineParser::parseSymbolSSAIdExpr() {
AffineExpr AffineParser::parseIntegerExpr() {
auto val = getToken().getUInt64IntegerValue();
if (!val.hasValue() || (int64_t)val.getValue() < 0)
- return (emitError("constant too large for index"), nullptr);
+ return emitError("constant too large for index"), nullptr;
consumeToken(Token::integer);
return builder.getAffineConstantExpr((int64_t)val.getValue());
@@ -459,7 +457,7 @@ AffineExpr AffineParser::parseAffineExpr() {
/// dimensional/symbolic identifier.
ParseResult AffineParser::parseIdentifierDefinition(AffineExpr idExpr) {
if (getToken().isNot(Token::bare_identifier))
- return emitError("expected bare identifier");
+ return emitWrongTokenError("expected bare identifier");
auto name = getTokenSpelling();
for (auto entry : dimsAndSymbols) {
@@ -520,9 +518,9 @@ ParseResult AffineParser::parseAffineMapOrIntegerSetInline(AffineMap &map,
// be parsing an integer set attribute or an affine map attribute.
bool isArrow = getToken().is(Token::arrow);
bool isColon = getToken().is(Token::colon);
- if (!isArrow && !isColon) {
- return emitError("expected '->' or ':'");
- }
+ if (!isArrow && !isColon)
+ return emitWrongTokenError("expected '->' or ':'");
+
if (isArrow) {
parseToken(Token::arrow, "expected '->' or '['");
map = parseAffineMapRange(numDims, numSymbols);
@@ -616,7 +614,7 @@ AffineExpr AffineParser::parseAffineConstraint(bool *isEq) {
*isEq = false;
return expr;
}
- return (emitError("expected '0' after '>='"), nullptr);
+ return emitError("expected '0' after '>='"), nullptr;
}
if (consumeIf(Token::equal) && consumeIf(Token::equal) &&
@@ -627,11 +625,11 @@ AffineExpr AffineParser::parseAffineConstraint(bool *isEq) {
*isEq = true;
return expr;
}
- return (emitError("expected '0' after '=='"), nullptr);
+ return emitError("expected '0' after '=='"), nullptr;
}
- return (emitError("expected '== 0' or '>= 0' at end of affine constraint"),
- nullptr);
+ return emitError("expected '== 0' or '>= 0' at end of affine constraint"),
+ nullptr;
}
/// Parse the constraints that are part of an integer set definition.
diff --git a/mlir/lib/Parser/AttributeParser.cpp b/mlir/lib/Parser/AttributeParser.cpp
index d891595658039..0161a3c151209 100644
--- a/mlir/lib/Parser/AttributeParser.cpp
+++ b/mlir/lib/Parser/AttributeParser.cpp
@@ -114,7 +114,8 @@ Attribute Parser::parseAttribute(Type type) {
if (getToken().is(Token::floatliteral))
return parseFloatAttr(type, /*isNegative=*/true);
- return (emitError("expected constant integer or floating point value"),
+ return (emitWrongTokenError(
+ "expected constant integer or floating point value"),
nullptr);
}
@@ -211,7 +212,7 @@ Attribute Parser::parseAttribute(Type type) {
Type type;
OptionalParseResult result = parseOptionalType(type);
if (!result.hasValue())
- return emitError("expected attribute value"), Attribute();
+ return emitWrongTokenError("expected attribute value"), Attribute();
return failed(*result) ? Attribute() : TypeAttr::get(type);
}
}
@@ -275,7 +276,7 @@ ParseResult Parser::parseAttributeDict(NamedAttrList &attributes) {
getToken().isKeyword())
nameId = builder.getStringAttr(getTokenSpelling());
else
- return emitError("expected attribute name");
+ return emitWrongTokenError("expected attribute name");
if (nameId->size() == 0)
return emitError("expected valid attribute name");
diff --git a/mlir/lib/Parser/DialectSymbolParser.cpp b/mlir/lib/Parser/DialectSymbolParser.cpp
index c72e720f4a7f5..5c5f3801e3391 100644
--- a/mlir/lib/Parser/DialectSymbolParser.cpp
+++ b/mlir/lib/Parser/DialectSymbolParser.cpp
@@ -134,7 +134,8 @@ static Symbol parseExtendedSymbol(Parser &p, Token::Kind identifierTok,
// Check for an alias for this type.
auto aliasIt = aliases.find(identifier);
if (aliasIt == aliases.end())
- return (p.emitError("undefined symbol alias id '" + identifier + "'"),
+ return (p.emitWrongTokenError("undefined symbol alias id '" + identifier +
+ "'"),
nullptr);
return aliasIt->second;
}
@@ -153,7 +154,8 @@ static Symbol parseExtendedSymbol(Parser &p, Token::Kind identifierTok,
// Parse the symbol specific data.
if (p.getToken().isNot(Token::string))
- return (p.emitError("expected string literal data in dialect symbol"),
+ return (p.emitWrongTokenError(
+ "expected string literal data in dialect symbol"),
nullptr);
symbolData = p.getToken().getStringValue();
loc = SMLoc::getFromPointer(p.getToken().getLoc().getPointer() + 1);
diff --git a/mlir/lib/Parser/LocationParser.cpp b/mlir/lib/Parser/LocationParser.cpp
index c81d0d673951d..fe365171a5756 100644
--- a/mlir/lib/Parser/LocationParser.cpp
+++ b/mlir/lib/Parser/LocationParser.cpp
@@ -41,7 +41,7 @@ ParseResult Parser::parseCallSiteLocation(LocationAttr &loc) {
// Parse the 'at'.
if (getToken().isNot(Token::bare_identifier) ||
getToken().getSpelling() != "at")
- return emitError("expected 'at' in callsite location");
+ return emitWrongTokenError("expected 'at' in callsite location");
consumeToken(Token::bare_identifier);
// Parse the caller location.
@@ -66,7 +66,8 @@ ParseResult Parser::parseFusedLocation(LocationAttr &loc) {
if (consumeIf(Token::less)) {
metadata = parseAttribute();
if (!metadata)
- return emitError("expected valid attribute metadata");
+ return failure();
+
// Parse the '>' token.
if (parseToken(Token::greater,
"expected '>' after fused location metadata"))
@@ -100,10 +101,12 @@ ParseResult Parser::parseNameOrFileLineColLocation(LocationAttr &loc) {
if (consumeIf(Token::colon)) {
// Parse the line number.
if (getToken().isNot(Token::integer))
- return emitError("expected integer line number in FileLineColLoc");
+ return emitWrongTokenError(
+ "expected integer line number in FileLineColLoc");
auto line = getToken().getUnsignedIntegerValue();
if (!line.hasValue())
- return emitError("expected integer line number in FileLineColLoc");
+ return emitWrongTokenError(
+ "expected integer line number in FileLineColLoc");
consumeToken(Token::integer);
// Parse the ':'.
@@ -112,7 +115,8 @@ ParseResult Parser::parseNameOrFileLineColLocation(LocationAttr &loc) {
// Parse the column number.
if (getToken().isNot(Token::integer))
- return emitError("expected integer column number in FileLineColLoc");
+ return emitWrongTokenError(
+ "expected integer column number in FileLineColLoc");
auto column = getToken().getUnsignedIntegerValue();
if (!column.hasValue())
return emitError("expected integer column number in FileLineColLoc");
@@ -151,7 +155,7 @@ ParseResult Parser::parseLocationInstance(LocationAttr &loc) {
// Bare tokens required for other cases.
if (!getToken().is(Token::bare_identifier))
- return emitError("expected location instance");
+ return emitWrongTokenError("expected location instance");
// Check for the 'callsite' signifying a callsite location.
if (getToken().getSpelling() == "callsite")
@@ -168,5 +172,5 @@ ParseResult Parser::parseLocationInstance(LocationAttr &loc) {
return success();
}
- return emitError("expected location instance");
+ return emitWrongTokenError("expected location instance");
}
diff --git a/mlir/lib/Parser/TypeParser.cpp b/mlir/lib/Parser/TypeParser.cpp
index 59ee740661286..01cba54bc980b 100644
--- a/mlir/lib/Parser/TypeParser.cpp
+++ b/mlir/lib/Parser/TypeParser.cpp
@@ -154,23 +154,21 @@ ParseResult Parser::parseStridedLayout(int64_t &offset,
SmallVectorImpl<int64_t> &strides) {
// Parse offset.
consumeToken(Token::kw_offset);
- if (!consumeIf(Token::colon))
- return emitError("expected colon after `offset` keyword");
+ if (parseToken(Token::colon, "expected colon after `offset` keyword"))
+ return failure();
+
auto maybeOffset = getToken().getUnsignedIntegerValue();
bool question = getToken().is(Token::question);
if (!maybeOffset && !question)
- return emitError("invalid offset");
+ return emitWrongTokenError("invalid offset");
offset = maybeOffset ? static_cast<int64_t>(maybeOffset.getValue())
: MemRefType::getDynamicStrideOrOffset();
consumeToken();
- if (!consumeIf(Token::comma))
- return emitError("expected comma after offset value");
-
// Parse stride list.
- if (parseToken(Token::kw_strides,
+ if (parseToken(Token::comma, "expected comma after offset value") ||
+ parseToken(Token::kw_strides,
"expected `strides` keyword after offset specification") ||
-
parseToken(Token::colon, "expected colon after `strides` keyword") ||
parseStrideList(strides))
return failure();
@@ -298,7 +296,7 @@ Type Parser::parseMemRefType() {
Type Parser::parseNonFunctionType() {
switch (getToken().getKind()) {
default:
- return (emitError("expected non-function type"), nullptr);
+ return (emitWrongTokenError("expected non-function type"), nullptr);
case Token::kw_memref:
return parseMemRefType();
case Token::kw_tensor:
@@ -508,9 +506,7 @@ Parser::parseVectorDimensionList(SmallVectorImpl<int64_t> &dimensions,
// Check if we have reached the end of the scalable dimension list
if (consumeIf(Token::r_square)) {
// Make sure we have something like 'xbf32'.
- if (parseXInDimensionList())
- return failure();
- return success();
+ return parseXInDimensionList();
}
// Make sure we have an 'x'
if (parseXInDimensionList())
@@ -518,7 +514,8 @@ Parser::parseVectorDimensionList(SmallVectorImpl<int64_t> &dimensions,
}
// If we make it here, we've finished parsing the dimension list
// without finding ']' closing the set of scalable dimensions
- return emitError("missing ']' closing set of scalable dimensions");
+ return emitWrongTokenError(
+ "missing ']' closing set of scalable dimensions");
}
return success();
@@ -538,9 +535,10 @@ ParseResult
Parser::parseDimensionListRanked(SmallVectorImpl<int64_t> &dimensions,
bool allowDynamic) {
while (getToken().isAny(Token::integer, Token::question)) {
+ auto loc = getToken().getLoc();
if (consumeIf(Token::question)) {
if (!allowDynamic)
- return emitError("expected static shape");
+ return emitError(loc, "expected static shape");
dimensions.push_back(-1);
} else {
int64_t value;
@@ -586,7 +584,7 @@ ParseResult Parser::parseIntegerInDimensionList(int64_t &value) {
/// token.
ParseResult Parser::parseXInDimensionList() {
if (getToken().isNot(Token::bare_identifier) || getTokenSpelling()[0] != 'x')
- return emitError("expected 'x' in dimension list");
+ return emitWrongTokenError("expected 'x' in dimension list");
// If we had a prefix of 'x', lex the next token immediately after the 'x'.
if (getTokenSpelling().size() != 1)
diff --git a/mlir/test/Dialect/SPIRV/IR/glsl-ops.mlir b/mlir/test/Dialect/SPIRV/IR/glsl-ops.mlir
index ffaaf4d5833cd..e253a1a74a705 100644
--- a/mlir/test/Dialect/SPIRV/IR/glsl-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/glsl-ops.mlir
@@ -43,7 +43,7 @@ func.func @exp(%arg0 : f32, %arg1 : f32) -> () {
// -----
func.func @exp(%arg0 : i32) -> () {
- // expected-error @+2 {{expected non-function type}}
+ // expected-error @+1 {{expected non-function type}}
%2 = spv.GLSL.Exp %arg0 :
return
}
diff --git a/mlir/test/Dialect/SPIRV/IR/logical-ops.mlir b/mlir/test/Dialect/SPIRV/IR/logical-ops.mlir
index a4d5eb3cd868e..1c462b54888d6 100644
--- a/mlir/test/Dialect/SPIRV/IR/logical-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/logical-ops.mlir
@@ -99,7 +99,7 @@ func.func @logicalBinary(%arg0 : i1, %arg1 : i1)
func.func @logicalBinary(%arg0 : i1, %arg1 : i1)
{
- // expected-error @+2 {{expected non-function type}}
+ // expected-error @+1 {{expected non-function type}}
%0 = spv.LogicalAnd %arg0, %arg1 :
return
}
@@ -148,7 +148,7 @@ func.func @logicalUnary(%arg0 : i1)
func.func @logicalUnary(%arg0 : i1)
{
- // expected-error @+2 {{expected non-function type}}
+ // expected-error @+1 {{expected non-function type}}
%0 = spv.LogicalNot %arg0 :
return
}
diff --git a/mlir/test/Dialect/SPIRV/IR/misc-ops.mlir b/mlir/test/Dialect/SPIRV/IR/misc-ops.mlir
index bdf779dfb20ee..ff9cb6d5e602f 100644
--- a/mlir/test/Dialect/SPIRV/IR/misc-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/misc-ops.mlir
@@ -15,7 +15,7 @@ func.func @undef() -> () {
// -----
func.func @undef() -> () {
- // expected-error @+2{{expected non-function type}}
+ // expected-error @+1{{expected non-function type}}
%0 = spv.Undef :
spv.Return
}
diff --git a/mlir/test/Dialect/SPIRV/IR/ocl-ops.mlir b/mlir/test/Dialect/SPIRV/IR/ocl-ops.mlir
index 4aa1e87eece5d..b904c2adf1dd1 100644
--- a/mlir/test/Dialect/SPIRV/IR/ocl-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/ocl-ops.mlir
@@ -43,7 +43,7 @@ func.func @exp(%arg0 : f32, %arg1 : f32) -> () {
// -----
func.func @exp(%arg0 : i32) -> () {
- // expected-error @+2 {{expected non-function type}}
+ // expected-error @+1 {{expected non-function type}}
%2 = spv.OCL.exp %arg0 :
return
}
@@ -99,7 +99,7 @@ func.func @fabs(%arg0 : f32, %arg1 : f32) -> () {
// -----
func.func @fabs(%arg0 : i32) -> () {
- // expected-error @+2 {{expected non-function type}}
+ // expected-error @+1 {{expected non-function type}}
%2 = spv.OCL.fabs %arg0 :
return
}
@@ -161,7 +161,7 @@ func.func @sabs(%arg0 : i32, %arg1 : i32) -> () {
// -----
func.func @sabs(%arg0 : i32) -> () {
- // expected-error @+2 {{expected non-function type}}
+ // expected-error @+1 {{expected non-function type}}
%2 = spv.OCL.s_abs %arg0 :
return
}
diff --git a/mlir/test/IR/invalid-locations.mlir b/mlir/test/IR/invalid-locations.mlir
index 6993e15597f64..fb5b80fe37767 100644
--- a/mlir/test/IR/invalid-locations.mlir
+++ b/mlir/test/IR/invalid-locations.mlir
@@ -75,7 +75,7 @@ func.func @location_fused_missing_greater() {
func.func @location_fused_missing_metadata() {
^bb:
// expected-error at +1 {{expected attribute value}}
- return loc(fused<) // expected-error {{expected valid attribute metadata}}
+ return loc(fused<)
}
// -----
diff --git a/mlir/test/IR/invalid.mlir b/mlir/test/IR/invalid.mlir
index a7448d790a528..59fe5ef6c23da 100644
--- a/mlir/test/IR/invalid.mlir
+++ b/mlir/test/IR/invalid.mlir
@@ -882,7 +882,7 @@ func.func @type_alias_unknown(!unknown_alias) -> () { // expected-error {{undefi
// -----
-!missing_type_alias = type // expected-error at +1 {{expected non-function type}}
+!missing_type_alias = type // expected-error {{expected non-function type}}
// -----
@@ -1690,4 +1690,11 @@ func.func @error_at_end_of_line() {
@foo // expected-error {{expected operation name in quotes}}
+// -----
+
+func.func @func() {
+ %c0 = arith.constant // expected-error {{expected attribute value}}
+
+ %x = arith.constant 1 : i32
+}
More information about the Mlir-commits
mailing list