Fix formatter abort on line-continuation before aligned ports - #2546
Open
kbrunham-intel wants to merge 3 commits into
Open
Fix formatter abort on line-continuation before aligned ports#2546kbrunham-intel wants to merge 3 commits into
kbrunham-intel wants to merge 3 commits into
Conversation
Skip alignment for partitions where non-tree tokens precede a forced wrap (e.g. // comment then `\`), and stop CHECK-failing in align.h so formatting no longer aborts (issue chipsalliance#2539).
hzeller
reviewed
Aug 2, 2026
| // } | ||
| CHECK(leading_tokens.empty() || first_tree_token_it == ftokens.end() || | ||
| first_tree_token_it->before.break_decision != | ||
| SpacingOptions::kMustWrap); |
Collaborator
There was a problem hiding this comment.
This was already addressed in
#2530 so creates a merge conflict here.
It is solved there slightly differently (just returning column entries) so double check and settle on the best solution
Contributor
Author
There was a problem hiding this comment.
I can confirm my test still fails after #2530. I will therefore merge and refactor.
hzeller
reviewed
Aug 2, 2026
hzeller
left a comment
hzeller
left a comment
Collaborator
There was a problem hiding this comment.
Thanks!
Sorry this confliced with some earlier change (there were a bunch of PRs that I only just now get to looking at).
The CHECK-fail in align.h was handled slightly differently there, so double checking and verifying with tests and refine if needed.
Keep PortListCommentWithLineContinuationDoesNotAbort and drop this branch's production alignment changes in favor of master's chipsalliance#2530 fix.
When a // comment and line-continuation `\` precede an aligned port, skip tabular alignment and preserve original spacing in ApplyAlignment so the comment is not glued onto the following declaration (issue chipsalliance#2539).
Contributor
Author
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.
Summary
//comment followed by line-continuation\before aligned ports was glued onto the following declaration (//\ input a).CHECKabort inalign.hby returning tree-scanned columns instead of asserting. That stopped the crash, but did not keep the comment on its own line.align.hhandling from Add --parameter_declaration_alignment for body-level param/localparam #2530, skips those partitions for tabular alignment, and preserves original spacing inApplyAlignmentif they still reach it.PortListCommentWithLineContinuationDoesNotAbort.Fixes #2539
Test plan
bazel test -c opt //...(608/608, including the Verible-verilog-format fails with Check failed: leading_tokens.empty() || first_tree_token_it == ftokens.end() || first_tree_token_it->before.break_decision != SpacingOptions::kMustWrap #2539 regression)