ParserScope now stores the nodes for which the resolution was triggered and first executes those resolutions that depend on not currently resolving nodes (this behaviour is needed for the resolution of constants).

3 files changed, 90 insertions, 54 deletions

diff --git a/src/core/parser/ParserScope.cpp b/src/core/parser/ParserScope.cpp
index 2bacf10..0de0dbf 100644
--- a/src/core/parser/ParserScope.cpp
+++ b/src/core/parser/ParserScope.cpp
@@ -64,28 +64,37 @@ DeferredResolution::DeferredResolution(const NodeVector<Node> &nodes,
                                        const std::vector<std::string> &path,
                                        const Rtti &type,
                                        ResolutionResultCallback resultCallback,
-                                       const SourceLocation &location)
+                                       Handle<Node> owner)
     : scope(nodes),
       resultCallback(resultCallback),
       path(path),
       type(type),
-      location(location)
+      owner(owner)
 {
 }
 
-bool DeferredResolution::resolve(Logger &logger)
+bool DeferredResolution::resolve(
+    const std::unordered_multiset<const Node *> &ignore, Logger &logger)
 {
-	Rooted<Node> res = scope.resolve(path, type, logger);
+	// Fork the logger to prevent error messages from being shown if we actively
+	// ignore the resolution result
+	LoggerFork loggerFork = logger.fork();
+	Rooted<Node> res = scope.resolve(path, type, loggerFork);
 	if (res != nullptr) {
-		try {
-			// Push the location onto the logger default location stack
-			GuardedLogger localLogger(logger, location);
-			resultCallback(res, localLogger);
-		}
-		catch (LoggableException ex) {
-			logger.log(ex);
+		if (!ignore.count(res.get())) {
+			loggerFork.commit();
+			try {
+				// Push the location onto the logger default location stack
+				GuardedLogger loggerGuard(logger, *owner);
+				resultCallback(res, logger);
+			}
+			catch (LoggableException ex) {
+				logger.log(ex);
+			}
+			return true;
 		}
-		return true;
+	} else {
+		loggerFork.commit();
 	}
 	return false;
 }
@@ -111,7 +120,7 @@ bool ParserScope::checkUnwound(Logger &logger) const
 			logger.note(std::string("Element of interal type ") +
 			                nodes[i]->type().name +
 			                std::string(" defined here:"),
-			            nodes[i]->getLocation());
+			            *nodes[i]);
 		}
 		return false;
 	}
@@ -130,6 +139,8 @@ bool ParserScope::join(const ParserScope &fork, Logger &logger)
 	// Insert the deferred resolutions of the fork into our own deferred
 	// resolution list
 	deferred.insert(deferred.end(), fork.deferred.begin(), fork.deferred.end());
+	awaitingResolution.insert(fork.awaitingResolution.begin(),
+	                          fork.awaitingResolution.end());
 	return true;
 }
 
@@ -220,9 +231,9 @@ bool ParserScope::resolve(const std::vector<std::string> &path,
                           const Rtti &type, Logger &logger,
                           ResolutionImposterCallback imposterCallback,
                           ResolutionResultCallback resultCallback,
-                          const SourceLocation &location)
+                          Handle<Node> owner)
 {
-	if (!resolve(path, type, logger, resultCallback, location)) {
+	if (!resolve(path, type, logger, resultCallback, owner)) {
 		resultCallback(imposterCallback(), logger);
 		return false;
 	}
@@ -232,19 +243,26 @@ bool ParserScope::resolve(const std::vector<std::string> &path,
 bool ParserScope::resolve(const std::vector<std::string> &path,
                           const Rtti &type, Logger &logger,
                           ResolutionResultCallback resultCallback,
-                          const SourceLocation &location)
+                          Handle<Node> owner)
 {
+	// Try to directly resolve the node
 	Rooted<Node> res = ParserScopeBase::resolve(path, type, logger);
-	if (res != nullptr) {
+	if (res != nullptr && !awaitingResolution.count(res.get())) {
 		try {
 			resultCallback(res, logger);
 		}
 		catch (LoggableException ex) {
-			logger.log(ex, location);
+			logger.log(ex, *owner);
 		}
 		return true;
 	}
-	deferred.emplace_back(nodes, path, type, resultCallback, location);
+
+	// Mark the owner as "awaitingResolution", preventing it from being returned
+	// as resolution result
+	if (owner != nullptr) {
+		awaitingResolution.insert(owner.get());
+	}
+	deferred.emplace_back(nodes, path, type, resultCallback, owner);
 	return false;
 }
 
@@ -256,7 +274,10 @@ bool ParserScope::performDeferredResolution(Logger &logger)
 		// Iterate over all deferred resolution processes,
 		bool hasChange = false;
 		for (auto it = deferred.begin(); it != deferred.end();) {
-			if (it->resolve(logger)) {
+			if (it->resolve(awaitingResolution, logger)) {
+				if (it->owner != nullptr) {
+					awaitingResolution.erase(it->owner.get());
+				}
 				it = deferred.erase(it);
 				hasChange = true;
 			} else {
@@ -266,7 +287,13 @@ bool ParserScope::performDeferredResolution(Logger &logger)
 
 		// Abort if nothing has changed in the last iteration
 		if (!hasChange) {
-			break;
+			// In a last step, clear the "awaitingResolution" list to allow
+			// cyclical dependencies to be resolved
+			if (!awaitingResolution.empty()) {
+				awaitingResolution.clear();
+			} else {
+				break;
+			}
 		}
 	}
 
@@ -281,7 +308,7 @@ bool ParserScope::performDeferredResolution(Logger &logger)
 		logger.error(std::string("Could not resolve ") + failed.type.name +
 		                 std::string(" \"") + Utils::join(failed.path, ".") +
 		                 std::string("\""),
-		             failed.location);
+		             *failed.owner);
 	}
 	deferred.clear();
 	return false;
diff --git a/src/core/parser/ParserScope.hpp b/src/core/parser/ParserScope.hpp
index 07fb639..2378967 100644
--- a/src/core/parser/ParserScope.hpp
+++ b/src/core/parser/ParserScope.hpp
@@ -21,6 +21,7 @@
 
 #include <functional>
 #include <list>
+#include <unordered_set>
 #include <vector>
 
 #include <core/common/Logger.hpp>
@@ -131,9 +132,9 @@ public:
 	const Rtti &type;
 
 	/**
-	 * Position at which the resolution was triggered.
+	 * Node for which the resolution is taking place.
 	 */
-	const SourceLocation location;
+	Rooted<Node> owner;
 
 	/**
 	 * Constructor of the DeferredResolutionScope class. Copies the given
@@ -146,23 +147,26 @@ public:
 	 * @param type is the Rtti of the element that should be queried.
 	 * @param resultCallback is the callback function that should be called if
 	 * the desired element has indeed been found.
-	 * @param location is the location at which the resolution was triggered.
+	 * @param owner is the node for which the resolution takes place.
 	 */
 	DeferredResolution(const NodeVector<Node> &nodes,
 	                   const std::vector<std::string> &path, const Rtti &type,
 	                   ResolutionResultCallback resultCallback,
-	                   const SourceLocation &location = SourceLocation{});
+	                   Handle<Node> owner);
 
 	/**
 	 * Performs the actual deferred resolution and calls the resultCallback
 	 * callback function in case the resolution is sucessful. In this case
 	 * returns true, false otherwise.
 	 *
+	 * @param ignore is a set of nodes that should be ignored if returned as
+	 * resolution result as they are
 	 * @param logger is the logger instance to which error messages should be
 	 * logged.
 	 * @return true if the resolution was successful, false otherwise.
 	 */
-	bool resolve(Logger &logger);
+	bool resolve(const std::unordered_multiset<const Node *> &ignore,
+	             Logger &logger);
 };
 
 /**
@@ -230,6 +234,16 @@ private:
 	std::list<DeferredResolution> deferred;
 
 	/**
+	 * Multiset storing the Nodes that are currently awaiting resolution. This
+	 * list has the purpose of forcing nodes to be resolved in the correct order
+	 * -- first nodes need to be returned as resolution result, that do
+	 * themselves not depend on other resolutions. However, if no further
+	 * resolutions are possible, this rule is ignored and all resolutions are
+	 * performed.
+	 */
+	std::unordered_multiset<const Node *> awaitingResolution;
+
+	/**
 	 * Vector containing all set flags. The vector contains triples of the
 	 * depth at which the flag was set, the flag itself and the value.
 	 */
@@ -349,8 +363,9 @@ public:
 	 * @param maxDepth is the maximum number of stack entries the selection
 	 * function may ascend. A negative value indicates no limitation.
 	 */
-	template<class T>
-	Rooted<T> select(int maxDepth = -1) {
+	template <class T>
+	Rooted<T> select(int maxDepth = -1)
+	{
 		return select(RttiSet{&typeOf<T>()}, maxDepth).cast<T>();
 	}
 
@@ -395,8 +410,7 @@ public:
 	 * the resolved object directly when this function is called. If the
 	 * resolution was not successful the first time, it may be called another
 	 * time later in the context of the "performDeferredResolution" function.
-	 * @param location is the location in the current source file in which
-	 * the resolution was triggered.
+	 * @param owner is the node for which the resolution takes place.
 	 * @return true if the resolution was immediately successful. This does
 	 * not mean, that the resolved object does not exist, as it may be resolved
 	 * later.
@@ -404,7 +418,7 @@ public:
 	bool resolve(const std::vector<std::string> &path, const Rtti &type,
 	             Logger &logger, ResolutionImposterCallback imposterCallback,
 	             ResolutionResultCallback resultCallback,
-	             const SourceLocation &location = SourceLocation{});
+	             Handle<Node> owner = nullptr);
 
 	/**
 	 * Tries to resolve a node for the given type and path for all nodes
@@ -419,15 +433,14 @@ public:
 	 * @param resultCallback is the callback function to which the result of
 	 * the resolution process is passed. This function is called once the
 	 * resolution was successful.
-	 * @param location is the location in the current source file in which the
-	 * resolution was triggered.
+	 * @param owner is the node for which the resolution takes place.
 	 * @return true if the resolution was immediately successful. This does not
 	 * mean, that the resolved object does not exist, as it may be resolved
 	 * later.
 	 */
 	bool resolve(const std::vector<std::string> &path, const Rtti &type,
 	             Logger &logger, ResolutionResultCallback resultCallback,
-	             const SourceLocation &location = SourceLocation{});
+	             Handle<Node> owner = nullptr);
 
 	/**
 	 * Tries to resolve a node for the given type and path for all nodes
@@ -452,8 +465,7 @@ public:
 	 * resolved object directly when this function is called. If the resolution
 	 * was not successful the first time, it may be called another time later
 	 * in the context of the "performDeferredResolution" function.
-	 * @param location is the location in the current source file in which the
-	 * resolution was triggered.
+	 * @param owner is the node for which the resolution takes place.
 	 * @return true if the resolution was immediately successful. This does not
 	 * mean, that the resolved object does not exist, as it may be resolved
 	 * later.
@@ -462,7 +474,7 @@ public:
 	bool resolve(const std::vector<std::string> &path, Logger &logger,
 	             std::function<Rooted<T>()> imposterCallback,
 	             std::function<void(Handle<T>, Logger &)> resultCallback,
-	             const SourceLocation &location = SourceLocation{})
+	             Handle<Node> owner = nullptr)
 	{
 		return resolve(
 		    path, typeOf<T>(), logger,
@@ -470,7 +482,7 @@ public:
 		    [resultCallback](Handle<Node> node, Logger &logger) {
 			    resultCallback(node.cast<T>(), logger);
 			},
-		    location);
+		    owner);
 	}
 
 	/**
@@ -486,8 +498,7 @@ public:
 	 * @param resultCallback is the callback function to which the result of
 	 * the resolution process is passed. This function is called once the
 	 * resolution was successful.
-	 * @param location is the location in the current source file in which the
-	 * resolution was triggered.
+	 * @param owner is the node for which the resolution takes place.
 	 * @return true if the resolution was immediately successful. This does not
 	 * mean, that the resolved object does not exist, as it may be resolved
 	 * later.
@@ -495,13 +506,13 @@ public:
 	template <class T>
 	bool resolve(const std::vector<std::string> &path, Logger &logger,
 	             std::function<void(Handle<T>, Logger &)> resultCallback,
-	             const SourceLocation &location = SourceLocation{})
+	             Handle<Node> owner = nullptr)
 	{
 		return resolve(path, typeOf<T>(), logger,
 		               [resultCallback](Handle<Node> node, Logger &logger) {
 			               resultCallback(node.cast<T>(), logger);
 			           },
-		               location);
+		               owner);
 	}
 
 	/**
@@ -528,8 +539,7 @@ public:
 	 * resolved object directly when this function is called. If the resolution
 	 * was not successful the first time, it may be called another time later
 	 * in the context of the "performDeferredResolution" function.
-	 * @param location is the location in the current source file in which the
-	 * resolution was triggered.
+	 * @param owner is the node for which the resolution takes place.
 	 * @return true if the resolution was immediately successful. This does not
 	 * mean, that the resolved object does not exist, as it may be resolved
 	 * later.
@@ -538,10 +548,10 @@ public:
 	bool resolve(const std::string &name, Logger &logger,
 	             std::function<Rooted<T>()> imposterCallback,
 	             std::function<void(Handle<T>, Logger &)> resultCallback,
-	             const SourceLocation &location = SourceLocation{})
+	             Handle<Node> owner = nullptr)
 	{
 		return resolve<T>(Utils::split(name, '.'), logger, imposterCallback,
-		                  resultCallback, location);
+		                  resultCallback, owner);
 	}
 
 	/**
@@ -558,8 +568,7 @@ public:
 	 * @param resultCallback is the callback function to which the result of
 	 * the resolution process is passed. This function is called once the
 	 * resolution was successful.
-	 * @param location is the location in the current source file in which the
-	 * resolution was triggered.
+	 * @param owner is the node for which the resolution takes place.
 	 * @return true if the resolution was immediately successful. This does not
 	 * mean, that the resolved object does not exist, as it may be resolved
 	 * later.
@@ -567,10 +576,10 @@ public:
 	template <class T>
 	bool resolve(const std::string &name, Logger &logger,
 	             std::function<void(Handle<T>, Logger &)> resultCallback,
-	             const SourceLocation &location = SourceLocation{})
+	             Handle<Node> owner = nullptr)
 	{
 		return resolve<T>(Utils::split(name, '.'), logger, resultCallback,
-		                  location);
+		                  owner);
 	}
 
 	/**
diff --git a/src/plugins/xml/XmlParser.cpp b/src/plugins/xml/XmlParser.cpp
index f254326..6e0fea0 100644
--- a/src/plugins/xml/XmlParser.cpp
+++ b/src/plugins/xml/XmlParser.cpp
@@ -173,7 +173,7 @@ public:
 				                            structType->setParentStructure(
 				                                parent, logger);
 				                        },
-			                            location());
+			                            structType);
 		}
 
 		// Descend into the struct type
@@ -216,7 +216,7 @@ public:
 		    [attribute](Handle<Type> type, Logger &logger) mutable {
 			    attribute->setType(type, logger);
 			},
-		    location());
+		    attribute);
 	}
 
 	void end() override {}
@@ -248,7 +248,7 @@ public:
 		    [constant](Handle<Type> type, Logger &logger) mutable {
 			    constant->setType(type, logger);
 			},
-		    location());
+		    constant);
 	}
 
 	void end() override {}