From 0b31d8d56ce55d4efb9f8a6a4c27af93de8b85d3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Patrick=20L=C3=BChne?= <patrick-github@luehne.de>
Date: Thu, 8 Dec 2016 02:54:28 +0100
Subject: [PATCH] Fixed bugs related in decomposition of expressions.

---
 src/plasp/pddl/Action.cpp                       |  2 ++
 src/plasp/pddl/expressions/And.cpp              |  9 +++++++--
 src/plasp/pddl/expressions/DerivedPredicate.cpp |  5 ++++-
 src/plasp/pddl/expressions/Not.cpp              |  2 +-
 src/plasp/pddl/expressions/Or.cpp               |  9 +++------
 src/plasp/pddl/expressions/Variable.cpp         | 17 ++++++++---------
 6 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/src/plasp/pddl/Action.cpp b/src/plasp/pddl/Action.cpp
index 1d86e51..0f5816b 100644
--- a/src/plasp/pddl/Action.cpp
+++ b/src/plasp/pddl/Action.cpp
@@ -93,6 +93,8 @@ void Action::normalize(expressions::DerivedPredicates &derivedPredicates)
 	// Normalize preconditions and effects
 	m_precondition = m_precondition->normalized()->decomposed(derivedPredicates);
 	{
+		m_precondition->print(std::cout);
+
 		// TODO: refactor
 		std::set<expressions::VariablePointer> parameters;
 		m_precondition->collectParameters(parameters);
diff --git a/src/plasp/pddl/expressions/And.cpp b/src/plasp/pddl/expressions/And.cpp
index 38d07c3..df7dfcf 100644
--- a/src/plasp/pddl/expressions/And.cpp
+++ b/src/plasp/pddl/expressions/And.cpp
@@ -25,14 +25,19 @@ const std::string And::Identifier = "and";
 
 ExpressionPointer And::decomposed(DerivedPredicates &derivedPredicates)
 {
+	BOOST_ASSERT(!m_arguments.empty());
+
 	derivedPredicates.emplace_back(new DerivedPredicate(derivedPredicates.size()));
-	auto &derivedPredicate = derivedPredicates.back();
+	auto derivedPredicate = derivedPredicates.back();
 
 	for (auto &argument : m_arguments)
 		argument = argument->decomposed(derivedPredicates);
 
+	std::vector<Expressions> preconditions;
+	preconditions.emplace_back(std::move(m_arguments));
+
 	// Move this expression’s arguments to the derived predicate
-	derivedPredicate->setPreconditions({m_arguments});
+	derivedPredicate->setPreconditions(std::move(preconditions));
 
 	return derivedPredicate;
 }
diff --git a/src/plasp/pddl/expressions/DerivedPredicate.cpp b/src/plasp/pddl/expressions/DerivedPredicate.cpp
index 2decbbe..02e7e63 100644
--- a/src/plasp/pddl/expressions/DerivedPredicate.cpp
+++ b/src/plasp/pddl/expressions/DerivedPredicate.cpp
@@ -64,7 +64,10 @@ void DerivedPredicate::collectParameters(std::set<VariablePointer> &parameters)
 {
 	for (auto &conjunction : m_preconditions)
 		for (auto &precondition : conjunction)
+		{
+			BOOST_ASSERT(precondition.get() != this);
 			precondition->collectParameters(m_parameters);
+		}
 
 	// Copy in order not to interfere with potentially bound variables in parent expressions
 	parameters = m_parameters;
@@ -74,7 +77,7 @@ void DerivedPredicate::collectParameters(std::set<VariablePointer> &parameters)
 
 void DerivedPredicate::print(std::ostream &ostream) const
 {
-	ostream << "(:derived <no name>";
+	ostream << "(:derived " << m_id << " ";
 
 	BOOST_ASSERT(m_preconditions.size() > 0);
 
diff --git a/src/plasp/pddl/expressions/Not.cpp b/src/plasp/pddl/expressions/Not.cpp
index fd77570..126ce05 100644
--- a/src/plasp/pddl/expressions/Not.cpp
+++ b/src/plasp/pddl/expressions/Not.cpp
@@ -108,7 +108,7 @@ ExpressionPointer Not::decomposed(DerivedPredicates &derivedPredicates)
 		return this;
 
 	derivedPredicates.emplace_back(new DerivedPredicate(derivedPredicates.size()));
-	auto &derivedPredicate = derivedPredicates.back();
+	auto derivedPredicate = derivedPredicates.back();
 
 	// Move this expression’s arguments to the derived predicate
 	derivedPredicate->setPreconditions({{this}});
diff --git a/src/plasp/pddl/expressions/Or.cpp b/src/plasp/pddl/expressions/Or.cpp
index 98fc5d6..3b090f3 100644
--- a/src/plasp/pddl/expressions/Or.cpp
+++ b/src/plasp/pddl/expressions/Or.cpp
@@ -23,8 +23,10 @@ const std::string Or::Identifier = "or";
 
 ExpressionPointer Or::decomposed(DerivedPredicates &derivedPredicates)
 {
+	BOOST_ASSERT(!m_arguments.empty());
+
 	derivedPredicates.emplace_back(new DerivedPredicate(derivedPredicates.size()));
-	auto &derivedPredicate = derivedPredicates.back();
+	auto derivedPredicate = derivedPredicates.back();
 
 	std::vector<Expressions> preconditions;
 
@@ -39,14 +41,9 @@ ExpressionPointer Or::decomposed(DerivedPredicates &derivedPredicates)
 
 			for (auto &argument : conjunction)
 				argument = argument->decomposed(derivedPredicates);
-
-			break;
 		}
 		else
-		{
 			conjunction.emplace_back(argument->decomposed(derivedPredicates));
-			break;
-		}
 
 		// Move this expression’s arguments to the derived predicate
 		preconditions.emplace_back(std::move(conjunction));
diff --git a/src/plasp/pddl/expressions/Variable.cpp b/src/plasp/pddl/expressions/Variable.cpp
index 1df7636..15b70e9 100644
--- a/src/plasp/pddl/expressions/Variable.cpp
+++ b/src/plasp/pddl/expressions/Variable.cpp
@@ -78,7 +78,7 @@ void Variable::parseTypedDeclaration(Context &context, ExpressionContext &expres
 	// Parse and store variable itself
 	parseDeclaration(context, variables);
 
-	auto &variable = variables.back();
+	auto variable = variables.back();
 
 	parser.skipWhiteSpace();
 
@@ -90,15 +90,14 @@ void Variable::parseTypedDeclaration(Context &context, ExpressionContext &expres
 		[&](ExpressionPointer type)
 		{
 			// Set the argument type for all previously flagged arguments
-			std::for_each(variables.begin(), variables.end(),
-				[&](auto &variable)
-				{
-					if (!variable->isDirty())
-						return;
+			for (auto &variable : variables)
+			{
+				if (!variable->isDirty())
+					return;
 
-					variable->setType(type);
-					variable->setDirty(false);
-				});
+				variable->setType(type);
+				variable->setDirty(false);
+			}
 		};
 
 	parser.skipWhiteSpace();