core: Move assembly format type parsing to directives #3516
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3516 +/- ##
==========================================
- Coverage 90.33% 90.33% -0.01%
==========================================
Files 464 464
Lines 58076 58160 +84
Branches 5552 5555 +3
==========================================
+ Hits 52464 52537 +73
- Misses 4184 4194 +10
- Partials 1428 1429 +1 ☔ View full report in Codecov by Sentry. |
|
|
||
| @abstractmethod | ||
| def set_types(self, types: Sequence[Attribute], state: ParsingState) -> None: ... | ||
| def parse_single_type(self, parser: Parser, state: ParsingState) -> None: ... |
There was a problem hiding this comment.
I'm not sure on the names parse_single_type and parse_many_types but couldn't think of anything better. I didn't want to go with parse_type and parse_types as the signatures are quite similar and this could cause confusion.
There was a problem hiding this comment.
They're not fantastically beautiful but they are quite clear, which is the most important thing. One thing to consider would be parse_type/parse_type_list
There was a problem hiding this comment.
I don't think I feel strongly either way. Happy to change it if you'd like me to.
There was a problem hiding this comment.
Going to go ahead and merge, can always refactor names later (will be easier to do at once with the following PRs in if we want to do it).
This level of abstraction works well for allowing parse errors to be put in the correct place.
Went slightly too far on the previous refactor. I think this level of abstraction works well for allowing parse errors to be put in the correct place. PRs for operands, results, and functional-type directives to follow based on this PR.