Implemented storing locations in the ParserStack, improved parsing typesystems

8 files changed, 281 insertions, 119 deletions

diff --git a/src/core/model/Typesystem.cpp b/src/core/model/Typesystem.cpp
index 726de3e..a3d354c 100644
--- a/src/core/model/Typesystem.cpp
+++ b/src/core/model/Typesystem.cpp
@@ -151,12 +151,7 @@ EnumType::Ordinal EnumType::valueOf(const std::string &name) const
 
 bool Attribute::doValidate(Logger &logger) const
 {
-	if (!Utils::isIdentifier(getName())) {
-		logger.error("Attribute name \"" + getName() +
-		             "\" is not a valid identifier.");
-		return false;
-	}
-	return true;
+	return validateName(logger);
 }
 
 /* Class StructType */
@@ -332,22 +327,8 @@ bool StructType::doBuild(Variant &data, Logger &logger) const
 
 bool StructType::doValidate(Logger &logger) const
 {
-	// Check whether all attributes are valid and unique
-	std::unordered_set<std::string> names;
-	bool res = true;
-	for (Handle<Attribute> a : attributes) {
-		res = a->validate(logger) && res;
-		const std::string &name = a->getName();
-		if (!names.emplace(name).second) {
-			logger.error(
-			    std::string("Attribute with name \"") + name +
-			    std::string("\" defined multiple times in structure \"") +
-			    Utils::join(path(), ".") + std::string("\""));
-			res = false;
-		}
-	}
-
-	return res;
+	return validateName(logger) &
+	       continueValidationCheckDuplicates(attributes, logger);
 }
 
 Rooted<StructType> StructType::createValidated(
@@ -473,6 +454,28 @@ bool ArrayType::doBuild(Variant &data, Logger &logger) const
 	return res;
 }
 
+/* Class Typesystem */
+
+void Typesystem::doResolve(ResolutionState &state)
+{
+	continueResolveComposita(constants, constants.getIndex(), state);
+	continueResolveComposita(types, constants.getIndex(), state);
+}
+
+bool Typesystem::doValidate(Logger &logger) const
+{
+	return validateName(logger) &
+	       continueValidationCheckDuplicates(constants, logger) &
+	       continueValidationCheckDuplicates(types, logger);
+}
+
+Rooted<StructType> Typesystem::createStructType(const std::string &name)
+{
+	Rooted<StructType> structType{new StructType(getManager(), name, this)};
+	addType(structType);
+	return structType;
+}
+
 /* Class SystemTypesystem */
 
 SystemTypesystem::SystemTypesystem(Manager &mgr)
diff --git a/src/core/model/Typesystem.hpp b/src/core/model/Typesystem.hpp
index 7bc8950..64922f0 100644
--- a/src/core/model/Typesystem.hpp
+++ b/src/core/model/Typesystem.hpp
@@ -368,10 +368,7 @@ public:
  */
 class Attribute : public Node {
 private:
-	/**
-	 * Reference to the actual type of the attribute.
-	 */
-	const Owned<Type> type;
+
 
 protected:
 	/**
@@ -384,14 +381,25 @@ protected:
 
 public:
 	/**
+	 * Reference to the actual type of the attribute.
+	 */
+	Owned<Type> type;
+
+	/**
+	 * Initial default value passed to the constructor of the Attribute class
+	 * that has not been passed through the "build" method of the type.
+	 */
+	Variant rawDefaultValue;
+
+	/**
 	 * Default value of the attribute.
 	 */
-	const Variant defaultValue;
+	Variant defaultValue;
 
 	/**
 	 * Flag indicating whether this attribute is actually optional or not.
 	 */
-	const bool optional;
+	bool optional;
 
 	/**
 	 * Constructor of the Attribute class.
@@ -430,6 +438,50 @@ public:
 	}
 
 	/**
+	 * Sets a new default value. This makes the Attribute optional. The given
+	 * default value is passed through the "build" function of the current
+	 * type.
+	 *
+	 * @param defaultValue is the new default value.
+	 * @param logger is the logger instance to which errors that occur during
+	 * reinterpretion of the default value.
+	 */
+	void setDefaultValue(const Variant &defaultValue, Logger &logger);
+
+	/**
+	 * Returns the default value of the attribute.
+	 *
+	 * @return the default value of the attribute. If no default value has been
+	 * given a null variant is returned (the opposite does not hold).
+	 */
+	Variant getDefaultValue() const;
+
+	/**
+	 * Removes any default value from the attribute, making this attribute
+	 * non-optional.
+	 */
+	void removeDefaultValue();
+
+	/**
+	 * Returns true if the attribute is optional (a default value has been
+	 * supplied by the user).
+	 *
+	 * @return true if the attribute is optional, false otherwise.
+	 */
+	bool isOptional() const {return optional; }
+
+	/**
+	 * Sets the type of the attribute to the specified value. This will
+	 * reinterpret the default value that has been passed to the attribute (if
+	 * available).
+	 *
+	 * @param type is the new type that should be used for the attribute.
+	 * @param logger is the logger instance to which errors that occur during
+	 * reinterpretion of the default value.
+	 */
+	void setType(Handle<Type> type, Logger &logger);
+
+	/**
 	 * Returns a reference to the type descriptor holding the type of the
 	 * attribute.
 	 *
@@ -859,6 +911,12 @@ private:
 	 */
 	NodeVector<Constant> constants;
 
+protected:
+
+	void doResolve(ResolutionState &state) override;
+
+	bool doValidate(Logger &logger) const override;
+
 public:
 	/**
 	 * Constructor of the Typesystem class.
@@ -867,11 +925,20 @@ public:
 	 * @param name is the name of the typesystem.
 	 */
 	Typesystem(Manager &mgr, std::string name)
-	    : Node(mgr, name), types(this), constants(this)
+	    : Node(mgr, std::move(name)), types(this), constants(this)
 	{
 	}
 
 	/**
+	 * Creates a new StructType instance with the given name. Adds the new
+	 * StructType as member to the typesystem.
+	 *
+	 * @param name is the name of the structure that should be created.
+	 * @return the new StructType instance.
+	 */
+	Rooted<StructType> createStructType(const std::string &name);
+
+	/**
 	 * Adds the given type to the to the type list.
 	 *
 	 * @param type is the Type that should be stored in this Typesystem
diff --git a/src/core/parser/ParserStack.cpp b/src/core/parser/ParserStack.cpp
index caf2116..9cf782f 100644
--- a/src/core/parser/ParserStack.cpp
+++ b/src/core/parser/ParserStack.cpp
@@ -63,11 +63,11 @@ void Handler::child(std::shared_ptr<Handler> handler)
 
 HandlerInstance HandlerDescriptor::create(const ParserContext &ctx,
                                           std::string name, State parentState,
-                                          bool isChild,
-                                          Variant::mapType &args) const
+                                          bool isChild, Variant::mapType &args,
+                                          const SourceLocation &location) const
 {
 	Handler *h;
-	HandlerData data{ctx, name, targetState, parentState, isChild};
+	HandlerData data{ctx, name, targetState, parentState, isChild, location};
 	if (ctor) {
 		h = ctor(data);
 	} else {
@@ -115,7 +115,8 @@ std::set<std::string> ParserStack::expectedCommands(State state)
 	return res;
 }
 
-void ParserStack::start(std::string name, Variant::mapType &args)
+void ParserStack::start(std::string name, Variant::mapType &args,
+                        const SourceLocation &location)
 {
 	// Fetch the current handler and the current state
 	const HandlerInstance *h = stack.empty() ? nullptr : &stack.top();
@@ -143,11 +144,11 @@ void ParserStack::start(std::string name, Variant::mapType &args)
 	}
 
 	// Instantiate the handler and call its start function
-	stack.emplace(
-	    descr->create(ctx, name, curState, isChild, args));
+	stack.emplace(descr->create(ctx, name, curState, isChild, args, location));
 }
 
-void ParserStack::start(std::string name, const Variant::mapType &args)
+void ParserStack::start(std::string name, const Variant::mapType &args,
+                        const SourceLocation &location)
 {
 	Variant::mapType argsCopy(args);
 	start(name, argsCopy);
diff --git a/src/core/parser/ParserStack.hpp b/src/core/parser/ParserStack.hpp
index 43d6529..aa196e7 100644
--- a/src/core/parser/ParserStack.hpp
+++ b/src/core/parser/ParserStack.hpp
@@ -86,6 +86,11 @@ struct HandlerData {
 	const bool isChild;
 
 	/**
+	 * Current source code location.
+	 */
+	const SourceLocation location;
+
+	/**
 	 * Constructor of the HandlerData class.
 	 *
 	 * @param ctx is the parser context the handler should be executed in.
@@ -94,14 +99,16 @@ struct HandlerData {
 	 * @param parentState is the state of the parent command.
 	 * @param isChild specifies whether this handler was called not for the
 	 * command that was specified in the state machine but a child command.
+	 * @param location is the location at which the handler is created.
 	 */
 	HandlerData(const ParserContext &ctx, std::string name, State state,
-	            State parentState, bool isChild)
+	            State parentState, bool isChild, const SourceLocation location)
 	    : ctx(ctx),
 	      name(std::move(name)),
 	      state(state),
 	      parentState(parentState),
-	      isChild(isChild){};
+	      isChild(isChild),
+	      location(location){};
 };
 
 /**
@@ -123,28 +130,29 @@ public:
 	 * @param data is a structure containing all data being passed to the
 	 * handler.
 	 */
-	Handler(const HandlerData &handlerData) : handlerData(handlerData) {};
+	Handler(const HandlerData &handlerData) : handlerData(handlerData){};
 
 	/**
 	 * Virtual destructor.
 	 */
 	virtual ~Handler(){};
 
-	
-	const std::string& name() {return handlerData.name;}
+	const std::string &name() { return handlerData.name; }
 
-	Scope &scope() {return handlerData.ctx.scope;}
+	Scope &scope() { return handlerData.ctx.scope; }
 
-	Registry &registry() {return handlerData.ctx.registry;}
+	Registry &registry() { return handlerData.ctx.registry; }
 
 	Manager &manager() { return handlerData.ctx.manager; }
 
 	Logger &logger() { return handlerData.ctx.logger; }
 
-	State state() {return handlerData.state; }
+	State state() { return handlerData.state; }
 
 	State parentState() { return handlerData.parentState; }
 
+	SourceLocation location() { return handlerData.location; }
+
 	bool isChild() { return handlerData.isChild; }
 
 	/**
@@ -279,7 +287,8 @@ struct HandlerDescriptor {
 	 */
 	HandlerInstance create(const ParserContext &ctx, std::string name,
 	                       State parentState, bool isChild,
-	                       Variant::mapType &args) const;
+	                       Variant::mapType &args,
+	                       const SourceLocation &location) const;
 };
 
 /**
@@ -294,9 +303,9 @@ private:
 	ParserContext &ctx;
 
 	/**
-	 * User specified data that will be passed to all handlers.
+	 * Current location in the source code.
 	 */
-	void *userData;
+	SourceLocation location;
 
 	/**
 	 * Map containing all registered command names and the corresponding
@@ -369,16 +378,22 @@ public:
 	 *
 	 * @param name is the name of the command.
 	 * @param args is a map from strings to variants (argument name and value).
+	 * @param location is the location in the source file at which the command
+	 * starts.
 	 */
-	void start(std::string name, Variant::mapType &args);
+	void start(std::string name, Variant::mapType &args,
+	           const SourceLocation &location = SourceLocation{});
 
 	/**
 	 * Function that should be called whenever a new command starts.
 	 *
 	 * @param name is the name of the command.
 	 * @param args is a map from strings to variants (argument name and value).
+	 * @param location is the location in the source file at which the command
+	 * starts.
 	 */
-	void start(std::string name, const Variant::mapType &args);
+	void start(std::string name, const Variant::mapType &args,
+	           const SourceLocation &location = SourceLocation{});
 
 	/**
 	 * Function called whenever a command ends.
diff --git a/src/core/parser/Scope.cpp b/src/core/parser/Scope.cpp
index d76af9c..6e7dceb 100644
--- a/src/core/parser/Scope.cpp
+++ b/src/core/parser/Scope.cpp
@@ -59,11 +59,11 @@ Rooted<Node> ScopeBase::resolve(const std::vector<std::string> &path,
 
 		// Log an error if the object is not unique
 		if (res.size() > 1) {
-			logger.error(std::string("The reference ") +
-			             Utils::join(path, ".") + (" is ambigous!"));
+			logger.error(std::string("The reference \"") +
+			             Utils::join(path, ".") + ("\" is ambigous!"));
 			logger.note("Referenced objects are:");
 			for (const ResolutionResult &r : res) {
-				logger.note(std::string("\t") + Utils::join(r.path(), "."));
+				logger.note(Utils::join(r.path(), "."));
 			}
 		}
 		return res[0].node;
@@ -73,10 +73,16 @@ Rooted<Node> ScopeBase::resolve(const std::vector<std::string> &path,
 
 /* Class DeferredResolution */
 
-DeferredResolution::DeferredResolution(
-    const NodeVector<Node> &nodes, const std::vector<std::string> &path,
-    const RttiType &type, std::function<void(Handle<Node>)> resultCallback)
-    : scope(nodes), resultCallback(resultCallback), path(path), type(type)
+DeferredResolution::DeferredResolution(const NodeVector<Node> &nodes,
+                                       const std::vector<std::string> &path,
+                                       const RttiType &type,
+                                       ResolutionResultCallback resultCallback,
+                                       const SourceLocation &location)
+    : scope(nodes),
+      resultCallback(resultCallback),
+      path(path),
+      type(type),
+      location(location)
 {
 }
 
@@ -84,7 +90,12 @@ bool DeferredResolution::resolve(Logger &logger)
 {
 	Rooted<Node> res = scope.resolve(path, type, logger);
 	if (res != nullptr) {
-		resultCallback(res);
+		try {
+			resultCallback(res, logger);
+		}
+		catch (LoggableException ex) {
+			logger.log(ex);
+		}
 		return true;
 	}
 	return false;
@@ -106,30 +117,32 @@ Rooted<Node> Scope::getRoot() const { return nodes.front(); }
 Rooted<Node> Scope::getLeaf() { return nodes.back(); }
 
 bool Scope::resolve(const std::vector<std::string> &path, const RttiType &type,
-                    Logger &logger,
-                    std::function<Rooted<Node>()> imposterCallback,
-                    std::function<void(Handle<Node>)> resultCallback)
+                    Logger &logger, ResolutionImposterCallback imposterCallback,
+                    ResolutionResultCallback resultCallback,
+	             const SourceLocation &location)
 {
-	Rooted<Node> res = ScopeBase::resolve(path, type, logger);
-	if (res != nullptr) {
-		resultCallback(res);
-		return true;
+	if (!resolve(path, type, logger, resultCallback, location)) {
+		resultCallback(imposterCallback(), logger);
+		return false;
 	}
-	resultCallback(imposterCallback());
-	deferred.emplace_back(nodes, path, type, resultCallback);
-	return false;
+	return true;
 }
 
 bool Scope::resolve(const std::vector<std::string> &path, const RttiType &type,
-                    Logger &logger,
-                    std::function<void(Handle<Node>)> successCallback)
+                    Logger &logger, ResolutionResultCallback resultCallback,
+                    const SourceLocation &location)
 {
 	Rooted<Node> res = ScopeBase::resolve(path, type, logger);
 	if (res != nullptr) {
-		successCallback(res);
+		try {
+			resultCallback(res, logger);
+		}
+		catch (LoggableException ex) {
+			logger.log(ex, location);
+		}
 		return true;
 	}
-	deferred.emplace_back(nodes, path, type, successCallback);
+	deferred.emplace_back(nodes, path, type, resultCallback, location);
 	return false;
 }
 
@@ -157,14 +170,13 @@ bool Scope::performDeferredResolution(Logger &logger)
 
 	// Output an error message if there are still deferred elements left that
 	// could not be resolved
-	// TODO: Log this at the position at which the resolution was originally
-	// triggered
 	if (!deferred.empty()) {
 		for (const auto &failed : deferred) {
 			logger.error(
-			    std::string("Could not resolve \"") +
-			    Utils::join(failed.path, ".") +
-			    std::string("\" of internal type " + failed.type.name));
+			    std::string("Could not resolve a reference to \"") +
+			        Utils::join(failed.path, ".") +
+			        std::string("\" of type " + failed.type.name),
+			    failed.location);
 		}
 	}
 
diff --git a/src/core/parser/Scope.hpp b/src/core/parser/Scope.hpp
index 2713c41..c99aa65 100644
--- a/src/core/parser/Scope.hpp
+++ b/src/core/parser/Scope.hpp
@@ -43,6 +43,18 @@ namespace parser {
 class Scope;
 
 /**
+ * Callback function type used for creating a dummy object while no correct
+ * object is available for resolution.
+ */
+using ResolutionImposterCallback = std::function<Rooted<Node>()>;
+
+/**
+ * Callback function type called whenever the result of a resolution is
+ * available.
+ */
+using ResolutionResultCallback = std::function<void(Handle<Node>, Logger &logger)>;
+
+/**
  * The GuardedScope class takes care of pushing a Node instance into the
  * name resolution stack of a Scope instance and poping this node once the
  * ScopedScope instance is deletes. This way you cannot forget to pop a Node
@@ -148,7 +160,7 @@ private:
 	/**
 	 * Callback function to be called when an element is successfully resolved.
 	 */
-	std::function<void(Handle<Node>)> resultCallback;
+	ResolutionResultCallback resultCallback;
 
 public:
 	/**
@@ -162,6 +174,11 @@ public:
 	const RttiType &type;
 
 	/**
+	 * Position at which the resolution was triggered.
+	 */
+	const SourceLocation location;
+
+	/**
 	 * Constructor of the DeferredResolutionScope class. Copies the given
 	 * arguments.
 	 *
@@ -172,11 +189,13 @@ public:
 	 * @param type is the RttiType 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.
 	 */
 	DeferredResolution(const NodeVector<Node> &nodes,
 	                   const std::vector<std::string> &path,
 	                   const RttiType &type,
-	                   std::function<void(Handle<Node>)> resultCallback);
+	                   ResolutionResultCallback resultCallback,
+	                   const SourceLocation &location = SourceLocation{});
 
 	/**
 	 * Performs the actual deferred resolution and calls the resultCallback
@@ -267,34 +286,39 @@ 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.
 	 * @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 RttiType &type,
-	             Logger &logger, std::function<Rooted<Node>()> imposterCallback,
-	             std::function<void(Handle<Node>)> resultCallback);
+	             Logger &logger, ResolutionImposterCallback imposterCallback,
+	             ResolutionResultCallback resultCallback,
+	             const SourceLocation &location = SourceLocation{});
 
 	/**
 	 * Tries to resolve a node for the given type and path for all nodes
 	 * currently on the stack, starting with the topmost node on the stack.
-	 * The "successCallback" is called when the resolution was successful, which
+	 * The "resultCallback" is called when the resolution was successful, which
 	 * may be at a later point in time.
 	 *
 	 * @param path is the path for which a node should be resolved.
 	 * @param type is the type of the node that should be resolved.
 	 * @param logger is the logger instance into which resolution problems
 	 * should be logged.
-	 * @param successCallback is the callback function to which the result of
+	 * @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.
 	 * @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 RttiType &type,
-	             Logger &logger,
-	             std::function<void(Handle<Node>)> successCallback);
+	             Logger &logger, ResolutionResultCallback resultCallback,
+	             const SourceLocation &location = SourceLocation{});
 
 	/**
 	 * Tries to resolve a node for the given type and path for all nodes
@@ -319,6 +343,8 @@ 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.
 	 * @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.
@@ -326,41 +352,45 @@ public:
 	template <class T>
 	bool resolve(const std::vector<std::string> &path, Logger &logger,
 	             std::function<Rooted<T>()> imposterCallback,
-	             std::function<void(Handle<T>)> successCallback)
+	             std::function<void(Handle<T>, Logger&)> resultCallback,
+	             const SourceLocation &location = SourceLocation{})
 	{
 		return resolve(
 		    path, typeOf<T>(), logger,
 		    [imposterCallback]() -> Rooted<Node> { return imposterCallback(); },
-		    [successCallback](Handle<Node> node) {
-			    successCallback(node.cast<T>());
-			});
+		    [resultCallback](Handle<Node> node, Logger &logger) {
+			    resultCallback(node.cast<T>(), logger);
+			}, location);
 	}
 
 	/**
 	 * Tries to resolve a node for the given type and path for all nodes
 	 * currently on the stack, starting with the topmost node on the stack.
-	 * The "successCallback" is called when the resolution was successful, which
+	 * The "resultCallback" is called when the resolution was successful, which
 	 * may be at a later point in time.
 	 *
 	 * @tparam is the type of the node that should be resolved.
 	 * @param path is the path for which a node should be resolved.
 	 * @param logger is the logger instance into which resolution problems
 	 * should be logged.
-	 * @param successCallback is the callback function to which the result of
+	 * @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.
 	 * @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.
 	 */
 	template <class T>
 	bool resolve(const std::vector<std::string> &path, Logger &logger,
-	             std::function<void(Handle<T>)> resultCallback)
+	             std::function<void(Handle<T>, Logger&)> resultCallback,
+	             const SourceLocation &location = SourceLocation{})
 	{
 		return resolve(path, typeOf<T>(), logger,
-		               [resultCallback](Handle<Node> node) {
-			resultCallback(node.cast<T>());
-		});
+		               [resultCallback](Handle<Node> node, Logger &logger) {
+			resultCallback(node.cast<T>(), logger);
+		}, location);
 	}
 
 	/**
diff --git a/src/plugins/xml/XmlParser.cpp b/src/plugins/xml/XmlParser.cpp
index ced61ee..cd220a9 100644
--- a/src/plugins/xml/XmlParser.cpp
+++ b/src/plugins/xml/XmlParser.cpp
@@ -23,6 +23,7 @@
 
 #include <core/common/CharReader.hpp>
 #include <core/common/Utils.hpp>
+#include <core/common/VariantReader.hpp>
 #include <core/parser/ParserStack.hpp>
 #include <core/model/Typesystem.hpp>
 
@@ -32,6 +33,8 @@ namespace ousia {
 namespace parser {
 namespace xml {
 
+using namespace ousia::model;
+
 /* Document structure */
 static const State STATE_DOCUMENT = 0;
 static const State STATE_HEAD = 1;
@@ -63,6 +66,8 @@ public:
 	void end() override
 	{
 		scope().performDeferredResolution(logger());
+		// TODO: Automatically call validate in "pop"?
+		scope().getLeaf()->validate(logger());
 		scope().pop();
 	}
 
@@ -76,22 +81,36 @@ class StructHandler : public Handler {
 public:
 	using Handler::Handler;
 
-	std::string name;
-	std::string parent;
-
-	NodeVector<model::Attribute> attributes;
-
 	void start(Variant::mapType &args) override
 	{
-		this->name = args["name"].asString();
-		this->parent = args["parent"].asString();
-	}
+		// Fetch the arguments used for creating this type
+		const std::string &name = args["name"].asString();
+		const std::string &parent = args["parent"].asString();
+
+		// Fetch the current typesystem and create the struct node
+		Rooted<Typesystem> typesystem = scope().getLeaf().cast<Typesystem>();
+		Rooted<StructType> structType = typesystem->createStructType(name);
+
+		// Try to resolve the parent type and set it as parent structure
+		if (!parent.empty()) {
+			scope().resolve<StructType>(Utils::split(parent, '.'), logger(),
+			                            [structType](Handle<StructType> parent,
+			                                         Logger &logger) mutable {
+				                            structType->setParentStructure(
+				                                parent, logger);
+				                        },
+			                            location());
+		}
 
-	void end() override {
-		
+		// Descend into the struct type
+		scope().push(structType);
 	}
 
-	void child(std::shared_ptr<Handler> handler) {}
+	void end() override
+	{
+		// Descend from the struct type
+		scope().pop();
+	}
 
 	static Handler *create(const HandlerData &handlerData)
 	{
@@ -103,15 +122,24 @@ class StructFieldHandler : public Handler {
 public:
 	using Handler::Handler;
 
-	Rooted<model::Attribute> attribute;
-
 	void start(Variant::mapType &args) override
 	{
-		/*		this->name = args["name"].asString();
-		        this->type = args["parent"].asString();*/
+		// Read the argument values
+		/*		const std::string &name = args["name"].asString();
+		        const std::string &type = args["parent"].asString();
+		        const Variant &defaultValue = args["default"];
+		        const bool optional = !(defaultValue.isObject() &&
+		   defaultValue.asObject() == nullptr);*/
+
+		// Try to resolve the
 	}
 
 	void end() override {}
+
+	static Handler *create(const HandlerData &handlerData)
+	{
+		return new StructFieldHandler{handlerData};
+	}
 };
 
 static const std::multimap<std::string, HandlerDescriptor> XML_HANDLERS{
@@ -142,7 +170,7 @@ static const std::multimap<std::string, HandlerDescriptor> XML_HANDLERS{
       {Argument::String("name"), Argument::String("parent", "")}}},
     {"field",
      {{{STATE_STRUCT}},
-      nullptr,
+      StructFieldHandler::create,
       STATE_FIELD,
       false,
       {Argument::String("name"), Argument::String("type"),
@@ -198,24 +226,25 @@ public:
 
 /* Adapter Expat -> ParserStack */
 
-static void syncLoggerPosition(XML_Parser p)
+static SourceLocation syncLoggerPosition(XML_Parser p)
 {
 	// Fetch the current location in the XML file
 	int line = XML_GetCurrentLineNumber(p);
 	int column = XML_GetCurrentColumnNumber(p);
 	size_t offs = XML_GetCurrentByteIndex(p);
+	SourceLocation loc{line, column, offs};
 
 	// Update the default location of the current logger instance
 	ParserStack *stack = static_cast<ParserStack *>(XML_GetUserData(p));
-	stack->getContext().logger.setDefaultLocation(
-	    SourceLocation{line, column, offs});
+	stack->getContext().logger.setDefaultLocation(loc);
+	return loc;
 }
 
 static void xmlStartElementHandler(void *p, const XML_Char *name,
                                    const XML_Char **attrs)
 {
 	XML_Parser parser = static_cast<XML_Parser>(p);
-	syncLoggerPosition(parser);
+	SourceLocation loc = syncLoggerPosition(parser);
 
 	ParserStack *stack = static_cast<ParserStack *>(XML_GetUserData(parser));
 
@@ -223,9 +252,11 @@ static void xmlStartElementHandler(void *p, const XML_Char *name,
 	const XML_Char **attr = attrs;
 	while (*attr) {
 		const std::string key{*(attr++)};
-		args.emplace(std::make_pair(key, Variant{*(attr++)}));
+		std::pair<bool, Variant> value = VariantReader::parseGenericString(
+		    *(attr++), stack->getContext().logger);
+		args.emplace(std::make_pair(key, value.second));
 	}
-	stack->start(std::string(name), args);
+	stack->start(std::string(name), args, loc);
 }
 
 static void xmlEndElementHandler(void *p, const XML_Char *name)
diff --git a/test/plugins/xml/XmlParserTest.cpp b/test/plugins/xml/XmlParserTest.cpp
index ce53eb3..2046940 100644
--- a/test/plugins/xml/XmlParserTest.cpp
+++ b/test/plugins/xml/XmlParserTest.cpp
@@ -53,14 +53,17 @@ const char *TEST_DATA =
     "	<head>\n"
     "		<typesystem name=\"color\">\n"
     "			<types>\n"
-    "				<struct name=\"blub\">\n"
-    "					<field name=\"a\" type=\"int\"/>\n"
-    "				</struct>\n"
     "				<struct name=\"color\" parent=\"blub\">\n"
     "					<field name=\"r\" type=\"int\"/>\n"
     "					<field name=\"g\" type=\"int\"/>\n"
     "					<field name=\"b\" type=\"int\"/>\n"
     "				</struct>\n"
+    "				<struct name=\"blub\">\n"
+    "					<field name=\"a\" type=\"int\"/>\n"
+    "				</struct>\n"
+    "				<struct name=\"blub\">\n"
+    "					<field name=\"a\" type=\"int\"/>\n"
+    "				</struct>\n"
     "			</types>\n"
     "		</typesystem>\n"
     "	</head>\n"