Finished refactoring of expressions.

include/plasp/pddl/expressions/PrimitiveType.h

@@ -22,11 +22,11 @@ namespace expressions
 class PrimitiveType: public Expression
 {
 	public:
-		static PrimitiveTypePointer parseDeclaration(utils::Parser &parser, Context &context);
+		static PrimitiveType *parseDeclaration(utils::Parser &parser, Context &context);
 		static void parseTypedDeclaration(utils::Parser &parser, Context &context);
 
 		template<class Container>
-		static PrimitiveType *parseExisting(utils::Parser &parser, const Container &primitiveTypes);
+		static PrimitiveType *parseExisting(utils::Parser &parser, Container &primitiveTypes);
 
 		// TODO: method for lazy creation if not existing
 
@@ -46,8 +46,6 @@ class PrimitiveType: public Expression
 
 		void setDeclared();
 
-		void addParentType(const PrimitiveType *parentType);
-
 		bool m_isDirty;
 		bool m_isDeclared;
 
@@ -60,22 +58,39 @@ class PrimitiveType: public Expression
 
 template<class Container>
 PrimitiveType *PrimitiveType::parseExisting(utils::Parser &parser,
-	const Container &primitiveTypes)
+	Container &primitiveTypes)
 {
 	parser.skipWhiteSpace();
 
 	const auto typeName = parser.parseIdentifier(isIdentifier);
+
+	BOOST_ASSERT(!typeName.empty());
+
 	// TODO: use hash map
 	const auto match = std::find_if(primitiveTypes.cbegin(), primitiveTypes.cend(),
 		[&](const auto &primitiveType)
 		{
 			return primitiveType->name() == typeName;
 		});
-	const auto typeExists = (match != primitiveTypes.cend());
 
-	// Return existing primitive types
+	if (match == primitiveTypes.cend())
-	if (!typeExists)
+	{
-		throw utils::ParserException(parser.row(), parser.column(), "Primitive type \"" + typeName + "\" used but never declared");
+		// Allow implicit type "object"
+		if (typeName == "object")
+		{
+			auto objectType = std::make_unique<PrimitiveType>(PrimitiveType());
+			objectType->m_name = typeName;
+			objectType->setDirty(false);
+			objectType->setDeclared();
+
+			// TODO: make std::vector-independent
+			primitiveTypes.emplace_back(std::move(objectType));
+
+			return primitiveTypes.back().get();
+		}
+		else
+			throw utils::ParserException(parser.row(), parser.column(), "Primitive type \"" + typeName + "\" used but never declared");
+	}
 
 	auto *type = match->get();
 	type->setDirty();

src/plasp/pddl/Domain.cpp

@@ -130,6 +130,8 @@ void Domain::parseSection(utils::Parser &parser)
 			}
 		};
 
+	// TODO: make naming consistent
+	// TODO: check order of the sections
 	if (sectionIdentifier == "requirements")
 		parseRequirementsSection(parser);
 	else if (sectionIdentifier == "types")
@@ -258,7 +260,7 @@ void Domain::parseConstantSection(utils::Parser &parser)
 	// Store constants
 	while (parser.currentCharacter() != ')')
 	{
-		expressions::Constant::parseDeclaration(parser, m_context);
+		expressions::Constant::parseTypedDeclaration(parser, m_context);
 
 		parser.skipWhiteSpace();
 	}

src/plasp/pddl/expressions/Constant.cpp

@@ -36,6 +36,8 @@ ConstantPointer Constant::parseDeclaration(utils::Parser &parser, Context &conte
 
 	constant->m_name = parser.parseIdentifier(isIdentifier);
 
+	BOOST_ASSERT(constant->m_name != "-");
+
 	// Flag constant for potentially upcoming type declaration
 	constant->setDirty();
 

src/plasp/pddl/expressions/PrimitiveType.cpp

@@ -26,31 +26,43 @@ PrimitiveType::PrimitiveType()
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 
-PrimitiveTypePointer PrimitiveType::parseDeclaration(utils::Parser &parser, Context &context)
+PrimitiveType *PrimitiveType::parseDeclaration(utils::Parser &parser, Context &context)
 {
-	// TODO: refactor
-	if (context.primitiveTypes.empty())
-	{
-		auto object = std::make_unique<PrimitiveType>(PrimitiveType());
-		object->m_name = "object";
-		object->setDirty();
-		object->setDeclared();
-
-		context.primitiveTypes.emplace_back(std::move(object));
-	}
-
 	parser.skipWhiteSpace();
 
+	const auto typeName = parser.parseIdentifier(isIdentifier);
+
+	// TODO: use hash map
+	const auto match = std::find_if(context.primitiveTypes.cbegin(), context.primitiveTypes.cend(),
+		[&](const auto &primitiveType)
+		{
+			return primitiveType->name() == typeName;
+		});
+
+	// Return existing primitive type
+	if (match != context.primitiveTypes.cend())
+	{
+		auto *type = match->get();
+
+		type->setDirty();
+
+		return type;
+	}
+
+	// Create new primitive type if not already existing
 	auto type = std::make_unique<PrimitiveType>(PrimitiveType());
 
-	type->m_name = parser.parseIdentifier(isIdentifier);
+	type->m_name = typeName;
+
+	BOOST_ASSERT(!type->m_name.empty());
 
 	// Flag type for potentially upcoming parent type declaration
 	type->setDirty();
 
 	// TODO: Store constant in hash map
+	context.primitiveTypes.emplace_back(std::move(type));
 
-	return type;
+	return context.primitiveTypes.back().get();
 }
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
@@ -58,9 +70,7 @@ PrimitiveTypePointer PrimitiveType::parseDeclaration(utils::Parser &parser, Cont
 void PrimitiveType::parseTypedDeclaration(utils::Parser &parser, Context &context)
 {
 	// Parse and store type
-	context.primitiveTypes.emplace_back(parseDeclaration(parser, context));
+	auto *type = parseDeclaration(parser, context);
-
-	const auto &type = context.primitiveTypes.back();
 
 	// Flag type as correctly declared in the types section
 	type->setDeclared();
@@ -86,7 +96,7 @@ void PrimitiveType::parseTypedDeclaration(utils::Parser &parser, Context &contex
 			if (!childType->isDirty())
 				return;
 
-			childType->addParentType(parentType);
+			childType->m_parentTypes.push_back(parentType);
 			childType->setDirty(false);
 		});
 }
@@ -135,13 +145,6 @@ const std::string &PrimitiveType::name() const
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 
-void PrimitiveType::addParentType(const PrimitiveType *parentType)
-{
-	m_parentTypes.push_back(parentType);
-}
-
-////////////////////////////////////////////////////////////////////////////////////////////////////
-
 const std::vector<const PrimitiveType *> &PrimitiveType::parentTypes() const
 {
 	return m_parentTypes;

src/plasp/pddl/expressions/Variable.cpp

@@ -77,11 +77,18 @@ void Variable::parseTypedDeclaration(utils::Parser &parser, Context &context, Va
 				});
 		};
 
+	parser.skipWhiteSpace();
+
 	// Parse argument of "either" type (always begins with opening parenthesis)
 	if (parser.currentCharacter() == '(')
 	{
+		parser.expect<std::string>("(");
+		parser.expect<std::string>("either");
+
 		parameter->m_eitherExpression = Either::parse(parser, context, parameters, parseExistingPrimitiveType);
 
+		parser.expect<std::string>(")");
+
 		setType(parameter->m_eitherExpression.get());
 		return;
 	}

tests/TestPDDLParser.cpp

@@ -6,8 +6,9 @@
 #include <stdexcept>
 
 #include <plasp/pddl/Description.h>
-#include <plasp/pddl/expressions/PrimitiveType.h>
 #include <plasp/pddl/expressions/Either.h>
+#include <plasp/pddl/expressions/PrimitiveType.h>
+#include <plasp/pddl/expressions/Reference.h>
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 
@@ -46,9 +47,11 @@ class PDDLParserTests : public ::testing::Test
 
 TEST_F(PDDLParserTests, ParseBlocksWorldDomain)
 {
+	using namespace plasp::pddl;
+
 	try
 	{
-		const auto description = plasp::pddl::Description::fromStream(m_blocksworldDomainFile);
+		const auto description = Description::fromStream(m_blocksworldDomainFile);
 
 		ASSERT_NO_THROW(description.domain());
 
@@ -59,8 +62,8 @@ TEST_F(PDDLParserTests, ParseBlocksWorldDomain)
 
 		// Requirements
 		ASSERT_EQ(domain.requirements().size(), 2u);
-		ASSERT_EQ(domain.requirements()[0].type(), plasp::pddl::Requirement::Type::STRIPS);
+		ASSERT_EQ(domain.requirements()[0].type(), Requirement::Type::STRIPS);
-		ASSERT_EQ(domain.requirements()[1].type(), plasp::pddl::Requirement::Type::Typing);
+		ASSERT_EQ(domain.requirements()[1].type(), Requirement::Type::Typing);
 
 		// Types
 		ASSERT_EQ(domain.types().size(), 1u);
@@ -78,10 +81,10 @@ TEST_F(PDDLParserTests, ParseBlocksWorldDomain)
 		ASSERT_EQ(on.name(), "on");
 		ASSERT_EQ(on.arguments().size(), 2u);
 		ASSERT_EQ(on.arguments()[0]->name(), "x");
-		const auto *onArgument0Type = dynamic_cast<const plasp::pddl::expressions::PrimitiveType *>(on.arguments()[0]->type());
+		const auto *onArgument0Type = dynamic_cast<const expressions::PrimitiveType *>(on.arguments()[0]->type());
 		ASSERT_EQ(onArgument0Type, &block);
 		ASSERT_EQ(on.arguments()[1]->name(), "y");
-		const auto onArgument1Type = dynamic_cast<const plasp::pddl::expressions::PrimitiveType *>(on.arguments()[1]->type());
+		const auto onArgument1Type = dynamic_cast<const expressions::PrimitiveType *>(on.arguments()[1]->type());
 		ASSERT_EQ(onArgument1Type, &block);
 
 		const auto &handempty = *domain.predicates()[3];
@@ -99,6 +102,8 @@ TEST_F(PDDLParserTests, ParseBlocksWorldDomain)
 
 TEST_F(PDDLParserTests, ParseStorageDomain)
 {
+	using namespace plasp::pddl;
+
 	try
 	{
 		const auto description = plasp::pddl::Description::fromStream(m_storageDomainFile);
@@ -112,7 +117,7 @@ TEST_F(PDDLParserTests, ParseStorageDomain)
 
 		// Requirements
 		ASSERT_EQ(domain.requirements().size(), 1u);
-		ASSERT_EQ(domain.requirements()[0].type(), plasp::pddl::Requirement::Type::Typing);
+		ASSERT_EQ(domain.requirements()[0].type(), Requirement::Type::Typing);
 
 		// Types
 		ASSERT_EQ(domain.types().size(), 10u);
@@ -141,20 +146,20 @@ TEST_F(PDDLParserTests, ParseStorageDomain)
 		ASSERT_EQ(on.name(), "on");
 		ASSERT_EQ(on.arguments().size(), 2u);
 		ASSERT_EQ(on.arguments()[0]->name(), "c");
-		const auto onArgument0Type = dynamic_cast<const plasp::pddl::expressions::PrimitiveType *>(on.arguments()[0]->type());
+		const auto onArgument0Type = dynamic_cast<const expressions::PrimitiveType *>(on.arguments()[0]->type());
 		ASSERT_EQ(onArgument0Type, &crate);
 		ASSERT_EQ(on.arguments()[1]->name(), "s");
-		const auto onArgument1Type = dynamic_cast<const plasp::pddl::expressions::PrimitiveType *>(on.arguments()[1]->type());
+		const auto onArgument1Type = dynamic_cast<const expressions::PrimitiveType *>(on.arguments()[1]->type());
 		ASSERT_EQ(onArgument1Type, &storearea);
 
 		const auto &in = *domain.predicates()[1];
 		ASSERT_EQ(in.name(), "in");
 		ASSERT_EQ(in.arguments().size(), 2u);
 		ASSERT_EQ(in.arguments()[0]->name(), "x");
-		const auto inArgument0Type = dynamic_cast<const plasp::pddl::expressions::Either *>(in.arguments()[0]->type());
+		const auto inArgument0Type = dynamic_cast<const expressions::Either *>(in.arguments()[0]->type());
 		ASSERT_EQ(inArgument0Type->arguments().size(), 2u);
-		ASSERT_EQ(dynamic_cast<const plasp::pddl::expressions::PrimitiveType *>(inArgument0Type->arguments()[0].get()), &storearea);
+		ASSERT_EQ(dynamic_cast<const expressions::Reference<expressions::PrimitiveType> *>(inArgument0Type->arguments()[0].get())->value(), &storearea);
-		ASSERT_EQ(dynamic_cast<const plasp::pddl::expressions::PrimitiveType *>(inArgument0Type->arguments()[1].get()), &crate);
+		ASSERT_EQ(dynamic_cast<const expressions::Reference<expressions::PrimitiveType> *>(inArgument0Type->arguments()[1].get())->value(), &crate);
 	}
 	catch (const std::exception &e)
 	{
@@ -166,9 +171,11 @@ TEST_F(PDDLParserTests, ParseStorageDomain)
 
 TEST_F(PDDLParserTests, ParseConstants)
 {
+	using namespace plasp::pddl;
+
 	try
 	{
-		const auto description = plasp::pddl::Description::fromStream(m_woodworkingDomainFile);
+		const auto description = Description::fromStream(m_woodworkingDomainFile);
 
 		ASSERT_NO_THROW(description.domain());
 
@@ -200,9 +207,11 @@ TEST_F(PDDLParserTests, ParseConstants)
 
 TEST_F(PDDLParserTests, ParseWithWhiteSpace)
 {
+	using namespace plasp::pddl;
+
 	try
 	{
-		ASSERT_NO_THROW(plasp::pddl::Description::fromStream(m_whiteSpaceTestFile));
+		ASSERT_NO_THROW(Description::fromStream(m_whiteSpaceTestFile));
 	}
 	catch (const std::exception &e)
 	{