Skip to content

Commit

Permalink
[DSLX:fmt] Format many-channel parameters case better.
Browse files Browse the repository at this point in the history 
Makes the channel type into a more atomic unit and groups it appropriately,
removes guts of custom FmtParams() implementation in favor of the generic
joiner.

Fixes #1229

PiperOrigin-RevId: 596111303
  • Loading branch information
@cdleary @copybara-github
cdleary authored and copybara-github committed Jan 6, 2024
1 parent 20374e0 commit 571af20
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 19 deletions.
43 changes: 24 additions & 19 deletions xls/dslx/fmt/ast_fmt.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ DocRef Fmt(const ChannelTypeAnnotation& n, const Comments& comments,
arena.oangle(),
Fmt(*n.payload(), comments, arena),
arena.cangle(),
arena.break1(),
arena.space(),
arena.Make(n.direction() == ChannelDirection::kIn ? Keyword::kIn
: Keyword::kOut),
};
Expand Down Expand Up @@ -1518,22 +1518,23 @@ DocRef FmtStatement(const Statement& n, const Comments& comments,
// Formats parameters (i.e. function parameters) with leading '(' and trailing
// ')'.
static DocRef FmtParams(absl::Span<const Param* const> params,
const Comments& comments, DocArena& arena) {
std::vector<DocRef> pieces = {arena.oparen()};
for (size_t i = 0; i < params.size(); ++i) {
const Param* param = params[i];
DocRef type = Fmt(*param->type_annotation(), comments, arena);
std::vector<DocRef> param_pieces = {arena.MakeText(param->identifier()),
arena.break0(), arena.colon(),
arena.break1(), type};
if (i + 1 != params.size()) {
param_pieces.push_back(arena.comma());
param_pieces.push_back(arena.break1());
}
pieces.push_back(ConcatNGroup(arena, param_pieces));
const Comments& comments, DocArena& arena,
bool align_after_oparen) {
DocRef guts = FmtJoin<const Param*>(
params, Joiner::kCommaBreak1AsGroupNoTrailingComma,
[](const Param* param, const Comments& comments, DocArena& arena) {
DocRef type = Fmt(*param->type_annotation(), comments, arena);
return ConcatN(arena, {arena.MakeText(param->identifier()),
arena.colon(), arena.space(), type});
},
comments, arena);

if (align_after_oparen) {
return ConcatNGroup(
arena, {arena.oparen(),
arena.MakeAlign(arena.MakeConcat(guts, arena.cparen()))});
}
pieces.push_back(arena.cparen());
return ConcatNGroup(arena, pieces);
return ConcatNGroup(arena, {arena.oparen(), guts, arena.cparen()});
}

static DocRef Fmt(const ParametricBinding& n, const Comments& comments,
Expand Down Expand Up @@ -1602,7 +1603,8 @@ DocRef Fmt(const Function& n, const Comments& comments, DocArena& arena) {
std::vector<DocRef> params_pieces;

params_pieces.push_back(arena.break0());
params_pieces.push_back(FmtParams(n.params(), comments, arena));
params_pieces.push_back(
FmtParams(n.params(), comments, arena, /*align_after_oparen=*/true));

if (n.return_type() == nullptr) {
params_pieces.push_back(arena.break1());
Expand Down Expand Up @@ -1682,7 +1684,8 @@ static DocRef Fmt(const Proc& n, const Comments& comments, DocArena& arena) {

std::vector<DocRef> config_pieces = {
arena.MakeText("config"),
FmtParams(n.config()->params(), comments, arena),
FmtParams(n.config()->params(), comments, arena,
/*align_after_oparen=*/true),
arena.space(),
arena.ocurl(),
arena.break1(),
Expand All @@ -1703,7 +1706,8 @@ static DocRef Fmt(const Proc& n, const Comments& comments, DocArena& arena) {

std::vector<DocRef> next_pieces = {
arena.MakeText("next"),
FmtParams(n.next()->params(), comments, arena),
FmtParams(n.next()->params(), comments, arena,
/*align_after_oparen=*/true),
arena.space(),
arena.ocurl(),
arena.break1(),
Expand Down Expand Up @@ -2094,6 +2098,7 @@ DocRef Fmt(const Module& n, const Comments& comments, DocArena& arena) {
pieces.push_back(arena.hard_line());
} else if (AreGroupedMembers<Import>(member, n.top()[i + 1]) ||
AreGroupedMembers<TypeAlias>(member, n.top()[i + 1]) ||
AreGroupedMembers<StructDef>(member, n.top()[i + 1]) ||
AreGroupedMembers<ConstantDef>(member, n.top()[i + 1])) {
// If two (e.g. imports) are adjacent to each other (i.e. no intervening
// newline) we keep them adjacent to each other in the formatted output.
Expand Down
65 changes: 65 additions & 0 deletions xls/dslx/fmt/ast_fmt_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1655,6 +1655,35 @@ pub proc q {
EXPECT_EQ(got, kProgram);
}

TEST(ModuleFmtTest, SimpleProcWithLotsOfChannels) {
const std::string_view kProgram =
R"(pub proc p {
cin: chan<u32> in;
ca: chan<u32> out;
cb: chan<u32> out;
cc: chan<u32> out;
cd: chan<u32> out;
ce: chan<u32> out;
cf: chan<u32> out;
cg: chan<u32> out;
config(cin: chan<u32> in, ca: chan<u32> out, cb: chan<u32> out, cc: chan<u32> out,
cd: chan<u32> out, ce: chan<u32> out, cf: chan<u32> out, cg: chan<u32> out) {
(cin, ca, cb, cc, cd, ce, cf, cg)
}
init { () }
next(tok: token, state: ()) { () }
}
)";
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);
}

// Based on report in https://github.com/google/xls/issues/1216
TEST(ModuleFmtTest, ProcSpawnImported) {
const std::string_view kProgram =
Expand Down Expand Up @@ -1948,5 +1977,41 @@ fn id(x: u32) { x }
EXPECT_EQ(got, kProgram);
}

TEST(ModuleFmtTest, GithubIssue1229) {
// Note: we just need it to parse, no need for it to typecheck.
const std::string_view kProgram = R"(struct ReadReq<X: u32> {}
struct ReadResp<X: u32> {}
struct WriteReq<X: u32, Y: u32> {}
struct WriteResp {}
struct AllCSR<X: u32, Y: u32> {}
proc CSR<X: u32, Y: u32, Z: u32> {
config() { () }
init { () }
next(tok: token, state: ()) { () }
}
proc csr_8_32_14 {
config(read_req: chan<ReadReq<u32:8>> in, read_resp: chan<ReadResp<u32:32>> out,
write_req: chan<WriteReq<u32:8, u32:32>> in, write_resp: chan<WriteResp> out,
all_csr: chan<AllCSR<u32:32, u32:14>> out) {
spawn CSR<u32:8, u32:32, u32:14>(read_req, read_resp, write_req, write_resp, all_csr);
()
}
init { () }
next(tok: token, state: ()) { () }
}
)";
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);
}

} // namespace
} // namespace xls::dslx

0 comments on commit 571af20

Please sign in to comment.