detect schema super cycles while validating a .cfg file

Note that if the .cfg file does not use the tags that causes a cycle, those won't be detected.

This change exposes the super_refs_ so the schema validator does not need to reinvent wml_tag::expand when detecting super cycles.

The super dependency graph is built during the validation of each tag, finishing and reporting cycles at the end of the root tag.

src/serialization/schema/tag.cpp

@@ -388,9 +388,6 @@ void wml_tag::expand(wml_tag& root)
 		if(super_tag) {
 			if(super_tag != this) {
 				super_refs_.emplace(super, super_tag);
-			} else {
-				// TODO: Detect super cycles too!
-				//PLAIN_LOG << "the same" << super_tag->name_;
 			}
 		}
 		// TODO: Warn if the super doesn't exist
@@ -524,6 +521,26 @@ void wml_tag::key_iterator::ensure_valid_or_end() {
 	}
 }
 
+template<>
+void wml_tag::super_iterator::init(const wml_tag& base_tag)
+{
+	current = base_tag.super_refs_.begin();
+	condition_queue.push(&base_tag);
+}
+
+template<>
+void wml_tag::super_iterator::ensure_valid_or_end() {
+	while(current == condition_queue.front()->super_refs_.end()) {
+		condition_queue.pop();
+		if(condition_queue.empty()) {
+			return;
+		}
+		const wml_tag& new_base = *condition_queue.front();
+		current = new_base.super_refs_.begin();
+		push_new_tag_conditions(new_base);
+	}
+}
+
 void wml_tag::push_new_tag_conditions(std::queue<const wml_tag*>& q, const config& match, const wml_tag& tag) {
 	for(const auto& condition : tag.conditions_) {
 		if(condition.matches(match)) {

src/serialization/schema/tag.hpp

@@ -107,6 +107,7 @@ private:
 	template<typename T, typename Map> friend class iterator;
 	using tag_iterator = iterator<wml_tag>;
 	using key_iterator = iterator<wml_key>;
+	using super_iterator = iterator<const wml_tag*>;
 public:
 
 	wml_tag()
@@ -307,6 +308,10 @@ public:
 		return {key_iterator(*this, cfg_match), key_iterator()};
 	}
 
+	boost::iterator_range<super_iterator> super(const config& cfg_match) const {
+		return {super_iterator(*this, cfg_match), super_iterator()};
+	}
+
 	const link_map& links() const
 	{
 		return links_;

src/serialization/schema_validator.cpp

@@ -140,6 +140,19 @@ static std::string wrong_value_error(const std::string& file,
 	return ss.str();
 }
 
+static std::string inheritance_cycle_error(const std::string& file,
+		int line,
+		const std::string& tag,
+		const std::string& schema_name,
+		const std::string& value,
+		bool flag_exception)
+{
+	std::ostringstream ss;
+	ss << "Inheritance cycle from " << tag << " to " << value << " found\n" << at(file, line) << "\nwith schema " << schema_name << "\n";
+	print_output(ss.str(), flag_exception);
+	return ss.str();
+}
+
 static void wrong_path_error(const std::string& file,
 		int line,
 		const std::string& tag,
@@ -334,6 +347,37 @@ void schema_validator::validate(const config& cfg, const std::string& name, int
 	// Checking all elements counters.
 	if(have_active_tag() && is_valid()) {
 		const wml_tag& active = active_tag();
+
+		if (&active == &root_) {
+			detect_derivation_cycles();
+		} else {
+			// Build derivation graph
+			const auto super_tags = active.super(cfg);
+
+			for (const auto& [super_path, super_tag] : super_tags) {
+				if (derivation_map_.find(&active) == derivation_map_.end()) {
+					derivation_map_.emplace(
+						&active,
+						boost::add_vertex({&active, active_tag_path()}, derivation_graph_)
+					);
+				}
+
+				if (derivation_map_.find(super_tag) == derivation_map_.end()) {
+					derivation_map_.emplace(
+						super_tag,
+						boost::add_vertex({super_tag, super_path}, derivation_graph_)
+					);
+				}
+
+				boost::add_edge(
+					derivation_map_[&active],
+					derivation_map_[super_tag],
+					{cfg, file, start_line},
+					derivation_graph_
+				);
+			}
+		}
+
 		for(const auto& tag : active.tags(cfg)) {
 			int cnt = counter_.top()[tag.first].cnt;
 
@@ -365,6 +409,40 @@ void schema_validator::validate(const config& cfg, const std::string& name, int
 	}
 }
 
+void schema_validator::detect_derivation_cycles() {
+	boost::depth_first_search(
+		derivation_graph_,
+		boost::visitor(utils::back_edge_detector([&] (const derivation_graph_t::edge_descriptor edge) {
+			const auto source = std::find_if(
+				derivation_map_.begin(),
+				derivation_map_.end(),
+				[&] (const auto& derivation) {
+					return derivation.second == boost::source(edge, derivation_graph_);
+				}
+			);
+
+			assert(source != derivation_map_.end());
+
+			const auto target = std::find_if(
+				derivation_map_.begin(),
+				derivation_map_.end(),
+				[&] (const auto& derivation) {
+					return derivation.second == boost::target(edge, derivation_graph_);
+				}
+			);
+
+			assert(target != derivation_map_.end());
+
+			const auto& tag_path = derivation_graph_[source->second].second;
+			const auto& super_path = derivation_graph_[target->second].second;
+
+			const auto& [cfg, file, line] = derivation_graph_[edge];
+
+			queue_message(cfg, SUPER_CYCLE, file, line, 0, tag_path, name_, super_path);
+		}))
+	);
+}
+
 void schema_validator::validate_key(
 		const config& cfg, const std::string& name, const config_attribute_value& value, int start_line, const std::string& file)
 {
@@ -444,6 +522,9 @@ void schema_validator::print(message_info& el)
 	case MISSING_KEY:
 		errors_.emplace_back(missing_key_error(el.file, el.line, el.tag, el.key, create_exceptions_));
 		break;
+	case SUPER_CYCLE:
+		errors_.emplace_back(inheritance_cycle_error(el.file, el.line, el.tag, el.key, el.value, create_exceptions_));
+		break;
 	}
 }
 

src/serialization/schema_validator.hpp

@@ -83,7 +83,7 @@ private:
 
 protected:
 	using message_type = int;
-	enum { WRONG_TAG, EXTRA_TAG, MISSING_TAG, EXTRA_KEY, MISSING_KEY, WRONG_VALUE, NEXT_ERROR };
+	enum { WRONG_TAG, EXTRA_TAG, MISSING_TAG, EXTRA_KEY, MISSING_KEY, WRONG_VALUE, SUPER_CYCLE, NEXT_ERROR };
 
 	/**
 	 * Messages are cached.
@@ -164,6 +164,20 @@ protected:
 	bool have_active_tag() const;
 	bool is_valid() const {return config_read_;}
 	wml_type::ptr find_type(const std::string& type) const;
+
+private:
+	using derivation_graph_t = boost::adjacency_list<
+		boost::vecS,
+		boost::vecS,
+		boost::directedS,
+		std::pair<const wml_tag*, std::string>,
+		std::tuple<config, std::string, int>
+	>;
+
+	derivation_graph_t derivation_graph_;
+	std::map<const wml_tag*, derivation_graph_t::vertex_descriptor> derivation_map_;
+
+	void detect_derivation_cycles();
 };
 
 // A validator specifically designed for validating a schema