-
Notifications
You must be signed in to change notification settings - Fork 74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New Parser #262
New Parser #262
Changes from all commits
483de42
8f39dbd
86fb17c
7de6326
1f0cecb
0560fde
3bdf8aa
b5123e8
57091bc
d416eff
48b92a1
2940b98
33d59fa
881f8d0
280d7ee
3073039
4464c32
cfd5c62
7f41009
4404ffc
3a06393
7b94084
ed3a997
34157f5
649e43a
a4bb049
af8cebf
64ac94e
c51e7a9
7782654
d77fa27
b415f6a
ba39207
1e9ccc8
e21d7a0
9365029
684c809
c49fc29
0420b56
b58143a
58339bc
d0b5a5b
41049fe
7fda7e0
70ecada
8e5ce88
6968619
ebbc7bc
56490fd
932e403
4e985cd
ea9a75b
db7c4cb
4e27ede
7a40bf2
46fee68
2f09883
91a8d12
85810fc
54a7788
9db3804
868e509
de5fa12
cc67615
3c4349a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,37 +1,43 @@ | ||
from io import StringIO | ||
|
||
import pytest | ||
|
||
from xdsl.ir import MLContext | ||
from xdsl.parser import Parser | ||
from xdsl.printer import Printer | ||
from xdsl.ir import MLContext, Attribute | ||
from xdsl.parser import XDSLParser | ||
from xdsl.dialects.builtin import IntAttr, DictionaryAttr, StringAttr, ArrayAttr, Builtin | ||
|
||
|
||
@pytest.mark.parametrize("input,expected", [("0, 1, 1", [0, 1, 1]), | ||
("1, 0, 1", [1, 0, 1]), | ||
("1, 1, 0", [1, 1, 0])]) | ||
def test_int_list_parser(input: str, expected: list[int]): | ||
ctx = MLContext() | ||
parser = Parser(ctx, input) | ||
|
||
int_list = parser.parse_list(parser.parse_int_literal) | ||
assert int_list == expected | ||
|
||
|
||
@pytest.mark.parametrize("input,expected", [('{"A"=0, "B"=1, "C"=2}', { | ||
"A": 0, | ||
"B": 1, | ||
"C": 2 | ||
}), ('{"MA"=10, "BR"=7, "Z"=3}', { | ||
"MA": 10, | ||
"BR": 7, | ||
"Z": 3 | ||
}), ('{"Q"=77, "VV"=12, "AA"=-8}', { | ||
"Q": 77, | ||
"VV": 12, | ||
"AA": -8 | ||
})]) | ||
def test_int_dictionary_parser(input: str, expected: dict[str, int]): | ||
parser = XDSLParser(ctx, input) | ||
|
||
int_list = parser.parse_list_of(parser.try_parse_integer_literal, '') | ||
assert [int(span.text) for span in int_list] == expected | ||
|
||
|
||
@pytest.mark.parametrize('data', [ | ||
dict(a=IntAttr.from_int(1), b=IntAttr.from_int(2), c=IntAttr.from_int(3)), | ||
dict(a=StringAttr.from_str('hello'), | ||
b=IntAttr.from_int(2), | ||
c=ArrayAttr.from_list( | ||
[IntAttr.from_int(2), | ||
StringAttr.from_str('world')])), | ||
dict(), | ||
]) | ||
def test_dictionary_attr(data: dict[str, Attribute]): | ||
attr = DictionaryAttr.from_dict(data) | ||
|
||
with StringIO() as io: | ||
Printer(io).print(attr) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this makes sense as this makes the Parser tests depend on the Printer. I feel that parser tests should really take strings and the data structure that is expected in order to test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Valid point. I'll change that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just noticed that most of the printer tests rely on the parser as well. What's up with that? Why is that more okay? |
||
text = io.getvalue() | ||
|
||
ctx = MLContext() | ||
parser = Parser(ctx, input) | ||
ctx.register_dialect(Builtin) | ||
|
||
attr = XDSLParser(ctx, text).parse_attribute() | ||
|
||
int_dict = parser.parse_dictionary(parser.parse_str_literal, | ||
parser.parse_int_literal) | ||
assert int_dict == expected | ||
assert attr.data == data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest having code like
Which is what MLIR people usually write, and to me makes more sense in that case (since we only have 2 cases).
However, we should definitely keep the regex for other use cases.