Bindings: Improve diagnostics for the rules engine

Note which rule has a modification effect.

2 files changed, 69 insertions, 25 deletions

diff --git a/find-modules/rules_engine.py b/find-modules/rules_engine.py
index ef4a59d6..b5b783e0 100755
--- a/find-modules/rules_engine.py
+++ b/find-modules/rules_engine.py
@@ -69,6 +69,11 @@ def _parents(container):
 
 class Rule(object):
     def __init__(self, db, rule_number, fn, pattern_zip):
+        #
+        # Derive a useful name for diagnostic purposes.
+        #
+        caller = os.path.basename(inspect.stack()[3][1])
+        self.name =  "{}:{}[{}],{}".format(caller, type(db).__name__, rule_number, fn.__name__)
         self.rule_number = rule_number
         self.fn = fn
         self.usage = 0
@@ -85,10 +90,22 @@ class Rule(object):
         return self.matcher.match(candidate)
 
     def trace_result(self, parents, item, original, modified):
+        """
+        Record any modification both in the log and the returned result. If a rule fired, but
+        caused no modification, that is logged.
+
+        :return: Modifying rule or None.
+        """
         fqn = parents + "::" + original["name"] + "[" + str(item.extent.start.line) + "]"
-        self._trace_result(fqn, original, modified)
+        return self._trace_result(fqn, original, modified)
 
     def _trace_result(self, fqn, original, modified):
+        """
+        Record any modification both in the log and the returned result. If a rule fired, but
+        caused no modification, that is logged.
+
+        :return: Modifying rule or None.
+        """
         if not modified["name"]:
             logger.debug(_("Rule {} suppressed {}, {}").format(self, fqn, original))
         else:
@@ -101,9 +118,11 @@ class Rule(object):
                 logger.debug(_("Rule {} modified {}, {}->{}").format(self, fqn, original, modified))
             else:
                 logger.warn(_("Rule {} did not modify {}, {}").format(self, fqn, original))
+                return None
+        return self
 
     def __str__(self):
-        return "[{},{}]".format(self.rule_number, self.fn.__name__)
+        return self.name
 
 
 class AbstractCompiledRuleDb(object):
@@ -116,7 +135,7 @@ class AbstractCompiledRuleDb(object):
             if len(raw_rule) != len(parameter_names) + 1:
                 raise RuntimeError(_("Bad raw rule {}: {}: {}").format(db.__name__, raw_rule, parameter_names))
             z = zip(raw_rule[:-1], parameter_names)
-            self.compiled_rules.append(Rule(db, i, raw_rule[-1], z))
+            self.compiled_rules.append(Rule(self, i, raw_rule[-1], z))
         self.candidate_formatter = _SEPARATOR.join(["{}"] * len(parameter_names))
 
     def _match(self, *args):
@@ -138,7 +157,7 @@ class AbstractCompiledRuleDb(object):
     def dump_usage(self, fn):
         """ Dump the usage counts."""
         for rule in self.compiled_rules:
-            fn(self.__class__.__name__, str(rule), rule.usage)
+            fn(str(rule), rule.usage)
 
 
 class ContainerRuleDb(AbstractCompiledRuleDb):
@@ -201,14 +220,16 @@ class ContainerRuleDb(AbstractCompiledRuleDb):
         Walk over the rules database for containers, applying the first matching transformation.
 
         :param container:           The clang.cindex.Cursor for the container.
-        :param sip:                 The SIP dict.
+        :param sip:                 The SIP dict (may be modified on return).
+        :return:                    Modifying rule or None (even if a rule matched, it may not modify things).
         """
         parents = _parents(container)
         matcher, rule = self._match(parents, sip["name"], sip["template_parameters"], sip["decl"], sip["base_specifiers"])
         if matcher:
             before = deepcopy(sip)
             rule.fn(container, sip, matcher)
-            rule.trace_result(parents, container, before, sip)
+            return rule.trace_result(parents, container, before, sip)
+        return None
 
 
 class FunctionRuleDb(AbstractCompiledRuleDb):
@@ -276,14 +297,17 @@ class FunctionRuleDb(AbstractCompiledRuleDb):
 
         :param container:           The clang.cindex.Cursor for the container.
         :param function:            The clang.cindex.Cursor for the function.
-        :param sip:                 The SIP dict.
+        :param sip:                 The SIP dict (may be modified on return).
+        :return:                    Modifying rule or None (even if a rule matched, it may not modify things).
         """
         parents = _parents(function)
         matcher, rule = self._match(parents, sip["name"], ", ".join(sip["template_parameters"]), sip["fn_result"], ", ".join(sip["parameters"]))
         if matcher:
+            sip.setdefault("code", "")
             before = deepcopy(sip)
             rule.fn(container, function, sip, matcher)
-            rule.trace_result(parents, function, before, sip)
+            return rule.trace_result(parents, function, before, sip)
+        return None
 
 
 class ParameterRuleDb(AbstractCompiledRuleDb):
@@ -349,14 +373,17 @@ class ParameterRuleDb(AbstractCompiledRuleDb):
         :param container:           The clang.cindex.Cursor for the container.
         :param function:            The clang.cindex.Cursor for the function.
         :param parameter:           The clang.cindex.Cursor for the parameter.
-        :param sip:                 The SIP dict.
+        :param sip:                 The SIP dict (may be modified on return).
+        :return:                    Modifying rule or None (even if a rule matched, it may not modify things).
         """
         parents = _parents(function)
         matcher, rule = self._match(parents, function.spelling, sip["name"], sip["decl"], sip["init"])
         if matcher:
+            sip.setdefault("code", "")
             before = deepcopy(sip)
             rule.fn(container, function, parameter, sip, matcher)
-            rule.trace_result(parents, parameter, before, sip)
+            return rule.trace_result(parents, parameter, before, sip)
+        return None
 
 
 class VariableRuleDb(AbstractCompiledRuleDb):
@@ -414,14 +441,17 @@ class VariableRuleDb(AbstractCompiledRuleDb):
 
         :param container:           The clang.cindex.Cursor for the container.
         :param variable:            The clang.cindex.Cursor for the variable.
-        :param sip:                 The SIP dict.
+        :param sip:                 The SIP dict (may be modified on return).
+        :return:                    Modifying rule or None (even if a rule matched, it may not modify things).
         """
         parents = _parents(variable)
         matcher, rule = self._match(parents, sip["name"], sip["decl"])
         if matcher:
+            sip.setdefault("code", "")
             before = deepcopy(sip)
             rule.fn(container, variable, sip, matcher)
-            rule.trace_result(parents, variable, before, sip)
+            return rule.trace_result(parents, variable, before, sip)
+        return None
 
 
 class RuleSet(object):
@@ -473,11 +503,12 @@ class RuleSet(object):
 
     def dump_unused(self):
         """Usage statistics, to identify unused rules."""
-        def dumper(db_name, rule, usage):
+        def dumper(rule, usage):
             if usage:
-                logger.info(_("Rule {}::{} used {} times".format(db_name, rule, usage)))
+                logger.info(_("Rule {} used {} times".format(rule, usage)))
             else:
-                logger.warn(_("Rule {}::{} unused".format(db_name, rule)))
+                logger.warn(_("Rule {} was not used".format(rule)))
+
         for db in [self.container_rules(), self.function_rules(), self.parameter_rules(),
                    self.variable_rules()]:
             db.dump_usage(dumper)
diff --git a/find-modules/sip_generator.py b/find-modules/sip_generator.py
index e3101fb1..a67263ff 100644
--- a/find-modules/sip_generator.py
+++ b/find-modules/sip_generator.py
@@ -304,16 +304,19 @@ class SipGenerator(object):
             sip["decl"] = container_type
             sip["base_specifiers"] = ", ".join(base_specifiers)
             sip["body"] = body
-            self.rules.container_rules().apply(container, sip)
+            modifying_rule = self.rules.container_rules().apply(container, sip)
             pad = " " * (level * 4)
             if sip["name"]:
-                decl = pad + sip["decl"]
+                decl = ""
+                if modifying_rule:
+                    decl += "// Modified {} (by {}):\n".format(SipGenerator.describe(container), modifying_rule)
+                decl += pad + sip["decl"]
                 if "External" in sip["annotations"]:
                     #
                     # SIP /External/ does not seem to work as one might wish. Suppress.
                     #
                     body = decl + " /External/;\n"
-                    body = pad + "// Discarded {}\n".format(SipGenerator.describe(container))
+                    body = pad + "// Discarded {} (by {})\n".format(SipGenerator.describe(container), "/External/ handling")
                 else:
                     if sip["base_specifiers"]:
                         decl += ": " + sip["base_specifiers"]
@@ -325,7 +328,7 @@ class SipGenerator(object):
                     decl += "%TypeHeaderCode\n#include <{}>\n%End\n".format(include_filename)
                     body = decl + sip["body"] + pad + "};\n"
             else:
-                body = pad + "// Discarded {}\n".format(SipGenerator.describe(container))
+                body = pad + "// Discarded {} (by {})\n".format(SipGenerator.describe(container), modifying_rule)
         return body
 
     def _get_access_specifier(self, member, level):
@@ -378,6 +381,7 @@ class SipGenerator(object):
             "name": function.spelling,
         }
         parameters = []
+        parameter_modifying_rules = []
         template_parameters = []
         for child in function.get_children():
             if child.kind == CursorKind.PARM_DECL:
@@ -394,7 +398,9 @@ class SipGenerator(object):
                     "init": self._fn_get_parameter_default(function, child),
                     "annotations": set()
                 }
-                self.rules.parameter_rules().apply(container, function, child, child_sip)
+                modifying_rule = self.rules.parameter_rules().apply(container, function, child, child_sip)
+                if modifying_rule:
+                    parameter_modifying_rules.append("// Modified {} (by {}):\n".format(SipGenerator.describe(child), modifying_rule))
                 decl = child_sip["decl"]
                 if child_sip["annotations"]:
                     decl += " /" + ",".join(child_sip["annotations"]) + "/"
@@ -433,11 +439,18 @@ class SipGenerator(object):
             sip["fn_result"] = function.result_type.spelling
         sip["parameters"] = parameters
         sip["prefix"], sip["suffix"] = self._fn_get_decorators(function)
-        self.rules.function_rules().apply(container, function, sip)
+        modifying_rule = self.rules.function_rules().apply(container, function, sip)
         pad = " " * (level * 4)
         if sip["name"]:
+            decl = ""
+            if modifying_rule:
+                decl += "// Modified {} (by {}):\n".format(SipGenerator.describe(function), modifying_rule) + pad
+            decl += pad.join(parameter_modifying_rules)
+            if parameter_modifying_rules:
+                decl += pad
+
             sip["template_parameters"] = ", ".join(sip["template_parameters"])
-            decl = sip["name"] + "(" + ", ".join(sip["parameters"]) + ")"
+            decl += sip["name"] + "(" + ", ".join(sip["parameters"]) + ")"
             if sip["fn_result"]:
                 decl = sip["fn_result"] + " " + decl
             decl = pad + sip["prefix"] + decl + sip["suffix"]
@@ -445,7 +458,7 @@ class SipGenerator(object):
                 decl = pad + "template <" + sip["template_parameters"] + ">\n" + decl
             decl += ";\n"
         else:
-            decl = pad + "// Discarded {}\n".format(SipGenerator.describe(function))
+            decl = pad + "// Discarded {} (by {})\n".format(SipGenerator.describe(function), modifying_rule)
         return decl
 
     def _fn_get_decorators(self, function):
@@ -610,7 +623,7 @@ class SipGenerator(object):
         #
         decl = "{} {}".format(variable.type.spelling, variable.spelling)
         sip["decl"] = decl
-        self.rules.variable_rules().apply(container, variable, sip)
+        modifying_rule = self.rules.variable_rules().apply(container, variable, sip)
 
         pad = " " * (level * 4)
         if sip["name"]:
@@ -623,7 +636,7 @@ class SipGenerator(object):
             else:
                 decl = pad + decl + ";\n"
         else:
-            decl = pad + "// Discarded {}\n".format(SipGenerator.describe(variable))
+            decl = pad + "// Discarded {} (by {})\n".format(SipGenerator.describe(variable), modifying_rule)
         return decl
 
     def _read_source(self, extent):