Skip to content

Commit

Permalink
[DSLX:fmt] Add support for array elements packing into lines.
Browse files Browse the repository at this point in the history 
E.g. this occurs in `acm_random.x`.

PiperOrigin-RevId: 573248304
  • Loading branch information
@cdleary @copybara-github
cdleary authored and copybara-github committed Oct 13, 2023
1 parent c8166e7 commit 87301c6
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 23 deletions.
88 changes: 66 additions & 22 deletions xls/dslx/fmt/ast_fmt.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,15 @@ static DocRef FmtExprPtr(const Expr* n, const Comments& comments,
enum class Joiner : uint8_t {
kCommaSpace,
kCommaBreak1,

// Separates via a comma and break1, but groups the element with its
// delimiter. This is useful when we're packing member elements that we want
// to be reflowed across lines.
//
// Note that, in this mode, if we span multiple lines, we'll put a trailing
// comma as well.
kCommaBreak1AsGroup,

kSpaceBarBreak,
kHardLine,
};
Expand All @@ -80,6 +89,18 @@ DocRef FmtJoin(
pieces.push_back(arena.comma());
pieces.push_back(arena.break1());
break;
case Joiner::kCommaBreak1AsGroup: {
DocRef member = pieces.back();
pieces.pop_back();
std::vector<DocRef> this_pieces;
if (i != 0) {
this_pieces.push_back(arena.break1());
}
this_pieces.push_back(member);
this_pieces.push_back(arena.comma());
pieces.push_back(ConcatNGroup(arena, this_pieces));
break;
}
case Joiner::kSpaceBarBreak:
pieces.push_back(arena.space());
pieces.push_back(arena.bar());
Expand All @@ -89,6 +110,16 @@ DocRef FmtJoin(
pieces.push_back(arena.hard_line());
break;
}
} else { // last member, no trailing delimiter
if (joiner == Joiner::kCommaBreak1AsGroup && i != 0) {
// Note: we only want to put a leading space in front of the last
// element if the last element is not also the first element.
pieces.back() = ConcatNGroup(arena, {arena.break1(), pieces.back()});

// With this pattern if we're in break mode (implying we spanned
// multiple lines), we allow a trailing comma.
pieces.push_back(arena.MakeFlatChoice(arena.empty(), arena.comma()));
}
}
}
return ConcatN(arena, pieces);
Expand Down Expand Up @@ -204,25 +235,35 @@ DocRef Fmt(const WildcardPattern& n, const Comments& comments,
}

DocRef Fmt(const Array& n, const Comments& comments, DocArena& arena) {
std::vector<DocRef> pieces;
std::vector<DocRef> leader_pieces;
if (TypeAnnotation* t = n.type_annotation()) {
pieces.push_back(Fmt(*t, comments, arena));
pieces.push_back(arena.colon());
leader_pieces.push_back(Fmt(*t, comments, arena));
leader_pieces.push_back(arena.colon());
}
pieces.push_back(arena.obracket());
leader_pieces.push_back(arena.obracket());

std::vector<DocRef> pieces;
pieces.push_back(ConcatNGroup(arena, leader_pieces));
pieces.push_back(arena.break0());
for (const Expr* member : n.members()) {
pieces.push_back(Fmt(*member, comments, arena));
pieces.push_back(arena.comma());
pieces.push_back(arena.break1());
}

std::vector<DocRef> member_pieces;
member_pieces.push_back(FmtJoin<const Expr*>(
n.members(), Joiner::kCommaBreak1AsGroup, FmtExprPtr, comments, arena));

if (n.has_ellipsis()) {
pieces.push_back(arena.MakeText("..."));
} else {
pieces.pop_back();
pieces.pop_back();
// Subtle implementation note: The Joiner::CommaBreak1AsGroup puts a
// trailing comma when we're in break mode, so we only insert the comma for
// ellipsis when we're in flat mode.
member_pieces.push_back(arena.MakeFlatChoice(arena.comma(), arena.empty()));

member_pieces.push_back(
ConcatNGroup(arena, {arena.break1(), arena.MakeText("...")}));
}

pieces.push_back(arena.MakeNest(ConcatNGroup(arena, member_pieces)));
pieces.push_back(arena.break0());
pieces.push_back(arena.cbracket());

return ConcatNGroup(arena, pieces);
}

Expand Down Expand Up @@ -1118,17 +1159,20 @@ static DocRef Fmt(const StructDef& n, const Comments& comments,

static DocRef Fmt(const ConstantDef& n, const Comments& comments,
DocArena& arena) {
std::vector<DocRef> pieces;
std::vector<DocRef> leader_pieces;
if (n.is_public()) {
pieces.push_back(arena.Make(Keyword::kPub));
pieces.push_back(arena.break1());
leader_pieces.push_back(arena.Make(Keyword::kPub));
leader_pieces.push_back(arena.break1());
}
pieces.push_back(arena.Make(Keyword::kConst));
pieces.push_back(arena.break1());
pieces.push_back(arena.MakeText(n.identifier()));
pieces.push_back(arena.break1());
pieces.push_back(arena.equals());
pieces.push_back(arena.break1());
leader_pieces.push_back(arena.Make(Keyword::kConst));
leader_pieces.push_back(arena.break1());
leader_pieces.push_back(arena.MakeText(n.identifier()));
leader_pieces.push_back(arena.break1());
leader_pieces.push_back(arena.equals());
leader_pieces.push_back(arena.space());

std::vector<DocRef> pieces;
pieces.push_back(ConcatNGroup(arena, leader_pieces));
pieces.push_back(Fmt(*n.value(), comments, arena));
pieces.push_back(arena.semi());
return ConcatNGroup(arena, pieces);
Expand Down
29 changes: 29 additions & 0 deletions xls/dslx/fmt/ast_fmt_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,35 @@ TEST(ModuleFmtTest, ConstantDefArray) {
EXPECT_EQ(got, kProgram);
}

TEST(ModuleFmtTest, ConstantDefArrayMultiline) {
const std::string_view kProgram = R"(pub const VALS = u64[5]:[
0x002698ad4b48ead0, 0x1bfb1e0316f2d5de, 0x173a623c9725b477, 0x0a447a02823ad868,
0x1df74948b3fbea7e, 0x1bc8b594bcf01a39, 0x07b767ca9520e99a, 0x05e28b4320bfd20e,
0x0105906a24823f57, 0x1a1e7d14a6d24384, 0x2a7326df322e084d, 0x120bc9cc3fac4ec7,
0x2c8f193a1b46a9c5, 0x2b9c95743bbe3f90, 0x0dcfc5b1d0398b46, 0x006ba47b3448bea3,
0x3fe4fbf9a522891b, 0x23e1a50ad6aebca3, 0x1b263d39ea62be44, 0x13581d282e643b0e,
];
)";
std::vector<CommentData> comments;
XLS_ASSERT_OK_AND_ASSIGN(std::unique_ptr<Module> m,
ParseModule(kProgram, "fake.x", "fake", &comments));
std::string got = AutoFmt(*m, Comments::Create(comments));
EXPECT_EQ(got, kProgram);
}

TEST(ModuleFmtTest, ConstantDefArrayMultilineWithEllipsis) {
const std::string_view kProgram = R"(pub const VALS = u64[8]:[
0x002698ad4b48ead0, 0x1bfb1e0316f2d5de, 0x173a623c9725b477, 0x0a447a02823ad868,
0x1df74948b3fbea7e, 0x1bc8b594bcf01a39, 0x07b767ca9520e99a, ...
];
)";
std::vector<CommentData> comments;
XLS_ASSERT_OK_AND_ASSIGN(std::unique_ptr<Module> m,
ParseModule(kProgram, "fake.x", "fake", &comments));
std::string got = AutoFmt(*m, Comments::Create(comments));
EXPECT_EQ(got, kProgram);
}

TEST(ModuleFmtTest, ConstantDefArrayEllipsis) {
const std::string_view kProgram = "pub const VALS = u32[2]:[32, ...];\n";
std::vector<CommentData> comments;
Expand Down
5 changes: 4 additions & 1 deletion xls/dslx/fmt/pretty_print.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ void PrettyPrintInternal(const DocArena& arena, const Doc& doc,
absl::visit(
Visitor{
[&](const std::string& s) {
XLS_VLOG(3) << "emitting text: `" << s
<< "` at outcol: " << outcol << " (size " << s.size()
<< ")";
// Text is simply emitted to the output and we bump the output
// column tracker accordingly.
pieces.push_back(s);
Expand Down Expand Up @@ -213,7 +216,7 @@ void PrettyPrintInternal(const DocArena& arena, const Doc& doc,
int64_t remaining_cols = text_width - outcol;
Requirement grouped_requirement =
arena.Deref(group.arg).flat_requirement;
XLS_VLOG(1) << "grouped_requirement: "
XLS_VLOG(3) << "grouped_requirement: "
<< RequirementToString(grouped_requirement)
<< " remaining_cols: " << remaining_cols;
if (grouped_requirement > remaining_cols) {
Expand Down

0 comments on commit 87301c6

Please sign in to comment.