Skip to content
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

core: (assembly-format) omit default attributes/properties from attr-dict directive #3783

Merged
merged 3 commits into from
Jan 24, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions tests/irdl/test_declarative_assembly_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -2965,6 +2965,68 @@ class DefaultConstantOp(IRDLOperation):
check_equivalence(program, generic, ctx)


@pytest.mark.parametrize(
"program, generic",
[
(
"test.default_attr_dict",
'"test.default_attr_dict"() <{prop = false}> {attr = false} : () -> ()',
),
(
"test.default_attr_dict {attr = true, prop = true}",
'"test.default_attr_dict"() <{prop = true}> {attr = true} : () -> ()',
),
],
)
def test_default_property_in_attr_dict(program: str, generic: str):
@irdl_op_definition
class DefaultAttrDictOp(IRDLOperation):
name = "test.default_attr_dict"

prop = prop_def(BoolAttr, default_value=BoolAttr.from_bool(False))

attr = attr_def(BoolAttr, default_value=BoolAttr.from_bool(False))

irdl_options = [ParsePropInAttrDict()]

assembly_format = "attr-dict"

ctx = MLContext()
ctx.load_op(DefaultAttrDictOp)

check_roundtrip(program, ctx)
check_equivalence(program, generic, ctx)


@pytest.mark.parametrize(
"program, generic",
[
(
"test.default_attr_dict",
'"test.default_attr_dict"() {attr = false} : () -> ()',
),
(
"test.default_attr_dict {attr = true}",
'"test.default_attr_dict"() {attr = true} : () -> ()',
),
],
)
def test_default_attr_in_attr_dict(program: str, generic: str):
@irdl_op_definition
class DefaultAttrDictOp(IRDLOperation):
name = "test.default_attr_dict"

attr = attr_def(BoolAttr, default_value=BoolAttr.from_bool(False))

assembly_format = "attr-dict"

ctx = MLContext()
ctx.load_op(DefaultAttrDictOp)

check_roundtrip(program, ctx)
check_equivalence(program, generic, ctx)


################################################################################
# Extractors #
################################################################################
Expand Down
34 changes: 24 additions & 10 deletions xdsl/irdl/declarative_assembly_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -438,27 +438,41 @@ def parse(self, parser: Parser, state: ParsingState) -> None:

def print(self, printer: Printer, state: PrintingState, op: IRDLOperation) -> None:
if self.print_properties:
if (
not (set(op.attributes.keys()) | set(op.properties.keys()))
- self.reserved_attr_names
):
return
if any(name in op.attributes for name in op.properties):
raise ValueError(
"Cannot print attributes and properties with the same name "
"in a signle dictionary"
"in a single dictionary"
)
op_def = op.get_irdl_definition()
dictionary = op.attributes | op.properties
default_names = set(
name
for name, d in (op_def.properties | op_def.attributes).items()
if d.default_value is not None
and dictionary.get(name) == d.default_value
)
reserved_or_default = self.reserved_attr_names | default_names
if not (set(dictionary.keys())) - reserved_or_default:
return
printer.print_op_attributes(
op.attributes | op.properties,
reserved_attr_names=self.reserved_attr_names,
dictionary,
reserved_attr_names=reserved_or_default,
print_keyword=self.with_keyword,
)
else:
if not set(op.attributes.keys()) - self.reserved_attr_names:
op_def = op.get_irdl_definition()
default_names = set(
name
for name, d in op_def.attributes.items()
if d.default_value is not None
and op.attributes.get(name) == d.default_value
)
reserved_or_default = self.reserved_attr_names | default_names
if not set(op.attributes.keys()) - reserved_or_default:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
default_names = set(
name
for name, d in op_def.attributes.items()
if d.default_value is not None
and op.attributes.get(name) == d.default_value
)
reserved_or_default = self.reserved_attr_names | default_names
if not set(op.attributes.keys()) - reserved_or_default:
reserved_or_default = self.reserved_attr_names.update(
name
for name, d in op_def.attributes.items()
if d.default_value is not None
and op.attributes.get(name) == d.default_value
)
if not reserved_or_default.is_superset(op.attributes.keys()):

Two proposals, each of which avoids a set construction, seems a tiny bit cleaner

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update is a mutating function right? (I can't mutate self.reserved_attr_names)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, I meant union

return
printer.print_op_attributes(
op.attributes,
reserved_attr_names=self.reserved_attr_names,
reserved_attr_names=reserved_or_default,
print_keyword=self.with_keyword,
)

Expand Down
Loading