update(userspace/engine): broader err catching support in macro resolver

Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>

userspace/engine/filter_macro_resolver.cpp

@@ -24,8 +24,9 @@ bool filter_macro_resolver::run(libsinsp::filter::ast::expr*& filter)
 {
 	m_unknown_macros.clear();
 	m_resolved_macros.clear();
+	m_errors.clear();
 
-	visitor v(m_unknown_macros, m_resolved_macros, m_macros);
+	visitor v(m_errors, m_unknown_macros, m_resolved_macros, m_macros);
 	v.m_node_substitute = nullptr;
 	filter->accept(&v);
 	if (v.m_node_substitute)
@@ -40,8 +41,9 @@ bool filter_macro_resolver::run(std::shared_ptr<libsinsp::filter::ast::expr>& fi
 {
 	m_unknown_macros.clear();
 	m_resolved_macros.clear();
+	m_errors.clear();
 
-	visitor v(m_unknown_macros, m_resolved_macros, m_macros);
+	visitor v(m_errors, m_unknown_macros, m_resolved_macros, m_macros);
 	v.m_node_substitute = nullptr;
 	filter->accept(&v);
 	if (v.m_node_substitute)
@@ -58,12 +60,17 @@ void filter_macro_resolver::set_macro(
 	m_macros[name] = macro;
 }
 
-const filter_macro_resolver::macro_info_map& filter_macro_resolver::get_unknown_macros() const
+const std::vector<filter_macro_resolver::value_info>& filter_macro_resolver::get_unknown_macros() const
 {
 	return m_unknown_macros;
 }
 
-const filter_macro_resolver::macro_info_map& filter_macro_resolver::get_resolved_macros() const
+const std::vector<filter_macro_resolver::value_info>& filter_macro_resolver::get_errors() const
+{
+	return m_errors;
+}
+
+const std::vector<filter_macro_resolver::value_info>& filter_macro_resolver::get_resolved_macros() const
 {
 	return m_resolved_macros;
 }
@@ -133,8 +140,13 @@ void filter_macro_resolver::visitor::visit(ast::value_expr* e)
 		const auto& prevref = std::find(m_macros_path.begin(), m_macros_path.end(), macro->first);
 		if (prevref != m_macros_path.end())
 		{
-			throw falco_exception("detected reference loop in macro '" + macro->first + "'");
+			auto msg = "reference loop in macro '" + macro->first + "'";
+			m_errors.push_back({msg, e->get_pos()});
+			m_node_substitute = nullptr;
+			m_unknown_macros.push_back({e->value, e->get_pos()});
+			return;
 		}
+
 		m_macros_path.push_back(macro->first);
 		m_node_substitute = nullptr;
 		auto new_node = ast::clone(macro->second.get());
@@ -145,12 +157,12 @@ void filter_macro_resolver::visitor::visit(ast::value_expr* e)
 		{
 			m_node_substitute = std::move(new_node);
 		}
-		m_resolved_macros[e->value] = e->get_pos();
+		m_resolved_macros.push_back({e->value, e->get_pos()});
 		m_macros_path.pop_back();
 	}
 	else
 	{
 		m_node_substitute = nullptr;
-		m_unknown_macros[e->value] = e->get_pos();
+		m_unknown_macros.push_back({e->value, e->get_pos()});
 	}
 }

userspace/engine/filter_macro_resolver.h

@@ -59,16 +59,17 @@ class filter_macro_resolver
 			std::shared_ptr<libsinsp::filter::ast::expr> macro);
 
 		/*!
-		        \brief used in get_{resolved,unknown}_macros
+		    \brief used in get_{resolved,unknown}_macros and get_errors
+			to represent an identifer/string value along with an AST position. 
 		*/
-		typedef std::unordered_map<std::string,libsinsp::filter::ast::pos_info> macro_info_map;
+		typedef std::pair<std::string,libsinsp::filter::ast::pos_info> value_info;
 
 		/*!
 			\brief Returns a set containing the names of all the macros
 			substituted during the last invocation of run(). Should be
 			non-empty if the last invocation of run() returned true.
 		*/
-		const macro_info_map& get_resolved_macros() const;
+		const std::vector<value_info>& get_resolved_macros() const;
 
 		/*!
 			\brief Returns a set containing the names of all the macros
@@ -76,7 +77,13 @@ class filter_macro_resolver
 			A macro remains unresolved if it is found inside the processed
 			filter but it was not defined with set_macro();
 		*/
-		const macro_info_map& get_unknown_macros() const;
+		const std::vector<value_info>& get_unknown_macros() const;
+
+		/*!
+			\brief Returns a list of errors occurred during
+			the latest invocation of run().
+		*/
+		const std::vector<value_info>& get_errors() const;
 
 	private:
 		typedef std::unordered_map<
@@ -86,8 +93,15 @@ class filter_macro_resolver
 
 		struct visitor : public libsinsp::filter::ast::expr_visitor
 		{
-			visitor(macro_info_map& unknown_macros, macro_info_map& resolved_macros, macro_defs& macros)
-				: m_unknown_macros(unknown_macros), m_resolved_macros(resolved_macros), m_macros(macros) {}
+			visitor(
+				std::vector<value_info>& errors,
+				std::vector<value_info>& unknown_macros,
+				std::vector<value_info>& resolved_macros,
+				macro_defs& macros):
+					m_errors(errors),
+					m_unknown_macros(unknown_macros),
+					m_resolved_macros(resolved_macros),
+					m_macros(macros) {}
 			visitor(visitor&&) = default;
 			visitor& operator = (visitor&&) = default;
 			visitor(const visitor&) = delete;
@@ -95,9 +109,9 @@ class filter_macro_resolver
 
 			std::vector<std::string> m_macros_path;
 			std::unique_ptr<libsinsp::filter::ast::expr> m_node_substitute;
-			macro_info_map& m_unknown_macros;
-			macro_info_map& m_resolved_macros;
-
+			std::vector<value_info>& m_errors;
+			std::vector<value_info>& m_unknown_macros;
+			std::vector<value_info>& m_resolved_macros;
 			macro_defs& m_macros;
 
 			void visit(libsinsp::filter::ast::and_expr* e) override;
@@ -109,7 +123,8 @@ class filter_macro_resolver
 			void visit(libsinsp::filter::ast::binary_check_expr* e) override;
 		};
 
-		macro_info_map m_unknown_macros;
-		macro_info_map m_resolved_macros;
+		std::vector<value_info> m_errors;
+		std::vector<value_info> m_unknown_macros;
+		std::vector<value_info> m_resolved_macros;
 		macro_defs m_macros;
 };

userspace/engine/rule_loader_compiler.cpp

@@ -248,18 +248,19 @@ static void resolve_macros(
 	}
 	macro_resolver.run(ast);
 
-	// Note: only complaining about the first unknown macro
-	const filter_macro_resolver::macro_info_map& unresolved_macros = macro_resolver.get_unknown_macros();
-	if(!unresolved_macros.empty())
+	// Note: only complaining about the first error or unknown macro
+	const auto& errors_macros = macro_resolver.get_errors();
+	const auto& unresolved_macros = macro_resolver.get_unknown_macros();
+	if(!errors_macros.empty() || !unresolved_macros.empty())
 	{
-		auto it = unresolved_macros.begin();
-		const rule_loader::context cond_ctx(it->second, condition, ctx);
-
-		THROW(true,
-		      std::string("Undefined macro '")
-		                    + it->first
-		      + "' used in filter.",
-		      cond_ctx);
+		auto errpos = !errors_macros.empty()
+			? errors_macros.begin()->second
+			: unresolved_macros.begin()->second;
+		std::string errmsg = !errors_macros.empty()
+			? errors_macros.begin()->first
+			: ("Undefined macro '" + unresolved_macros.begin()->first + "' used in filter.");
+		const rule_loader::context cond_ctx(errpos, condition, ctx);
+		THROW(true, errmsg, cond_ctx);
 	}
 
 	for (auto &it : macro_resolver.get_resolved_macros())