From ad30d03b8c4115905e465aba45a86299ea75ec9d Mon Sep 17 00:00:00 2001 From: mwilliams Date: Thu, 18 Apr 2013 11:01:36 -0700 Subject: [PATCH] Fix order of evaluation for unused binary operators eg (new X) == (new X); Was converted to something like: (new X); (new X); Which calls the destructor for the first X before constructing the second. I tried a fix where we used an ExpressionList with ListKindLeft, which would preserve both expressions until after both objects are created; but that ends up calling the second object's destructor first. Thats much better; its not clear to me that there's any guarantee about which object's destructor is called first when they both go out of scope at the same point; but currently hhvm and zend seem to aggree, so Im going with a solution that preserves the left-to-right order. --- hphp/compiler/expression/binary_op_expression.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/hphp/compiler/expression/binary_op_expression.cpp b/hphp/compiler/expression/binary_op_expression.cpp index a843b2d66..08aacb0fb 100644 --- a/hphp/compiler/expression/binary_op_expression.cpp +++ b/hphp/compiler/expression/binary_op_expression.cpp @@ -163,12 +163,16 @@ bool BinaryOpExpression::isLogicalOrOperator() const { } ExpressionPtr BinaryOpExpression::unneededHelper() { - if (!isShortCircuitOperator() || !m_exp2->getContainedEffects()) { + bool shortCircuit = isShortCircuitOperator(); + if (!m_exp2->getContainedEffects() || + (!shortCircuit && !m_exp1->getContainedEffects())) { return Expression::unneededHelper(); } - m_exp2 = m_exp2->unneeded(); - m_exp2->setExpectedType(Type::Boolean); + if (shortCircuit) { + m_exp2 = m_exp2->unneeded(); + m_exp2->setExpectedType(Type::Boolean); + } return static_pointer_cast(shared_from_this()); }