Benoît Canet <benoit.ca...@irqsave.net> writes: > The Thursday 08 May 2014 à 20:30:33 (+0200), Markus Armbruster wrote : [...] >> There are two reasons to detect cycles. The technical one is preventing >> infinite expansion. No longer applies with idempotent include. The >> other, more abstract one is rejecting nonsensical use of the include >> directive. I think that one still applies. [...] >> > @@ -102,17 +102,16 @@ class QAPISchema: >> > 'Expected a file name (string), >> > got: %s' >> > % include) >> > include_path = os.path.join(self.input_dir, include) >> > - if any(include_path == elem[1] >> > - for elem in self.include_hist): >> > - raise QAPIExprError(expr_info, "Inclusion loop for %s" >> > - % include) >> > + # make include idempotent by skipping further includes >> > + if include_path in [x[1] for x in include_hist]: >> > + continue >> >> Loses cycle detection. > > It simply also skip cycle includes. If cycle are skipped then cannot do no > harm.
Your argument is based exclusively on the technical reason to detect cycles: cycles need to be caught because they cause infinite recursion. Since there is no infinite recursion with idempotent include, cycles are just fine. I'm arguing from a more abstract point of view: cycles should be rejected because they're nonsensical. The fact that they can cause infinite recursion is an implementation detail. Even without infinite recursion, they're just as nonsensical as ever. [...]
