[MLIR] Imporve location tracking for parseElementsLiteralType
#127992
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit improves line location tracking in case of error reporting to the
user in `parseElementsLiteralType`. There are two cases: the type is already
parsed [1] or not yet parsed [2]. We print the error at the attribute's location
in both cases to ensure consistency.
Case 1)
```mlir
memref<i32> = dense<[3]>
^
```
Case 2)
```mlir
dense<[3]> : memref<i32>
^
```
|
@llvm/pr-subscribers-mlir Author: lorenzo chelini (chelini) ChangesThis commit improves line location tracking in case of error reporting to the user in Case 1) memref<i32> = dense<[3]>
^Case 2) dense<[3]> : memref<i32>
^Note that today for a simple: func.func @<!-- -->main() {
%0 = arith.constant dense<[3]> : i32
return
}we print the error after the constant: Full diff: https://github.com/llvm/llvm-project/pull/127992.diff 3 Files Affected:
diff --git a/mlir/lib/AsmParser/AttributeParser.cpp b/mlir/lib/AsmParser/AttributeParser.cpp
index ff616dac9625b..32c68e093a0b0 100644
--- a/mlir/lib/AsmParser/AttributeParser.cpp
+++ b/mlir/lib/AsmParser/AttributeParser.cpp
@@ -951,14 +951,10 @@ Attribute Parser::parseDenseElementsAttr(Type attrType) {
return nullptr;
}
- // If the type is specified `parseElementsLiteralType` will not parse a type.
- // Use the attribute location as the location for error reporting in that
- // case.
- auto loc = attrType ? attribLoc : getToken().getLoc();
- auto type = parseElementsLiteralType(attrType);
+ auto type = parseElementsLiteralType(attribLoc, attrType);
if (!type)
return nullptr;
- return literalParser.getAttr(loc, type);
+ return literalParser.getAttr(attribLoc, type);
}
Attribute Parser::parseDenseResourceElementsAttr(Type attrType) {
@@ -999,7 +995,7 @@ Attribute Parser::parseDenseResourceElementsAttr(Type attrType) {
/// elements-literal-type ::= vector-type | ranked-tensor-type
///
/// This method also checks the type has static shape.
-ShapedType Parser::parseElementsLiteralType(Type type) {
+ShapedType Parser::parseElementsLiteralType(SMLoc loc, Type type) {
// If the user didn't provide a type, parse the colon type for the literal.
if (!type) {
if (parseToken(Token::colon, "expected ':'"))
@@ -1010,12 +1006,14 @@ ShapedType Parser::parseElementsLiteralType(Type type) {
auto sType = dyn_cast<ShapedType>(type);
if (!sType) {
- emitError("elements literal must be a shaped type");
+ emitError(loc, "elements literal must be a shaped type");
return nullptr;
}
- if (!sType.hasStaticShape())
- return (emitError("elements literal type must have static shape"), nullptr);
+ if (!sType.hasStaticShape()) {
+ emitError(loc, "elements literal type must have static shape");
+ return nullptr;
+ }
return sType;
}
@@ -1032,7 +1030,7 @@ Attribute Parser::parseSparseElementsAttr(Type attrType) {
// of the type.
Type indiceEltType = builder.getIntegerType(64);
if (consumeIf(Token::greater)) {
- ShapedType type = parseElementsLiteralType(attrType);
+ ShapedType type = parseElementsLiteralType(loc, attrType);
if (!type)
return nullptr;
@@ -1065,7 +1063,7 @@ Attribute Parser::parseSparseElementsAttr(Type attrType) {
if (parseToken(Token::greater, "expected '>'"))
return nullptr;
- auto type = parseElementsLiteralType(attrType);
+ auto type = parseElementsLiteralType(loc, attrType);
if (!type)
return nullptr;
diff --git a/mlir/lib/AsmParser/Parser.h b/mlir/lib/AsmParser/Parser.h
index 1b8aa7c9dce6f..ecc128cf767b3 100644
--- a/mlir/lib/AsmParser/Parser.h
+++ b/mlir/lib/AsmParser/Parser.h
@@ -288,7 +288,7 @@ class Parser {
/// Parse a dense elements attribute.
Attribute parseDenseElementsAttr(Type attrType);
- ShapedType parseElementsLiteralType(Type type);
+ ShapedType parseElementsLiteralType(SMLoc loc, Type type);
/// Parse a dense resource elements attribute.
Attribute parseDenseResourceElementsAttr(Type attrType);
diff --git a/mlir/test/IR/invalid-builtin-attributes.mlir b/mlir/test/IR/invalid-builtin-attributes.mlir
index 5098fe751fd01..64201950b2306 100644
--- a/mlir/test/IR/invalid-builtin-attributes.mlir
+++ b/mlir/test/IR/invalid-builtin-attributes.mlir
@@ -591,3 +591,38 @@ func.func @duplicate_dictionary_attr_key() {
#attr1 = distinct[0]<43 : i32>
// -----
+
+// Make sure the error is not printed on the return.
+func.func @print_error_on_correct_line() {
+ %0 = arith.constant
+ // expected-error@below {{elements literal must be a shaped type}}
+ dense<[3]> : i32
+ return
+}
+
+// -----
+
+// Make sure the error is not printed on the return.
+func.func @print_error_on_correct_line() {
+ %0 = arith.constant
+ // expected-error@below {{elements literal must be a shaped type}}
+ sparse<
+ [
+ [0, 1, 2, 3],
+ [1, 1, 2, 3],
+ [1, 2, 2, 3],
+ [1, 2, 3, 4]
+ ],
+ [1, 1, 1, 1] > : i32
+ return
+}
+
+// -----
+
+// Make sure the error is not printed on the return.
+func.func @print_error_on_correct_line() {
+ %0 = arith.constant
+ // expected-error@below {{elements literal must be a shaped type}}
+ sparse <> : i32
+ return
+}
|
|
@llvm/pr-subscribers-mlir-core Author: lorenzo chelini (chelini) ChangesThis commit improves line location tracking in case of error reporting to the user in Case 1) memref<i32> = dense<[3]>
^Case 2) dense<[3]> : memref<i32>
^Note that today for a simple: func.func @<!-- -->main() {
%0 = arith.constant dense<[3]> : i32
return
}we print the error after the constant: Full diff: https://github.com/llvm/llvm-project/pull/127992.diff 3 Files Affected:
diff --git a/mlir/lib/AsmParser/AttributeParser.cpp b/mlir/lib/AsmParser/AttributeParser.cpp
index ff616dac9625b..32c68e093a0b0 100644
--- a/mlir/lib/AsmParser/AttributeParser.cpp
+++ b/mlir/lib/AsmParser/AttributeParser.cpp
@@ -951,14 +951,10 @@ Attribute Parser::parseDenseElementsAttr(Type attrType) {
return nullptr;
}
- // If the type is specified `parseElementsLiteralType` will not parse a type.
- // Use the attribute location as the location for error reporting in that
- // case.
- auto loc = attrType ? attribLoc : getToken().getLoc();
- auto type = parseElementsLiteralType(attrType);
+ auto type = parseElementsLiteralType(attribLoc, attrType);
if (!type)
return nullptr;
- return literalParser.getAttr(loc, type);
+ return literalParser.getAttr(attribLoc, type);
}
Attribute Parser::parseDenseResourceElementsAttr(Type attrType) {
@@ -999,7 +995,7 @@ Attribute Parser::parseDenseResourceElementsAttr(Type attrType) {
/// elements-literal-type ::= vector-type | ranked-tensor-type
///
/// This method also checks the type has static shape.
-ShapedType Parser::parseElementsLiteralType(Type type) {
+ShapedType Parser::parseElementsLiteralType(SMLoc loc, Type type) {
// If the user didn't provide a type, parse the colon type for the literal.
if (!type) {
if (parseToken(Token::colon, "expected ':'"))
@@ -1010,12 +1006,14 @@ ShapedType Parser::parseElementsLiteralType(Type type) {
auto sType = dyn_cast<ShapedType>(type);
if (!sType) {
- emitError("elements literal must be a shaped type");
+ emitError(loc, "elements literal must be a shaped type");
return nullptr;
}
- if (!sType.hasStaticShape())
- return (emitError("elements literal type must have static shape"), nullptr);
+ if (!sType.hasStaticShape()) {
+ emitError(loc, "elements literal type must have static shape");
+ return nullptr;
+ }
return sType;
}
@@ -1032,7 +1030,7 @@ Attribute Parser::parseSparseElementsAttr(Type attrType) {
// of the type.
Type indiceEltType = builder.getIntegerType(64);
if (consumeIf(Token::greater)) {
- ShapedType type = parseElementsLiteralType(attrType);
+ ShapedType type = parseElementsLiteralType(loc, attrType);
if (!type)
return nullptr;
@@ -1065,7 +1063,7 @@ Attribute Parser::parseSparseElementsAttr(Type attrType) {
if (parseToken(Token::greater, "expected '>'"))
return nullptr;
- auto type = parseElementsLiteralType(attrType);
+ auto type = parseElementsLiteralType(loc, attrType);
if (!type)
return nullptr;
diff --git a/mlir/lib/AsmParser/Parser.h b/mlir/lib/AsmParser/Parser.h
index 1b8aa7c9dce6f..ecc128cf767b3 100644
--- a/mlir/lib/AsmParser/Parser.h
+++ b/mlir/lib/AsmParser/Parser.h
@@ -288,7 +288,7 @@ class Parser {
/// Parse a dense elements attribute.
Attribute parseDenseElementsAttr(Type attrType);
- ShapedType parseElementsLiteralType(Type type);
+ ShapedType parseElementsLiteralType(SMLoc loc, Type type);
/// Parse a dense resource elements attribute.
Attribute parseDenseResourceElementsAttr(Type attrType);
diff --git a/mlir/test/IR/invalid-builtin-attributes.mlir b/mlir/test/IR/invalid-builtin-attributes.mlir
index 5098fe751fd01..64201950b2306 100644
--- a/mlir/test/IR/invalid-builtin-attributes.mlir
+++ b/mlir/test/IR/invalid-builtin-attributes.mlir
@@ -591,3 +591,38 @@ func.func @duplicate_dictionary_attr_key() {
#attr1 = distinct[0]<43 : i32>
// -----
+
+// Make sure the error is not printed on the return.
+func.func @print_error_on_correct_line() {
+ %0 = arith.constant
+ // expected-error@below {{elements literal must be a shaped type}}
+ dense<[3]> : i32
+ return
+}
+
+// -----
+
+// Make sure the error is not printed on the return.
+func.func @print_error_on_correct_line() {
+ %0 = arith.constant
+ // expected-error@below {{elements literal must be a shaped type}}
+ sparse<
+ [
+ [0, 1, 2, 3],
+ [1, 1, 2, 3],
+ [1, 2, 2, 3],
+ [1, 2, 3, 4]
+ ],
+ [1, 1, 1, 1] > : i32
+ return
+}
+
+// -----
+
+// Make sure the error is not printed on the return.
+func.func @print_error_on_correct_line() {
+ %0 = arith.constant
+ // expected-error@below {{elements literal must be a shaped type}}
+ sparse <> : i32
+ return
+}
|
matthias-springer
approved these changes
Feb 20, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit improves line location tracking in case of error reporting to the user in
parseElementsLiteralType. There are two cases: the type is already parsed [1] or not yet parsed [2]. With these changes we print the error at the attribute's location in both cases to ensure consistency.Case 1)
Case 2)
Note that today for a simple:
we print the error after the constant: