Define move semantics for peg::ast<> and return by value

Author: wravery

GraphQLService.cpp

@@ -1317,7 +1317,7 @@ void OperationDefinitionVisitor::visit(const peg::ast_node& operationDefinition)
 }
 
 SubscriptionData::SubscriptionData(std::shared_ptr<OperationData>&& data, std::unordered_map<SubscriptionName, std::vector<response::Value>>&& fieldNamesAndArgs,
-	std::unique_ptr<peg::ast<std::string>>&& query, std::string&& operationName, SubscriptionCallback&& callback,
+	peg::ast<std::string>&& query, std::string&& operationName, SubscriptionCallback&& callback,
 	const peg::ast_node& selection)
 	: data(std::move(data))
 	, fieldNamesAndArgs(std::move(fieldNamesAndArgs))
@@ -1363,7 +1363,7 @@ SubscriptionDefinitionVisitor::SubscriptionDefinitionVisitor(SubscriptionParams&
 
 const peg::ast_node& SubscriptionDefinitionVisitor::getRoot() const
 {
-	return *_params.query->root;
+	return *_params.query.root;
 }
 
 std::shared_ptr<SubscriptionData> SubscriptionDefinitionVisitor::getRegistration()
@@ -1645,7 +1645,7 @@ SubscriptionKey Request::subscribe(SubscriptionParams&& params, SubscriptionCall
 
 	FragmentDefinitionVisitor fragmentVisitor(params.variables);
 
-	peg::for_each_child<peg::fragment_definition>(*params.query->root,
+	peg::for_each_child<peg::fragment_definition>(*params.query.root,
 		[&fragmentVisitor](const peg::ast_node& child)
 		{
 			fragmentVisitor.visit(child);

GraphQLTree.cpp

@@ -561,38 +561,54 @@ struct ast_selector<input_object_type_extension>
 {
 };
 
-std::unique_ptr<ast<std::string>> parseString(std::string&& input)
+template <>
+ast<std::string>::~ast()
 {
-	std::unique_ptr<ast<std::string>> result(new ast<std::string> { std::move(input), nullptr });
-	memory_input<> in(result->input.c_str(), result->input.size(), "GraphQL");
+	// The default destructor gets inlined and may use a different allocator to free ast<>'s member
+	// variables than the graphqlservice module used to allocate them. So even though this could be
+	// omitted, declare it explicitly and define it in graphqlservice.
+}
 
-	result->root = parse_tree::parse<document, ast_node, ast_selector>(std::move(in));
+template <>
+ast<std::unique_ptr<file_input<>>>::~ast()
+{
+	// The default destructor gets inlined and may use a different allocator to free ast<>'s member
+	// variables than the graphqlservice module used to allocate them. So even though this could be
+	// omitted, declare it explicitly and define it in graphqlservice.
+}
 
-	return result;
+template <>
+ast<const char*>::~ast()
+{
+	// The default destructor gets inlined and may use a different allocator to free ast<>'s member
+	// variables than the graphqlservice module used to allocate them. So even though this could be
+	// omitted, declare it explicitly and define it in graphqlservice.
 }
 
-std::unique_ptr<ast<std::unique_ptr<file_input<>>>> parseFile(const char* filename)
+ast<std::string> parseString(std::string&& input)
 {
-	std::unique_ptr<ast<std::unique_ptr<file_input<>>>> result(new ast<std::unique_ptr<file_input<>>> {
-		std::unique_ptr<file_input<>>(new file_input<>(std::string(filename))),
-		nullptr
-		});
+	memory_input<> in(input.c_str(), input.size(), "GraphQL");
+
+	return { std::move(input), parse_tree::parse<document, ast_node, ast_selector>(std::move(in)) };
+}
 
-	result->root = parse_tree::parse<document, ast_node, ast_selector>(std::move(*result->input));
+ast<std::unique_ptr<file_input<>>> parseFile(const char* filename)
+{
+	std::unique_ptr<file_input<>> in(new file_input<>(std::string(filename)));
+	ast<std::unique_ptr<file_input<>>> result { std::move(in), nullptr };
+
+	result.root = parse_tree::parse<document, ast_node, ast_selector>(std::move(*result.input));
 
 	return result;
 }
 
 } /* namespace peg */
 
-std::unique_ptr<peg::ast<const char*>> operator "" _graphql(const char* text, size_t size)
+peg::ast<const char*> operator "" _graphql(const char* text, size_t size)
 {
-	std::unique_ptr<peg::ast<const char*>> result(new peg::ast<const char*> { text, nullptr });
 	peg::memory_input<> in(text, size, "GraphQL");
 
-	result->root = peg::parse_tree::parse<peg::document, peg::ast_node, peg::ast_selector>(std::move(in));
-
-	return result;
+	return { text, peg::parse_tree::parse<peg::document, peg::ast_node, peg::ast_selector>(std::move(in)) };
 }
 
 } /* namespace graphql */

SchemaGenerator.cpp

@@ -135,12 +135,12 @@ Generator::Generator()
 		  INPUT_FIELD_DEFINITION
 		})"_graphql;
 
-	if (!ast)
+	if (!ast.root)
 	{
 		throw std::logic_error("Unable to parse the introspection schema, but there was no error message from the parser!");
 	}
 
-	for (const auto& child : ast->root->children)
+	for (const auto& child : ast.root->children)
 	{
 		visitDefinition(*child);
 	}
@@ -158,12 +158,12 @@ Generator::Generator(std::string schemaFileName, std::string filenamePrefix, std
 {
 	auto ast = peg::parseFile(schemaFileName.c_str());
 
-	if (!ast)
+	if (!ast.root)
 	{
 		throw std::logic_error("Unable to parse the service schema, but there was no error message from the parser!");
 	}
 
-	for (const auto& child : ast->root->children)
+	for (const auto& child : ast.root->children)
 	{
 		visitDefinition(*child);
 	}

include/graphqlservice/GraphQLService.h

@@ -498,7 +498,7 @@ using TypeMap = std::unordered_map<std::string, std::shared_ptr<Object>>;
 struct SubscriptionParams
 {
 	std::shared_ptr<RequestState> state;
-	std::unique_ptr<peg::ast<std::string>> query;
+	peg::ast<std::string> query;
 	std::string operationName;
 	response::Value variables;
 };
@@ -534,12 +534,12 @@ using SubscriptionName = std::string;
 struct SubscriptionData : std::enable_shared_from_this<SubscriptionData>
 {
 	explicit SubscriptionData(std::shared_ptr<OperationData>&& data, std::unordered_map<SubscriptionName, std::vector<response::Value>>&& fieldNamesAndArgs,
-		std::unique_ptr<peg::ast<std::string>>&& query, std::string&& operationName, SubscriptionCallback&& callback,
+		peg::ast<std::string>&& query, std::string&& operationName, SubscriptionCallback&& callback,
 		const peg::ast_node& selection);
 
 	std::shared_ptr<OperationData> data;
 	std::unordered_map<SubscriptionName, std::vector<response::Value>> fieldNamesAndArgs;
-	std::unique_ptr<peg::ast<std::string>> query;
+	peg::ast<std::string> query;
 	std::string operationName;
 	SubscriptionCallback callback;
 	const peg::ast_node& selection;

include/graphqlservice/GraphQLTree.h

@@ -26,16 +26,22 @@ struct ast_node
 template <typename _Input>
 struct ast
 {
+	ast() = default;
+	ast(ast&& other) = default;
+	~ast();
+
+	ast& operator=(ast&& other) = default;
+
 	_Input input;
 	std::unique_ptr<ast_node> root;
 };
 
-std::unique_ptr<ast<std::string>> parseString(std::string&& input);
-std::unique_ptr<ast<std::unique_ptr<file_input<>>>> parseFile(const char* filename);
+ast<std::string> parseString(std::string&& input);
+ast<std::unique_ptr<file_input<>>> parseFile(const char* filename);
 
 } /* namespace peg */
 
-std::unique_ptr<peg::ast<const char*>> operator "" _graphql(const char* text, size_t size);
+peg::ast<const char*> operator "" _graphql(const char* text, size_t size);
 
 } /* namespace graphql */
 } /* namespace facebook */

test_today.cpp

@@ -59,13 +59,13 @@ int main(int argc, char** argv)
 	try
 	{
 		const peg::ast_node* ast = nullptr;
-		std::unique_ptr<peg::ast<std::string>> ast_input;
-		std::unique_ptr<peg::ast<std::unique_ptr<peg::file_input<>>>> ast_file;
+		peg::ast<std::string> ast_input;
+		peg::ast<std::unique_ptr<peg::file_input<>>> ast_file;
 
 		if (argc > 1)
 		{
 			ast_file = peg::parseFile(argv[1]);
-			ast = ast_file->root.get();
+			ast = ast_file.root.get();
 		}
 		else
 		{
@@ -78,7 +78,7 @@ int main(int argc, char** argv)
 			}
 
 			ast_input = peg::parseString(std::move(input));
-			ast = ast_input->root.get();
+			ast = ast_input.root.get();
 		}
 
 		if (!ast)

tests.cpp

@@ -126,7 +126,7 @@ TEST_F(TodayServiceCase, QueryEverything)
 		})"_graphql;
 	response::Value variables(response::Type::Map);
 	auto state = std::make_shared<today::RequestState>(1);
-	auto result = _service->resolve(state, *ast->root, "Everything", std::move(variables)).get();
+	auto result = _service->resolve(state, *ast.root, "Everything", std::move(variables)).get();
 	EXPECT_EQ(size_t(1), _getAppointmentsCount) << "today service lazy loads the appointments and caches the result";
 	EXPECT_EQ(size_t(1), _getTasksCount) << "today service lazy loads the tasks and caches the result";
 	EXPECT_EQ(size_t(1), _getUnreadCountsCount) << "today service lazy loads the unreadCounts and caches the result";
@@ -198,7 +198,7 @@ TEST_F(TodayServiceCase, QueryAppointments)
 		})"_graphql;
 	response::Value variables(response::Type::Map);
 	auto state = std::make_shared<today::RequestState>(2);
-	auto result = _service->resolve(state, *ast->root, "", std::move(variables)).get();
+	auto result = _service->resolve(state, *ast.root, "", std::move(variables)).get();
 	EXPECT_EQ(size_t(1), _getAppointmentsCount) << "today service lazy loads the appointments and caches the result";
 	EXPECT_GE(size_t(1), _getTasksCount) << "today service lazy loads the tasks and caches the result";
 	EXPECT_GE(size_t(1), _getUnreadCountsCount) << "today service lazy loads the unreadCounts and caches the result";
@@ -250,7 +250,7 @@ TEST_F(TodayServiceCase, QueryTasks)
 		})gql"_graphql;
 	response::Value variables(response::Type::Map);
 	auto state = std::make_shared<today::RequestState>(3);
-	auto result = _service->resolve(state, *ast->root, "", std::move(variables)).get();
+	auto result = _service->resolve(state, *ast.root, "", std::move(variables)).get();
 	EXPECT_GE(size_t(1), _getAppointmentsCount) << "today service lazy loads the appointments and caches the result";
 	EXPECT_EQ(size_t(1), _getTasksCount) << "today service lazy loads the tasks and caches the result";
 	EXPECT_GE(size_t(1), _getUnreadCountsCount) << "today service lazy loads the unreadCounts and caches the result";
@@ -301,7 +301,7 @@ TEST_F(TodayServiceCase, QueryUnreadCounts)
 		})"_graphql;
 	response::Value variables(response::Type::Map);
 	auto state = std::make_shared<today::RequestState>(4);
-	auto result = _service->resolve(state, *ast->root, "", std::move(variables)).get();
+	auto result = _service->resolve(state, *ast.root, "", std::move(variables)).get();
 	EXPECT_GE(size_t(1), _getAppointmentsCount) << "today service lazy loads the appointments and caches the result";
 	EXPECT_GE(size_t(1), _getTasksCount) << "today service lazy loads the tasks and caches the result";
 	EXPECT_EQ(size_t(1), _getUnreadCountsCount) << "today service lazy loads the unreadCounts and caches the result";
@@ -351,7 +351,7 @@ TEST_F(TodayServiceCase, MutateCompleteTask)
 		})"_graphql;
 	response::Value variables(response::Type::Map);
 	auto state = std::make_shared<today::RequestState>(5);
-	auto result = _service->resolve(state, *ast->root, "", std::move(variables)).get();
+	auto result = _service->resolve(state, *ast.root, "", std::move(variables)).get();
 
 	try
 	{
@@ -509,7 +509,7 @@ TEST_F(TodayServiceCase, Introspection)
 		})"_graphql;
 	response::Value variables(response::Type::Map);
 	auto state = std::make_shared<today::RequestState>(8);
-	auto result = _service->resolve(state, *ast->root, "", std::move(variables)).get();
+	auto result = _service->resolve(state, *ast.root, "", std::move(variables)).get();
 
 	try
 	{
@@ -571,7 +571,7 @@ TEST_F(TodayServiceCase, SkipDirective)
 		})"_graphql;
 	response::Value variables(response::Type::Map);
 	auto state = std::make_shared<today::RequestState>(9);
-	auto result = _service->resolve(state, *ast->root, "", std::move(variables)).get();
+	auto result = _service->resolve(state, *ast.root, "", std::move(variables)).get();
 
 	try
 	{
@@ -633,7 +633,7 @@ TEST_F(TodayServiceCase, IncludeDirective)
 		})"_graphql;
 	response::Value variables(response::Type::Map);
 	auto state = std::make_shared<today::RequestState>(10);
-	auto result = _service->resolve(state, *ast->root, "", std::move(variables)).get();
+	auto result = _service->resolve(state, *ast.root, "", std::move(variables)).get();
 
 	try
 	{
@@ -690,7 +690,7 @@ TEST_F(TodayServiceCase, NestedFragmentDirectives)
 		})"_graphql;
 	response::Value variables(response::Type::Map);
 	auto state = std::make_shared<today::RequestState>(11);
-	auto result = _service->resolve(state, *ast->root, "", std::move(variables)).get();
+	auto result = _service->resolve(state, *ast.root, "", std::move(variables)).get();
 
 	try
 	{
@@ -798,7 +798,7 @@ TEST_F(TodayServiceCase, QueryAppointmentsById)
 	response::Value variables(response::Type::Map);
 	variables.emplace_back("appointmentId", response::Value(std::string("ZmFrZUFwcG9pbnRtZW50SWQ=")));
 	auto state = std::make_shared<today::RequestState>(12);
-	auto result = _service->resolve(state, *ast->root, "", std::move(variables)).get();
+	auto result = _service->resolve(state, *ast.root, "", std::move(variables)).get();
 	EXPECT_EQ(size_t(1), _getAppointmentsCount) << "today service lazy loads the appointments and caches the result";
 	EXPECT_GE(size_t(1), _getTasksCount) << "today service lazy loads the tasks and caches the result";
 	EXPECT_GE(size_t(1), _getUnreadCountsCount) << "today service lazy loads the unreadCounts and caches the result";