core: (attr assembly fmt) add core directive classes (pt1)#5900
Conversation
Add the foundational classes for a declarative assembly format system for ParametrizedAttribute types, mirroring the existing op-side system: AttrFormatDirective ABC, structural directives (whitespace, punctuation, keyword), ParameterVariable, AttrFormatProgram, and AttrFormatParser.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5900 +/- ##
==========================================
- Coverage 86.23% 86.20% -0.03%
==========================================
Files 418 418
Lines 59893 60077 +184
Branches 6920 6950 +30
==========================================
+ Hits 51649 51791 +142
- Misses 6660 6689 +29
- Partials 1584 1597 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| def _attr_def_for(cls: type[ParametrizedAttribute]) -> ParamAttrDef: | ||
| """Build a ParamAttrDef from a ParametrizedAttribute class.""" | ||
| return ParamAttrDef.from_pyrdl(cls) | ||
|
|
There was a problem hiding this comment.
No need to re-parse it, let's just get the attr def inline?
| def _attr_def_for(cls: type[ParametrizedAttribute]) -> ParamAttrDef: | |
| """Build a ParamAttrDef from a ParametrizedAttribute class.""" | |
| return ParamAttrDef.from_pyrdl(cls) | |
| def _attr_def_for(cls: type[ParametrizedAttribute]) -> ParamAttrDef: | |
| """Build a ParamAttrDef from a ParametrizedAttribute class.""" | |
| return cls.get_irdl_definition() |
| @irdl_attr_definition | ||
| class SimpleType(ParametrizedAttribute, TypeAttribute): | ||
| name = "test_af.simple" | ||
| value: IntegerType = param_def() |
There was a problem hiding this comment.
I can think of a simpler type :)
| @irdl_attr_definition | |
| class SimpleType(ParametrizedAttribute, TypeAttribute): | |
| name = "test_af.simple" | |
| value: IntegerType = param_def() | |
| @irdl_attr_definition | |
| class SimpleType(ParametrizedAttribute, TypeAttribute): | |
| name = "test_af.simple" |
|
I would love for this part of the codebase to have 100% code coverage. Could you please take a look at what's not covered and add tests for it? Also, I still feel like this first PR is bigger than ideal. Could you please try removing anything to do with parameters to start with? In theory we can test independently parsing punctuation and whitespace. |
Adding foundational classes for a declarative assembly format system for ParametrizedAttribute types, mirroring the implementations for op assembly format.
The existing format directives are tightly wired to operations, so this PR proposes instead to have a separate hierararchy based on AttrFormatDirective to keep the systems separate which helped keep the implementation clean. A purposefully small set of simple directives is implemented (whitespace, punctuation, keyword, param variable). Some of these overlap with implementations for op directives, so I factored out
_print_whitespace,_print_punctuation, and_print_keyword. TheAttrFormatProgramandAttrFormatParserresemble what is implemented for operations.This PR is the first in a stacked series to enable full support for attribute/type assembly_format: