Fix formatter non-convergence for multi-line macro sums - #2548
Fix formatter non-convergence for multi-line macro sums#2548kbrunham-intel wants to merge 4 commits into
Conversation
Always attach infix-operator-only partitions to the following operand in localparam expressions, and treat spelling-equal MacroIdentifier and MacroIdItem as format-equivalent, so long `MACRO + MACRO` sums no longer oscillate under re-format (issue chipsalliance#2547).
|
Note on overlap with #2545: this PR also includes treating spelling-equal If #2545 lands first, please drop or rebase away the |
|
I've changed the Mac CI, can you rebase and see if that resolves that CI failure ? |
| // Some token enums differ only by surrounding whitespace (e.g. whether a | ||
| // macro or ')' ends a line). Treat those pairs as matching enums when the | ||
| // spelling is unchanged so FormatEquivalent tolerates re-wrapping. | ||
| const bool whitespace_dependent_enum_match = |
There was a problem hiding this comment.
(similar to the other PR).
Maybe we should have some predicate function getting left/right const Token references and return the context-dependent result ? TODO: find good name for that predicate function)
There was a problem hiding this comment.
I've created a new predicate function to help here.
|
Just had a quick look, but maybe we should rebase first so that we can get the formatter test unconflicted. |
Resolve formatter_test.cc conflict by keeping LongMacroSumLocalparamConverges alongside master's stacked regression tests.
Factor MacroCallCloseToEndLine/')' and MacroIdentifier/MacroIdItem matching into TokensAreWhitespaceDependentFormatEquivalent(), reusing it in LexicallyEquivalent and FormatEquivalent (PR review feedback).
|
HI @hzeller Please review this update and let me know if there is any more feedback. |
2 participants
Summary
localparamis initialized to a multi-line sum of long`MACROidentifiers (format(format(x)) != format(x)).+partitions underkParamDeclarationare always attached to the following operand, so partition shape no longer depends on original newlines.MacroIdentifier/MacroIdItem(andMacroCallCloseToEndLine/)) as format-equivalent via shared helperTokensAreWhitespaceDependentFormatEquivalent(), so verification tolerates macros moving relative to;.LongMacroSumLocalparamConverges.Fixes #2547
Overlap with #2545
This PR includes the
MacroIdentifier/MacroIdItemFormatEquivalentchange also proposed in #2545. If #2545 merges first, those equivalence-file hunks should be dropped from this PR (or this PR rebased) to avoid duplicate commits. The tree-unwrapper infix-operator attachment is unique to this fix.Test plan
master(merge conflict informatter_test.ccresolved)bazel build -c opt //...andbazel test -c opt //...(608/608)