Skip to content

Commit 36e32ea

Browse files
facontidavideclaude
andcommitted
Fix #1029: correct left-associativity for arithmetic operators in script parser
The scripting engine evaluated "A - B + C" as "A - (B + C)" instead of "(A - B) + C". The root cause was that string_concat's operand chain referenced math_sum, causing it to appear twice in lexy's operation list with conflicting binding power levels. Changed string_concat::operand from math_sum to math_prefix to eliminate the duplicate entry. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent b3dbe30 commit 36e32ea

File tree

2 files changed

+40
-1
lines changed

2 files changed

+40
-1
lines changed

include/behaviortree_cpp/scripting/operators.hpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,10 @@ struct Expression : lexy::expression_production
747747
return dsl::op<Ast::ExprBinaryArithmetic::concat>(LEXY_LIT(".."));
748748
}();
749749

750-
using operand = math_sum;
750+
// Use math_prefix (not math_sum) to avoid duplicating math_sum/math_product
751+
// in the operation list, which would break left-associativity of +/-/*//
752+
// due to conflicting binding power levels in lexy's _operation_list_of.
753+
using operand = math_prefix;
751754
};
752755

753756
// ~x

tests/script_parser_test.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,42 @@ TEST(ParserTest, Issue595)
424424
ASSERT_EQ(0, counters[0]);
425425
}
426426

427+
// https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1029
428+
TEST(ParserTest, OperatorAssociativity_Issue1029)
429+
{
430+
BT::Ast::Environment environment = { BT::Blackboard::create(), {} };
431+
432+
auto GetResult = [&environment](const char* text) -> BT::Any {
433+
return GetScriptResult(environment, text);
434+
};
435+
436+
// Addition and subtraction are left-associative:
437+
// "5 - 2 + 1" should be (5-2)+1 = 4, NOT 5-(2+1) = 2
438+
EXPECT_EQ(GetResult("5 - 2 + 1").cast<double>(), 4.0);
439+
440+
// "10 - 3 - 2" should be (10-3)-2 = 5, NOT 10-(3-2) = 9
441+
EXPECT_EQ(GetResult("10 - 3 - 2").cast<double>(), 5.0);
442+
443+
// "2 + 3 - 1" should be (2+3)-1 = 4
444+
EXPECT_EQ(GetResult("2 + 3 - 1").cast<double>(), 4.0);
445+
446+
// Multiplication and division are also left-associative:
447+
// "12 / 3 / 2" should be (12/3)/2 = 2, NOT 12/(3/2) = 8
448+
EXPECT_EQ(GetResult("12 / 3 / 2").cast<double>(), 2.0);
449+
450+
// "12 / 3 * 2" should be (12/3)*2 = 8, NOT 12/(3*2) = 2
451+
EXPECT_EQ(GetResult("12 / 3 * 2").cast<double>(), 8.0);
452+
453+
// Mixed precedence: "2 + 3 * 4 - 1" should be 2+(3*4)-1 = 13
454+
EXPECT_EQ(GetResult("2 + 3 * 4 - 1").cast<double>(), 13.0);
455+
456+
// Verify string concatenation operator (..) still works after operand fix
457+
EXPECT_EQ(GetResult("A:='hello'; B:=' world'; A .. B").cast<std::string>(), "hello "
458+
"world");
459+
// Chained concatenation (left-associative)
460+
EXPECT_EQ(GetResult("A .. ' ' .. B").cast<std::string>(), "hello world");
461+
}
462+
427463
TEST(ParserTest, NewLine)
428464
{
429465
BT::BehaviorTreeFactory factory;

0 commit comments

Comments
 (0)