From 0dbe8469c02d6f0f59bc7cb61029404a79e4758f Mon Sep 17 00:00:00 2001 From: David Plass Date: Thu, 21 Nov 2024 07:21:13 -0800 Subject: [PATCH] Fix `Binop`'s span to start at the start of the lhs and end at the limit of the rhs. This will be needed to more accurately determine if a disable format comment is within an expression or not. PiperOrigin-RevId: 698771405 --- xls/dslx/bytecode/bytecode_interpreter_test.cc | 8 ++++---- xls/dslx/frontend/parser.cc | 3 ++- ...on_converter_test_ConvertsSimpleFunctionWithAsserts.ir | 6 +++--- ...nction_converter_test_TracksMultipleTypeAliasSvType.ir | 2 +- ...on_converter_test_TracksTypeAliasStopsAtFirstSvType.ir | 2 +- .../function_converter_test_TracksTypeAliasSvType.ir | 2 +- xls/dslx/ir_convert/testdata/ir_converter_test_Concat.ir | 2 +- .../testdata/ir_converter_test_InvocationMultiSymbol.ir | 2 +- .../testdata/ir_converter_test_InvokeMultipleArgs.ir | 2 +- .../ir_convert/testdata/ir_converter_test_LetBinding.ir | 2 +- .../testdata/ir_converter_test_LetTupleBinding.ir | 2 +- .../ir_convert/testdata/ir_converter_test_SignedDiv.ir | 2 +- .../ir_convert/testdata/ir_converter_test_TwoPlusTwo.ir | 2 +- xls/dslx/tests/errors/error_modules_test.py | 4 ++-- xls/flows/testdata/ir_wrapper_test_DslxProcsToIrOk.ir | 2 +- 15 files changed, 22 insertions(+), 21 deletions(-) diff --git a/xls/dslx/bytecode/bytecode_interpreter_test.cc b/xls/dslx/bytecode/bytecode_interpreter_test.cc index 3fab329f4d..294ccf3893 100644 --- a/xls/dslx/bytecode/bytecode_interpreter_test.cc +++ b/xls/dslx/bytecode/bytecode_interpreter_test.cc @@ -3107,7 +3107,7 @@ fn main(x: u2, y: u2) -> u2 { if (x + y >= 4) { // We should have a rollover in these cases. ASSERT_EQ(source_spans.size(), 1); - EXPECT_EQ(source_spans.at(0).ToString(file_table), "test.x:3:5-3:6"); + EXPECT_EQ(source_spans.at(0).ToString(file_table), "test.x:3:3-3:8"); } else { ASSERT_TRUE(source_spans.empty()); } @@ -3147,7 +3147,7 @@ fn main(x: u2, y: u2) -> u2 { if (static_cast(x) - static_cast(y) < 0) { // We should have a rollover in these cases. ASSERT_EQ(source_spans.size(), 1); - EXPECT_EQ(source_spans.at(0).ToString(file_table), "test.x:3:5-3:6"); + EXPECT_EQ(source_spans.at(0).ToString(file_table), "test.x:3:3-3:8"); } else { ASSERT_TRUE(source_spans.empty()); } @@ -3186,7 +3186,7 @@ fn main(x: u2, y: u2) -> u2 { if (x * y >= 4) { // We should have a rollover in these cases. ASSERT_EQ(source_spans.size(), 1); - EXPECT_EQ(source_spans.at(0).ToString(file_table), "test.x:3:5-3:6"); + EXPECT_EQ(source_spans.at(0).ToString(file_table), "test.x:3:3-3:8"); } else { ASSERT_TRUE(source_spans.empty()); } @@ -3222,7 +3222,7 @@ fn main(x: s2, y: s2) -> s2 { << " rollover: " << rollover; if (rollover) { ASSERT_EQ(source_spans.size(), 1); - EXPECT_EQ(source_spans.at(0).ToString(file_table), "test.x:3:5-3:6"); + EXPECT_EQ(source_spans.at(0).ToString(file_table), "test.x:3:3-3:8"); } else { ASSERT_TRUE(source_spans.empty()); } diff --git a/xls/dslx/frontend/parser.cc b/xls/dslx/frontend/parser.cc index d970bb05e1..38ca73f8ba 100644 --- a/xls/dslx/frontend/parser.cc +++ b/xls/dslx/frontend/parser.cc @@ -1291,7 +1291,8 @@ absl::StatusOr Parser::ParseBinopChain( XLS_ASSIGN_OR_RETURN(Expr * rhs, sub_production()); XLS_ASSIGN_OR_RETURN(BinopKind kind, BinopKindFromString(TokenKindToString(op.kind()))); - lhs = module_->Make(op.span(), kind, lhs, rhs); + Span span(lhs->span().start(), rhs->span().limit()); + lhs = module_->Make(span, kind, lhs, rhs); } else { break; } diff --git a/xls/dslx/ir_convert/testdata/function_converter_test_ConvertsSimpleFunctionWithAsserts.ir b/xls/dslx/ir_convert/testdata/function_converter_test_ConvertsSimpleFunctionWithAsserts.ir index 7ca6a5cafd..a125f1047d 100644 --- a/xls/dslx/ir_convert/testdata/function_converter_test_ConvertsSimpleFunctionWithAsserts.ir +++ b/xls/dslx/ir_convert/testdata/function_converter_test_ConvertsSimpleFunctionWithAsserts.ir @@ -10,11 +10,11 @@ fn __itok__test_module__f(__token: token id=1, __activated: bits[1] id=2) -> (to literal.23: bits[32] = literal(value=31, id=23, pos=[(0,3,26)]) literal.24: bits[32] = literal(value=1, id=24, pos=[(0,3,35)]) literal.3: bits[32] = literal(value=42, id=3, pos=[(0,1,16)]) - add.6: bits[32] = add(literal.4, literal.5, id=6, pos=[(0,1,33)]) + add.6: bits[32] = add(literal.4, literal.5, id=6, pos=[(0,1,26)]) literal.13: bits[32] = literal(value=42, id=13, pos=[(0,2,18)]) - add.16: bits[32] = add(literal.14, literal.15, id=16, pos=[(0,2,33)]) + add.16: bits[32] = add(literal.14, literal.15, id=16, pos=[(0,2,26)]) literal.22: bits[32] = literal(value=41, id=22, pos=[(0,3,18)]) - add.25: bits[32] = add(literal.23, literal.24, id=25, pos=[(0,3,33)]) + add.25: bits[32] = add(literal.23, literal.24, id=25, pos=[(0,3,26)]) not.9: bits[1] = not(__activated, id=9) eq.7: bits[1] = eq(literal.3, add.6, id=7, pos=[(0,1,23)]) not.18: bits[1] = not(__activated, id=18) diff --git a/xls/dslx/ir_convert/testdata/function_converter_test_TracksMultipleTypeAliasSvType.ir b/xls/dslx/ir_convert/testdata/function_converter_test_TracksMultipleTypeAliasSvType.ir index 67f5ce177e..b0de198a71 100644 --- a/xls/dslx/ir_convert/testdata/function_converter_test_TracksMultipleTypeAliasSvType.ir +++ b/xls/dslx/ir_convert/testdata/function_converter_test_TracksMultipleTypeAliasSvType.ir @@ -4,5 +4,5 @@ file_number 0 "test_module.x" top fn __test_module__f(b: bits[32] id=1) -> bits[32] { literal.2: bits[32] = literal(value=42, id=2, pos=[(0,3,56)]) - ret add.3: bits[32] = add(b, literal.2, id=3, pos=[(0,3,54)]) + ret add.3: bits[32] = add(b, literal.2, id=3, pos=[(0,3,52)]) } diff --git a/xls/dslx/ir_convert/testdata/function_converter_test_TracksTypeAliasStopsAtFirstSvType.ir b/xls/dslx/ir_convert/testdata/function_converter_test_TracksTypeAliasStopsAtFirstSvType.ir index 44347031d3..54cc15311d 100644 --- a/xls/dslx/ir_convert/testdata/function_converter_test_TracksTypeAliasStopsAtFirstSvType.ir +++ b/xls/dslx/ir_convert/testdata/function_converter_test_TracksTypeAliasStopsAtFirstSvType.ir @@ -4,5 +4,5 @@ file_number 0 "test_module.x" top fn __test_module__f(b: bits[32] id=1) -> bits[32] { literal.2: bits[32] = literal(value=42, id=2, pos=[(0,5,29)]) - ret add.3: bits[32] = add(b, literal.2, id=3, pos=[(0,5,27)]) + ret add.3: bits[32] = add(b, literal.2, id=3, pos=[(0,5,25)]) } diff --git a/xls/dslx/ir_convert/testdata/function_converter_test_TracksTypeAliasSvType.ir b/xls/dslx/ir_convert/testdata/function_converter_test_TracksTypeAliasSvType.ir index 7a30ba5b98..c8a0131961 100644 --- a/xls/dslx/ir_convert/testdata/function_converter_test_TracksTypeAliasSvType.ir +++ b/xls/dslx/ir_convert/testdata/function_converter_test_TracksTypeAliasSvType.ir @@ -4,5 +4,5 @@ file_number 0 "test_module.x" top fn __test_module__f(b: bits[32] id=1) -> bits[32] { literal.2: bits[32] = literal(value=42, id=2, pos=[(0,4,56)]) - ret add.3: bits[32] = add(b, literal.2, id=3, pos=[(0,4,54)]) + ret add.3: bits[32] = add(b, literal.2, id=3, pos=[(0,4,52)]) } diff --git a/xls/dslx/ir_convert/testdata/ir_converter_test_Concat.ir b/xls/dslx/ir_convert/testdata/ir_converter_test_Concat.ir index 8484057430..c4f34104e2 100644 --- a/xls/dslx/ir_convert/testdata/ir_converter_test_Concat.ir +++ b/xls/dslx/ir_convert/testdata/ir_converter_test_Concat.ir @@ -4,5 +4,5 @@ file_number 0 "test_module.x" top fn __test_module__f(x: bits[31] id=1) -> bits[32] { literal.2: bits[1] = literal(value=1, id=2, pos=[(0,1,2)]) - ret concat.3: bits[32] = concat(literal.2, x, id=3, pos=[(0,1,12)]) + ret concat.3: bits[32] = concat(literal.2, x, id=3, pos=[(0,1,2)]) } diff --git a/xls/dslx/ir_convert/testdata/ir_converter_test_InvocationMultiSymbol.ir b/xls/dslx/ir_convert/testdata/ir_converter_test_InvocationMultiSymbol.ir index 7bb728078b..bc27dfdc66 100644 --- a/xls/dslx/ir_convert/testdata/ir_converter_test_InvocationMultiSymbol.ir +++ b/xls/dslx/ir_convert/testdata/ir_converter_test_InvocationMultiSymbol.ir @@ -6,7 +6,7 @@ fn __test_module__parametric__3_5_8(x: bits[3] id=1, y: bits[5] id=2) -> bits[8] M: bits[32] = literal(value=3, id=3, pos=[(0,0,14)]) N: bits[32] = literal(value=5, id=4, pos=[(0,0,22)]) R: bits[32] = literal(value=8, id=5, pos=[(0,0,30)]) - ret concat.6: bits[8] = concat(x, y, id=6, pos=[(0,1,4)]) + ret concat.6: bits[8] = concat(x, y, id=6, pos=[(0,1,2)]) } fn __test_module__main() -> bits[8] { diff --git a/xls/dslx/ir_convert/testdata/ir_converter_test_InvokeMultipleArgs.ir b/xls/dslx/ir_convert/testdata/ir_converter_test_InvokeMultipleArgs.ir index d7d428fe9b..5fa8801374 100644 --- a/xls/dslx/ir_convert/testdata/ir_converter_test_InvokeMultipleArgs.ir +++ b/xls/dslx/ir_convert/testdata/ir_converter_test_InvokeMultipleArgs.ir @@ -3,7 +3,7 @@ package test_module file_number 0 "test_module.x" fn __test_module__callee(x: bits[32] id=1, y: bits[32] id=2) -> bits[32] { - ret add.3: bits[32] = add(x, y, id=3, pos=[(0,1,4)]) + ret add.3: bits[32] = add(x, y, id=3, pos=[(0,1,2)]) } fn __test_module__caller() -> bits[32] { diff --git a/xls/dslx/ir_convert/testdata/ir_converter_test_LetBinding.ir b/xls/dslx/ir_convert/testdata/ir_converter_test_LetBinding.ir index 694cddca5a..44adf27ada 100644 --- a/xls/dslx/ir_convert/testdata/ir_converter_test_LetBinding.ir +++ b/xls/dslx/ir_convert/testdata/ir_converter_test_LetBinding.ir @@ -4,5 +4,5 @@ file_number 0 "test_module.x" top fn __test_module__f() -> bits[32] { x: bits[32] = literal(value=2, id=1, pos=[(0,1,15)]) - ret add.2: bits[32] = add(x, x, id=2, pos=[(0,2,3)]) + ret add.2: bits[32] = add(x, x, id=2, pos=[(0,2,2)]) } diff --git a/xls/dslx/ir_convert/testdata/ir_converter_test_LetTupleBinding.ir b/xls/dslx/ir_convert/testdata/ir_converter_test_LetTupleBinding.ir index 4daedd2206..c0c5f5769c 100644 --- a/xls/dslx/ir_convert/testdata/ir_converter_test_LetTupleBinding.ir +++ b/xls/dslx/ir_convert/testdata/ir_converter_test_LetTupleBinding.ir @@ -8,5 +8,5 @@ top fn __test_module__f() -> bits[32] { t: (bits[32], bits[32]) = tuple(literal.1, literal.2, id=3, pos=[(0,1,10)]) x: bits[32] = tuple_index(t, index=0, id=4, pos=[(0,2,7)]) y: bits[32] = tuple_index(t, index=1, id=5, pos=[(0,2,10)]) - ret add.6: bits[32] = add(x, y, id=6, pos=[(0,3,3)]) + ret add.6: bits[32] = add(x, y, id=6, pos=[(0,3,2)]) } diff --git a/xls/dslx/ir_convert/testdata/ir_converter_test_SignedDiv.ir b/xls/dslx/ir_convert/testdata/ir_converter_test_SignedDiv.ir index 726b591d2a..3ad6d097cc 100644 --- a/xls/dslx/ir_convert/testdata/ir_converter_test_SignedDiv.ir +++ b/xls/dslx/ir_convert/testdata/ir_converter_test_SignedDiv.ir @@ -3,5 +3,5 @@ package test_module file_number 0 "test_module.x" top fn __test_module__signed_div(x: bits[32] id=1, y: bits[32] id=2) -> bits[32] { - ret sdiv.3: bits[32] = sdiv(x, y, id=3, pos=[(0,1,4)]) + ret sdiv.3: bits[32] = sdiv(x, y, id=3, pos=[(0,1,2)]) } diff --git a/xls/dslx/ir_convert/testdata/ir_converter_test_TwoPlusTwo.ir b/xls/dslx/ir_convert/testdata/ir_converter_test_TwoPlusTwo.ir index 52d38c87f8..8c24e40e8b 100644 --- a/xls/dslx/ir_convert/testdata/ir_converter_test_TwoPlusTwo.ir +++ b/xls/dslx/ir_convert/testdata/ir_converter_test_TwoPlusTwo.ir @@ -5,5 +5,5 @@ file_number 0 "test_module.x" top fn __test_module__two_plus_two() -> bits[32] { literal.1: bits[32] = literal(value=2, id=1, pos=[(0,1,2)]) literal.2: bits[32] = literal(value=2, id=2, pos=[(0,1,10)]) - ret add.3: bits[32] = add(literal.1, literal.2, id=3, pos=[(0,1,8)]) + ret add.3: bits[32] = add(literal.1, literal.2, id=3, pos=[(0,1,2)]) } diff --git a/xls/dslx/tests/errors/error_modules_test.py b/xls/dslx/tests/errors/error_modules_test.py index 5d43b197a4..84a416b6bb 100644 --- a/xls/dslx/tests/errors/error_modules_test.py +++ b/xls/dslx/tests/errors/error_modules_test.py @@ -599,7 +599,7 @@ def test_useless_expression_statement_warning(self): warnings_as_errors=False, want_err_retcode=False, ) - self.assertIn('useless_expression_statement.x:19:9-19:11', stderr) + self.assertIn('useless_expression_statement.x:19:5-19:16', stderr) self.assertIn( 'Expression statement `bar || abcd` appears useless (i.e. has no' ' side-effects)', @@ -873,7 +873,7 @@ def test_diag_block_with_trailing_semi(self): stderr = self._run( 'xls/dslx/tests/errors/add_to_name_from_trailing_semi_block.x' ) - self.assertIn('add_to_name_from_trailing_semi_block.x:19:5-19:6', stderr) + self.assertIn('add_to_name_from_trailing_semi_block.x:19:3-19:13', stderr) self.assertIn( 'note that "x" was defined by a block with a trailing semicolon', stderr ) diff --git a/xls/flows/testdata/ir_wrapper_test_DslxProcsToIrOk.ir b/xls/flows/testdata/ir_wrapper_test_DslxProcsToIrOk.ir index 6167d003e3..501d1ae5fc 100644 --- a/xls/flows/testdata/ir_wrapper_test_DslxProcsToIrOk.ir +++ b/xls/flows/testdata/ir_wrapper_test_DslxProcsToIrOk.ir @@ -14,6 +14,6 @@ proc __top__foo_0_next() { a: bits[32] = tuple_index(receive.17, index=1, id=8, pos=[(0,12,18)]) b: bits[32] = tuple_index(receive.18, index=1, id=12, pos=[(0,13,18)]) tok__2: token = tuple_index(receive.18, index=0, id=11, pos=[(0,13,13)]) - add.13: bits[32] = add(a, b, id=13, pos=[(0,14,38)]) + add.13: bits[32] = add(a, b, id=13, pos=[(0,14,36)]) tok__3: token = send(tok__2, add.13, channel=test_package__output, id=19) }