Skip to content

Commit

Permalink
Fix bug when reflowing comments that are exactly one character longer…
Browse files Browse the repository at this point in the history
… than the maximum line length.

PiperOrigin-RevId: 712521148
  • Loading branch information
dplassgit authored and copybara-github committed Jan 6, 2025
1 parent 9dfd0b4 commit 1efca05
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 7 deletions.
22 changes: 22 additions & 0 deletions xls/dslx/fmt/ast_fmt_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3310,6 +3310,7 @@ fn test() {
}
)");
}

TEST_F(ModuleFmtTest, StructLiteralBreak) {
DoFmt(R"(struct Foo { a: u1, b: u1 }
Expand All @@ -3321,5 +3322,26 @@ fn test() {
}
)");
}

TEST_F(ModuleFmtTest, CommentsDoNotReflow100Chars) {
// This is 100 characters, and should not reflow.
DoFmt(
R"(// Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labo
struct Foo { a: u1, b: u1 }
)");
}

TEST_F(ModuleFmtTest, CommentsReflow101Chars) {
// This is 101 characters, and wasn't reflowing before.
DoFmt(
R"(// Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labor
struct Foo { a: u1, b: u1 }
)",
R"(// Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut
// labor
struct Foo { a: u1, b: u1 }
)");
}

} // namespace
} // namespace xls::dslx
4 changes: 3 additions & 1 deletion xls/dslx/fmt/pretty_print.cc
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,9 @@ void PrettyPrintInternal(const DocArena& arena, const Doc& doc,
// If the next token isn't going to fit we make a
// carriage return and go to the next prefix insertion.
const std::string& next_tok = remaining_toks.front();
if (virtual_outcol + next_tok.size() >
// Note: we add 1 to the next_tok size because we're
// going to emit a space before it.
if (virtual_outcol + next_tok.size() + 1 >
entry.text_width()) {
VLOG(5) << "PrefixedReflow; adding carriage "
"return in advance of: `"
Expand Down
10 changes: 10 additions & 0 deletions xls/dslx/fmt/pretty_print_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,16 @@ TEST(PrettyPrintTest, PrefixedReflowAfterIndentWidth40) {
// indented)");
}

TEST(PrettyPrintTest, PrefixedReflowOffByOne) {
FileTable file_table;
DocArena arena(file_table);
// Total width is 8, so it should reflow since the max width is 7.
DocRef ref = arena.MakePrefixedReflow("// ", "1 2 3");
EXPECT_EQ(PrettyPrint(arena, ref, 7),
R"(// 1 2
// 3)");
}

// Demonstrates that the empty line between nested elements don't have leading
// spaces.
TEST(PrettyPrintTest, NestLevelWithEmptyLine) {
Expand Down
4 changes: 2 additions & 2 deletions xls/dslx/stdlib/std.x
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@ fn smul_test() {
// Returns unsigned division of `n` (N bits) and `d` (M bits) as quotient (N bits) and remainder (M
// bits).
// If dividing by `0`, returns all `1`s for quotient and `n` for remainder.
// Implements binary long division; should be expected to use a large number of gates and have a slow
// critical path when using combinational codegen.
// Implements binary long division; should be expected to use a large number of gates and have a
// slow critical path when using combinational codegen.
pub fn iterative_div_mod<N: u32, M: u32>(n: uN[N], d: uN[M]) -> (uN[N], uN[M]) {
// Zero extend divisor by 1 bit.
let divisor = d as uN[M + u32:1];
Expand Down
4 changes: 2 additions & 2 deletions xls/dslx/tests/constexpr_attrs.x
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ const LOCAL = LocalStruct { a: u32:8 };
const LOCAL_STRUCT = u32[LOCAL.a]:[u32:0, u32:1, u32:2, u32:3, ...];

//const imported_struct = u32[IMPORTED.a]:[u32:8, u32:9, u32:10, u32:11, ...];
//const imported_instance = u32[constexpr::IMPORTED_STRUCT_INSTANCE.a]:[u32:8, u32:9, u32:10, u32:11,
//...];
//const imported_instance = u32[constexpr::IMPORTED_STRUCT_INSTANCE.a]:[u32:8, u32:9, u32:10,
//u32:11, ...];

// TODO(rspringer): 2021/03/04 Add a test that dereferences an attribute of an attribute,
// e.g., "u32[IMPORTED_STRUCT.a.b]:[...]".
Expand Down
4 changes: 2 additions & 2 deletions xls/dslx/tests/tuple_indexing.x
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
// limitations under the License.

fn tuple_index_all_bits_set(x: (u1, u2, u3, u4)) -> u4 {
// Tuple index access where the index has all bits set (e.g. demonstrates the index is treated as
// unsigned).
// Tuple index access where the index has all bits set (e.g. demonstrates the index is treated
// as unsigned).
x.0b11
}

Expand Down

0 comments on commit 1efca05

Please sign in to comment.