core: (attr assembly fmt) add core directive classes (pt1)#5900
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5900 +/- ##
==========================================
+ Coverage 86.88% 86.89% +0.01%
==========================================
Files 431 431
Lines 64559 64599 +40
Branches 7395 7396 +1
==========================================
+ Hits 56090 56132 +42
+ Misses 6897 6896 -1
+ Partials 1572 1571 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
n-io
changed the title
superlopuh
reviewed
Apr 21, 2026
superlopuh
reviewed
Apr 21, 2026
Member
|
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. |
…rmat Add the foundational classes for a declarative assembly format system for ParametrizedAttribute types, mirroring the op-side system: the AttrFormatDirective ABC, the structural directives (whitespace, punctuation, keyword), AttrFormatProgram, and AttrFormatParser. Parameter and other directives are added in follow-up PRs.
n-io
force-pushed
the
nicolai/attr-asm-fmt-pt1-core-directives
branch
from
377ba90 to
b8e73f1
Compare
The op-side WhitespaceDirective, PunctuationDirective and KeywordDirective now delegate their print logic to the shared _print_whitespace / _print_punctuation / _print_keyword helpers instead of duplicating it, matching how the attr-side directives already use them.
Collaborator
Author
|
Both done as suggested. |
superlopuh
reviewed
Jul 8, 2026
superlopuh
reviewed
Jul 8, 2026
An empty format string is a valid program (no directives): it prints nothing and parses as a no-op. Addresses review feedback on #5900.
superlopuh
reviewed
Jul 8, 2026
Per review, shrink this PR to only what is needed for an empty assembly_format to work: drop the whitespace, punctuation and keyword directives, and revert the shared print helpers (restoring the op-side directives to their original inline logic). Concrete directives will follow in later PRs.
superlopuh
reviewed
Jul 8, 2026
superlopuh
reviewed
Jul 8, 2026
- Assert the format is empty in AttrFormatProgram.parse/print instead of iterating over stmts, removing the two uncovered loop bodies (the loop returns in a later PR alongside concrete directives). - Reformat the new docstrings to start on their own line. - Split the empty-format test into program/print-parse cases and add an end-to-end parse of !test_af.empty.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adding foundational classes for a declarative assembly format system, 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 hierarchy based on AttrFormatDirective to keep the systems separate which helped keep the implementation clean. This first PR contains only the plumbing — the
AttrFormatDirectivebase class,AttrFormatProgram, and anAttrFormatParserskeleton — and currently supports only the empty format; concrete directives follow in later PRs. TheAttrFormatProgramandAttrFormatParserresemble what is implemented for operations.This PR is the first in a stacked series to enable full support for attribute/type assembly_format. See also: