From 5dccbd0c9386a2a585ab7de90dd0e24d1a6f65ab Mon Sep 17 00:00:00 2001 From: emmau678 Date: Fri, 17 Jan 2025 15:01:05 +0000 Subject: [PATCH 1/5] fix filecheck and parser to giev a better error message --- tests/filecheck/parser-printer/graph_region.mlir | 10 ++++++++++ tests/filecheck/parser-printer/parse_error.mlir | 8 ++++++++ xdsl/parser/core.py | 9 ++++++--- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/tests/filecheck/parser-printer/graph_region.mlir b/tests/filecheck/parser-printer/graph_region.mlir index a0b1c95f8e..b051ef909f 100644 --- a/tests/filecheck/parser-printer/graph_region.mlir +++ b/tests/filecheck/parser-printer/graph_region.mlir @@ -61,6 +61,16 @@ builtin.module { // ----- +// A graph region that refers to values that are not defined in the module. + +builtin.module { + %0 = "test.termop"(%1, %2) : (i32, i32) -> i32 +} + +// CHECK: values %1, %2 were used but not defined + +// ----- + // A forward value used with a wrong index builtin.module { diff --git a/tests/filecheck/parser-printer/parse_error.mlir b/tests/filecheck/parser-printer/parse_error.mlir index 383fe99ed7..358d54e548 100644 --- a/tests/filecheck/parser-printer/parse_error.mlir +++ b/tests/filecheck/parser-printer/parse_error.mlir @@ -14,3 +14,11 @@ test.op : () -> () // CHECK-NEXT: test.op : () -> () // CHECK-NEXT: ^ // CHECK-NEXT: Operation test.op does not have a custom format. + +// ----- + +module { + "test.op"() [^unknown_successor]: () -> () +} + +// CHECK: Unknown location of span region ends with missing block declarations for block(s) unknown_successor. diff --git a/xdsl/parser/core.py b/xdsl/parser/core.py index 86ac8ac297..f3392bda23 100644 --- a/xdsl/parser/core.py +++ b/xdsl/parser/core.py @@ -100,7 +100,7 @@ def __init__( super().__init__(ParserState(MLIRLexer(Input(input, name))), ctx) self.ssa_values = dict() self.blocks = dict() - self.forward_block_references = dict() + self.forward_block_references = defaultdict(list) self.forward_ssa_references = dict() def parse_module(self, allow_implicit_module: bool = True) -> ModuleOp: @@ -145,7 +145,10 @@ def parse_module(self, allow_implicit_module: bool = True) -> ModuleOp: value_names = ", ".join( "%" + name for name in self.forward_ssa_references.keys() ) - self.raise_error(f"values used but not defined: [{value_names}]") + if len(self.forward_ssa_references.keys()) > 1: + self.raise_error(f"values {value_names} were used but not defined") + else: + self.raise_error(f"value {value_names} was used but not defined") return module_op @@ -561,7 +564,7 @@ def parse_optional_region( region.add_block(block) # Finally, check that all forward block references have been resolved. - if len(self.forward_block_references) > 0: + if self.forward_block_references: pos = self.lexer.pos raise MultipleSpansParseError( Span(pos, pos + 1, self.lexer.input), From 062af77f30381be3a727affa3830fc4f8084b9fa Mon Sep 17 00:00:00 2001 From: emmau678 Date: Tue, 4 Feb 2025 16:40:41 +0000 Subject: [PATCH 2/5] identify bug in MultipleSpansParseError --- xdsl/parser/core.py | 2 +- xdsl/utils/exceptions.py | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/xdsl/parser/core.py b/xdsl/parser/core.py index f3392bda23..9081f40460 100644 --- a/xdsl/parser/core.py +++ b/xdsl/parser/core.py @@ -565,7 +565,7 @@ def parse_optional_region( # Finally, check that all forward block references have been resolved. if self.forward_block_references: - pos = self.lexer.pos + pos = self.pos raise MultipleSpansParseError( Span(pos, pos + 1, self.lexer.input), "region ends with missing block declarations for block(s) {}".format( diff --git a/xdsl/utils/exceptions.py b/xdsl/utils/exceptions.py index 2cf52c0db2..42394d8a93 100644 --- a/xdsl/utils/exceptions.py +++ b/xdsl/utils/exceptions.py @@ -110,6 +110,12 @@ def __str__(self) -> str: res += span.print_with_context(msg) + "\n" return res + def __str__(self) -> str: + res = "" + for span, msg in self.refs: + res += span.print_with_context(msg) + "\n" + return res + class PassPipelineParseError(BaseException): def __init__(self, token: Token, msg: str): From e988e3d24a35297c80ef24ea3d2e903964b58183 Mon Sep 17 00:00:00 2001 From: emmau678 Date: Mon, 17 Feb 2025 15:50:27 +0000 Subject: [PATCH 3/5] remove __str__ as it has been re-implemented since this PR was opened --- xdsl/utils/exceptions.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/xdsl/utils/exceptions.py b/xdsl/utils/exceptions.py index 42394d8a93..2cf52c0db2 100644 --- a/xdsl/utils/exceptions.py +++ b/xdsl/utils/exceptions.py @@ -110,12 +110,6 @@ def __str__(self) -> str: res += span.print_with_context(msg) + "\n" return res - def __str__(self) -> str: - res = "" - for span, msg in self.refs: - res += span.print_with_context(msg) + "\n" - return res - class PassPipelineParseError(BaseException): def __init__(self, token: Token, msg: str): From 6bad6f7b972d115e6feb8d9b3dce3cb63f49bc40 Mon Sep 17 00:00:00 2001 From: emmau678 Date: Mon, 17 Feb 2025 15:58:05 +0000 Subject: [PATCH 4/5] revert revert --- tests/filecheck/parser-printer/graph_region.mlir | 10 ---------- xdsl/parser/core.py | 5 +---- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/tests/filecheck/parser-printer/graph_region.mlir b/tests/filecheck/parser-printer/graph_region.mlir index b051ef909f..a0b1c95f8e 100644 --- a/tests/filecheck/parser-printer/graph_region.mlir +++ b/tests/filecheck/parser-printer/graph_region.mlir @@ -61,16 +61,6 @@ builtin.module { // ----- -// A graph region that refers to values that are not defined in the module. - -builtin.module { - %0 = "test.termop"(%1, %2) : (i32, i32) -> i32 -} - -// CHECK: values %1, %2 were used but not defined - -// ----- - // A forward value used with a wrong index builtin.module { diff --git a/xdsl/parser/core.py b/xdsl/parser/core.py index 9081f40460..cf69b2bd16 100644 --- a/xdsl/parser/core.py +++ b/xdsl/parser/core.py @@ -145,10 +145,7 @@ def parse_module(self, allow_implicit_module: bool = True) -> ModuleOp: value_names = ", ".join( "%" + name for name in self.forward_ssa_references.keys() ) - if len(self.forward_ssa_references.keys()) > 1: - self.raise_error(f"values {value_names} were used but not defined") - else: - self.raise_error(f"value {value_names} was used but not defined") + self.raise_error(f"values used but not defined: [{value_names}]") return module_op From 667a4626f5c442910ea7a26002a484ca61a49ef5 Mon Sep 17 00:00:00 2001 From: emmau678 Date: Mon, 17 Feb 2025 16:00:23 +0000 Subject: [PATCH 5/5] correct error msg --- tests/filecheck/parser-printer/parse_error.mlir | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/filecheck/parser-printer/parse_error.mlir b/tests/filecheck/parser-printer/parse_error.mlir index 358d54e548..c734b10c40 100644 --- a/tests/filecheck/parser-printer/parse_error.mlir +++ b/tests/filecheck/parser-printer/parse_error.mlir @@ -21,4 +21,4 @@ module { "test.op"() [^unknown_successor]: () -> () } -// CHECK: Unknown location of span region ends with missing block declarations for block(s) unknown_successor. +// CHECK: reference to block "^unknown_successor" without implementation