Recursion guard and static klammer checking

A klammer that reaches itself, directly or through a cycle, expanded
until the C++ stack was exhausted: the process died from SIGSEGV with no
message and no location. The former limit guarded only the top-level
fixed-point iteration, never the descent through klammer application. A
depth guard now raises a recursion error naming the klammer and where it
was applied. The same loop's termination test moves from "the katom list
stopped growing" to "a pass applied no klammer", since a klammer whose
body expands to nothing is a reduction that adds no katoms; exceeding the
round limit is now an error rather than a message followed by rendering a
document with live klammers still in it.

ktext --check locates every klammer application written in a document or
in a klammer body and checks name existence, argument count, option
names, and target coverage without applying anything, reporting all
problems at once. This is possible because Klammertext has no catcodes:
katom structure is fixed when a file is read, so a klammer body has a
determinate shape before it is expanded. The check therefore reaches what
the engine cannot -- the branch of a @cond that is not selected, and
bodies a given render never enters.

@cond's set of truth values is an open language question, so its meaning
is unchanged here; an unrecognized predicate now warns, giving its value
and location.

tst/ gains recursion_test.sh (7 cases) and check_test.sh (19 cases), and
this snapshot's test Makefile is generated from the shipped suite list so
the two cannot drift apart.

(from dev c27e63802406)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

mac/Makefile

@@ -10,7 +10,7 @@ include $(K)/env/makefile.env
 BASENAMES := util error locator file argv character ktype katom katom_list \
              log show command argument argument_set argtype argtype_registry \
              state eval eval_python eval_cpp klammer klammer_registry klammerset klammerset_registry deftype \
-             target target_registry machine font_store
+             target target_registry machine font_store check
 
 SOURCES := $(addsuffix .cpp,$(BASENAMES))
 OBJECTS := $(addsuffix .o,$(BASENAMES))

mac/check.cpp

@@ -0,0 +1,298 @@
+#include <algorithm>
+#include <iostream>
+#include <optional>
+#include <sstream>
+
+#include "check.h"
+#include "machine.h"
+#include "katom.h"
+#include "katom_list.h"
+#include "util.h"
+
+namespace {
+
+// One application's argument shape, as written: how many positional parts it
+// supplies and which option names it names.  Both are counted at nesting
+// depth 0 within the application's span, so a bar or an option name belonging
+// to a nested klammer is not miscounted as this one's.
+//
+// This is the same rule the engine uses at run time, but it has to be stated
+// again here rather than reused: argument_split() walks the range flatly,
+// which is correct THERE because application is post-order -- by the time a
+// klammer is applied its nested spans have already been reduced to text.  At
+// check time nothing has been reduced, so the nesting is still present and
+// must be tracked.  (The depth-0 rule is the same one cond_separator_bars()
+// applies for @cond; see doc/cond_evaluation_order.md.)
+struct Application_shape
+{
+    int m_positional { 0 };
+    std::vector<std::string> m_options {};
+};
+
+bool is_boundary_katom(const Katom& k)
+{
+    return k.m_type == katom_t::bar || k.m_type == katom_t::option_name;
+}
+
+Application_shape application_shape(katom_list::const_iterator begin, katom_list::const_iterator end)
+{
+    Application_shape shape {};
+
+    auto first = begin;
+    while (first != end && first->is_whitespace()) ++first;
+    if (first == end) return shape;
+
+    // function_symbol_parts() prepends a synthetic bar when the argument list
+    // does not open with an option name, so that content before the first bar
+    // counts as a positional part.  Mirror that, or "@f a @" would count zero
+    // positional arguments.
+    bool in_positional = first->m_type != katom_t::option_name;
+    if (in_positional) shape.m_positional = 1;
+
+    int depth = 0;
+    for (auto k = first; k != end; ++k) {
+        if (depth == 0 && is_boundary_katom(*k)) {
+            if (k->m_type == katom_t::bar) {
+                ++shape.m_positional;
+            } else {
+                shape.m_options.push_back(k->m_text.substr(1));
+            }
+        }
+        if (level_increase(*k)) {
+            ++depth;
+        } else if (level_decrease(*k)) {
+            --depth;
+        }
+    }
+    return shape;
+}
+
+// The span of the application opening at `begin`, as [begin, end): end is one
+// past the matching close.  Empty when the span is unclosed -- which the
+// engine reports separately, so the checker just stops descending.
+//
+// The result must be an optional rather than "list_end means unclosed": a
+// span that closes on the very last katom of the list -- a klammer body that
+// is nothing but one application, "@@u : @nosuch x @ @@" -- ends exactly AT
+// list_end while being perfectly well formed, and conflating the two made the
+// checker silently skip every such body.
+std::optional<katom_list::const_iterator> span_end(
+    katom_list::const_iterator begin, katom_list::const_iterator list_end)
+{
+    int depth = 0;
+    for (auto k = begin; k != list_end; ++k) {
+        if (level_increase(*k)) {
+            ++depth;
+        } else if (level_decrease(*k)) {
+            if (--depth == 0) return k + 1;
+        }
+    }
+    return {};
+}
+
+bool skip_katom(const Katom& k)
+{
+    return k.m_type == katom_t::replaced
+        || k.m_type == katom_t::ignored
+        || k.m_type == katom_t::literal;
+}
+
+// Argument spans of the primitives whose contents are not Klammertext: @eval
+// receives code, @read a filename.  @cond is NOT in this set -- its branches
+// are Klammertext, and checking the branch that is not selected is the main
+// thing the checker is for.
+bool opens_uncheckable_span(const Katom& k)
+{
+    return k.m_type == katom_t::eval_begin || k.m_type == katom_t::read_begin;
+}
+
+class Checker
+{
+public:
+    Checker(Machine& machine, std::vector<Diagnostic>& diagnostics)
+        : m_machine(machine)
+        , m_diagnostics(diagnostics)
+        {}
+
+    void check_list(const katom_list& katoms, const std::string& target,
+                    const std::string& context);
+
+private:
+    void check_application(
+        const std::string& name, const Klammer& klammer,
+        katom_list::const_iterator begin, katom_list::const_iterator end,
+        const std::string& target, const std::string& context);
+
+    void error(const std::string& message, const std::string& context, const Locator& loc)
+    {
+        m_diagnostics.emplace_back("error", message, context, loc);
+    }
+
+    Machine& m_machine;
+    std::vector<Diagnostic>& m_diagnostics;
+};
+
+void Checker::check_application(
+    const std::string& name, const Klammer& klammer,
+    katom_list::const_iterator begin, katom_list::const_iterator end,
+    const std::string& target, const std::string& context)
+{
+    // Arity is a property of the klammer's rationalized parameter set, which
+    // is shared by all of its target definitions, so it is checked once here
+    // rather than per target.
+    const Parameter_set& parameters = klammer.m_parameters;
+    Application_shape shape = application_shape(begin + 1, end - 1);
+
+    auto required = static_cast<int>(parameters.m_positional.size());
+    bool has_rest = !parameters.m_rest.empty();
+
+    if (shape.m_positional < required) {
+        std::stringstream ss {};
+        ss << "@" << name << " needs " << required << " positional "
+           << plural("argument", required) << " but is given " << shape.m_positional
+           << ".  Positional arguments are separated by \"|\".";
+        error(ss.str(), context, begin->m_loc);
+    } else if (shape.m_positional > required && !has_rest) {
+        std::stringstream ss {};
+        ss << "@" << name << " takes " << required << " positional "
+           << plural("argument", required) << " but is given " << shape.m_positional << ".";
+        error(ss.str(), context, begin->m_loc);
+    }
+
+    std::vector<std::string> seen {};
+    for (const auto& option : shape.m_options) {
+        if (std::ranges::count(parameters.m_optional_names, option) == 0) {
+            std::stringstream ss {};
+            ss << "@" << name << " has no optional argument \":" << option << "\".";
+            if (!parameters.m_optional_names.empty()) {
+                ss << "  It accepts: :" << join(parameters.m_optional_names, " :") << ".";
+            }
+            error(ss.str(), context, begin->m_loc);
+        } else if (std::ranges::count(seen, option) > 0) {
+            error("@" + name + " is given \":" + option + "\" more than once.",
+                  context, begin->m_loc);
+        }
+        seen.push_back(option);
+    }
+
+    // Target coverage.  A klammer may be declared (.k) and defined for some
+    // targets but not the one being built; run time only discovers this if the
+    // application is actually reached.
+    //
+    // Not checked under the general target: a general body is not applied
+    // under "*", it is copied to every target that lacks its own definition
+    // and applied under whichever of those is in force (copy_general_klammer_
+    // to_undefined() in klammer.cpp).  So an application inside it resolves
+    // against a real target, and the per-target passes are where coverage is
+    // decided.  Checking it here reported @b -- defined for html/tex/pdf/txt
+    // but not for "*" -- as missing from a general body that in fact works.
+    if (target != Target_registry::general_name && klammer.m_defloc.count(target) == 0) {
+        std::stringstream ss {};
+        ss << "@" << name << " is not defined for the target \"" << target << "\".";
+        strings_t targets = klammer.get_target_names();
+        if (!targets.empty()) {
+            ss << "  It is defined for: " << join(targets, ", ") << ".";
+        }
+        error(ss.str(), context, begin->m_loc);
+    }
+}
+
+void Checker::check_list(
+    const katom_list& katoms, const std::string& target, const std::string& context)
+{
+    for (auto k = katoms.begin(); k != katoms.end(); ++k) {
+        if (k->m_type == katom_t::ignore_rest) break;
+        if (skip_katom(*k)) continue;
+
+        // Code and filenames, not applications: skip the whole span.
+        if (opens_uncheckable_span(*k)) {
+            auto skip_to = span_end(k, katoms.end());
+            if (!skip_to) return;
+            k = *skip_to - 1;
+            continue;
+        }
+        if (k->m_type != katom_t::apply_begin) continue;
+
+        auto closed = span_end(k, katoms.end());
+        if (!closed) return;               // unclosed; the engine reports it
+        auto end = *closed;
+
+        std::string name = trim_char(k->m_text, '@');
+        auto found = m_machine.m_klammers.m_klammers.find(name);
+        if (found == m_machine.m_klammers.m_klammers.end()) {
+            error("The klammer @" + name + " is not defined.", context, k->m_loc);
+            continue;
+        }
+        check_application(name, found->second, k, end, target, context);
+
+        // A literal parameter's content is raw text -- a "@" inside it is not
+        // an application -- so do not descend into it.
+        if (found->second.has_literal_param()) {
+            k = end - 1;
+        }
+    }
+}
+
+} // namespace
+
+std::vector<Diagnostic> check_machine(Machine& machine, const std::string& target)
+{
+    std::vector<Diagnostic> diagnostics {};
+    Checker checker(machine, diagnostics);
+
+    // With no target named, check every target the machine defines, plus the
+    // general one -- a klammer defined without a target suffix has its body
+    // filed under the general name, and with no klammer set loaded that is the
+    // only target there is.
+    strings_t targets {};
+    if (target == Target_registry::general_name) {
+        targets = machine.m_targets.user_defined();
+        targets.push_back(Target_registry::general_name);
+    } else {
+        targets.push_back(target);
+    }
+
+    for (const auto& t : targets) {
+        checker.check_list(machine.m_katoms, t, "document");
+        for (const auto& [name, klammer] : machine.m_klammers.m_klammers) {
+            auto body = klammer.m_body.find(t);
+            if (body == klammer.m_body.end()) continue;
+            checker.check_list(body->second, t, "body of @" + name);
+        }
+    }
+
+    // The same text is checked once per target, so a fault that does not
+    // depend on the target -- an undefined name, a wrong argument count --
+    // is found once per target and must be reported once.  Target coverage
+    // names its target in the message, so those stay distinct.  Hence the
+    // context deliberately does NOT carry the target: it is what makes the
+    // target-independent duplicates compare equal.
+    std::vector<Diagnostic> unique {};
+    for (const auto& d : diagnostics) {
+        bool seen = std::any_of(
+            unique.begin(), unique.end(), [&d](const Diagnostic& u) {
+                return u.m_severity == d.m_severity && u.m_message == d.m_message
+                    && u.m_context == d.m_context && u.m_loc.str() == d.m_loc.str(); });
+        if (!seen) unique.push_back(d);
+    }
+    return unique;
+}
+
+int report_diagnostics(const std::vector<Diagnostic>& diagnostics, std::ostream& os)
+{
+    int errors = 0;
+    for (const auto& d : diagnostics) {
+        if (d.m_severity == "error") ++errors;
+        os << d.m_severity << ": " << d.m_message << "\n";
+        if (!d.m_context.empty()) {
+            os << "  in " << d.m_context << "\n";
+        }
+        if (!d.m_loc.m_filename.empty()) {
+            os << "  " << d.m_loc.desc() << "\n";
+        }
+        os << "\n";
+    }
+    os << diagnostics.size() << " " << plural("diagnostic", diagnostics.size())
+       << ", " << errors << " " << plural("error", errors) << "\n";
+    return errors;
+}

mac/check.h

@@ -0,0 +1,65 @@
+#pragma once
+
+#include <iosfwd>
+#include <string>
+#include <vector>
+
+#include "locator.h"
+
+class Machine;
+
+// Static checking of klammer applications.
+//
+// Klammertext can do something TeX structurally cannot: know the shape of a
+// klammer body before that body is expanded.  Katom structure is fixed when a
+// file is read -- there are no catcodes, so no later assignment can change how
+// text already read is divided into katoms -- which means every klammer
+// application that appears literally in a document or in a klammer body can be
+// located, named, and checked against the registry without running anything.
+//
+// This matters most where dynamic checking cannot reach.  @cond is a
+// non-strict special form: the branch it does not select is never applied, so
+// an undefined klammer or a wrong argument count sitting in that branch is
+// invisible at run time and stays invisible until the day the predicate flips.
+// The same holds for a klammer body that is never applied for the target being
+// built.  The checker reports all of them.
+//
+// What it deliberately does NOT see: klammers produced by @eval (a generator's
+// result is text computed at run time), and the contents of @eval and @read
+// argument spans (code and filenames, not applications).  Its guarantee is
+// therefore about what is written, not about what will run.
+//
+// One thing it does not see that it SHOULD: a @cond written at the top level
+// of a document is resolved when the file is read (process_cond_katoms() runs
+// inside process_katoms()), so by the time anything can be checked the
+// unselected branch has already been discarded.  Inside a klammer body the
+// @cond survives until the klammer is applied, so body branches ARE checked --
+// which is where most of them are written.  Closing the gap means resolving
+// @cond at application time rather than at read time, which is part of the
+// pass-ordering question; see notes/Klammertext_improvements.md.
+struct Diagnostic
+{
+    Diagnostic(const std::string& severity, const std::string& message,
+               const std::string& context, const Locator& loc)
+        : m_severity(severity)
+        , m_message(message)
+        , m_context(context)
+        , m_loc(loc)
+        {}
+
+    std::string m_severity {};  // "error" or "warning"
+    std::string m_message {};
+    std::string m_context {};   // where it was found, e.g. "body of @s1 (tex)"
+    Locator m_loc {};
+};
+
+// Check every statically visible klammer application in the document and in
+// the body of every defined klammer, for the named target.  A target of "*"
+// (Target_registry::general_name) checks every defined target.  Diagnostics
+// accumulate: checking never stops at the first failure, because the point is
+// to see all of them at once.
+std::vector<Diagnostic> check_machine(Machine& machine, const std::string& target);
+
+// Print diagnostics, grouped in the order found, and return the number of
+// errors (warnings do not count).  Used by "ktext --check".
+int report_diagnostics(const std::vector<Diagnostic>& diagnostics, std::ostream& os);

mac/error.h

@@ -76,6 +76,17 @@ public:
         : Error("environment", description, locator, do_justify) {};
 };
 
+// Klammer application nested deeper than the engine's limit.  Raised by the
+// depth guard in Machine::apply_klammer(); without it a klammer that applies
+// itself (directly or through a cycle) exhausts the C++ stack and the process
+// dies with SIGSEGV and no diagnostic at all.
+class Recursion_error : public Error {
+public:
+    explicit Recursion_error(
+        const std::string& description, const Locator& locator=Locator(), bool do_justify=true)
+        : Error("recursion", description, locator, do_justify) {};
+};
+
 class Internal_error : public Error {
 public:
     explicit Internal_error(

mac/machine.cpp

@@ -25,6 +25,56 @@ Machine::Machine()
     */
 }
 
+// Klammer application recursion guard.
+//
+// Applying a klammer expands its body, which is processed and applied in
+// turn (apply_klammer -> process_katoms -> apply -> apply_klammer), so a
+// klammer that reaches itself -- directly (@@f : x @f@ @@) or through a
+// cycle -- descends without bound.  Before this guard the descent simply
+// exhausted the C++ stack: SIGSEGV, no message, no location.
+//
+// The counter is a translation-unit static rather than a Machine member for
+// two reasons: recursion can cross Machine instances (Eval::eval builds a
+// sub-Machine to re-read an @eval result, and that sub-Machine applies
+// klammers on the same C++ stack), and keeping it out of Machine avoids
+// changing the class layout shared with the dlopened sks/document.so.
+//
+// The limit bounds the C++ stack, not the language: it is far above any
+// plausible nesting depth in a document, and reaching it means a klammer
+// does not terminate.  NOTE: not thread-safe; if input files are ever
+// processed in parallel this needs to become thread_local.
+namespace {
+
+constexpr int max_apply_depth = 200;
+int apply_depth = 0;
+
+// Rounds of the top-level fixed-point loop (see Machine::apply below).  The
+// former limit of 5 was a silent truncation; it is now an error, so it is set
+// well clear of any legitimate chain of klammers generating klammers.
+constexpr int apply_round_limit = 100;
+
+class Depth_guard
+{
+public:
+    Depth_guard(const std::string& name, const Locator& loc)
+    {
+        if (apply_depth >= max_apply_depth) {
+            std::stringstream ss {};
+            ss << "Klammer application nested more than " << max_apply_depth
+               << " levels deep while applying " << q_(name) << ".\n"
+               << "A klammer that applies itself, directly or through a cycle "
+               << "of klammers, does not terminate.";
+            throw Recursion_error(ss.str(), loc, false);
+        }
+        ++apply_depth;
+    }
+    ~Depth_guard() { --apply_depth; }
+    Depth_guard(const Depth_guard&) = delete;
+    Depth_guard& operator=(const Depth_guard&) = delete;
+};
+
+} // namespace
+
 void Machine::process_eval_katoms(katom_list& katoms)
 {
     (void)K::log(3);
@@ -88,6 +138,35 @@ bool is_true(const std::string& s)
     return s == "True" || s == "true" || s == "1";
 }
 
+// @cond's predicate relation is currently partial in effect: is_true()
+// recognizes three strings as true and treats EVERYTHING else as false, so a
+// misspelled state variable, a "TRUE", a "yes", or a Python traceback all
+// silently select the false branch.
+//
+// What the truth values should be is an open language-policy question (see
+// notes/Klammertext_improvements.md, "The @cond predicate relation"), so the
+// semantics here is deliberately unchanged.  What is added is visibility: a
+// predicate outside the provisionally recognized sets below is reported, with
+// its value and location, so the cases can be found in real documents while
+// the policy is decided.  The recognized false set carries no semantics -- it
+// exists only to keep the diagnostic quiet for values that plainly mean false.
+bool is_recognized_predicate(const std::string& s)
+{
+    return s.empty()
+        || s == "True"  || s == "true"  || s == "1"
+        || s == "False" || s == "false" || s == "0";
+}
+
+void warn_unrecognized_predicate(const std::string& predicate, const Locator& loc)
+{
+    if (is_recognized_predicate(predicate)) return;
+    std::stringstream ss {};
+    ss << "The @cond predicate " << q_(predicate)
+       << " is not a recognized truth value, so the false branch was taken.\n"
+       << "  Recognized: true, True, 1 (true); false, False, 0, empty (false).";
+    warning(ss.str(), loc);
+}
+
 void Machine::process_cond_katoms(katom_list& katoms)
 {
     if (std::find_if(katoms.begin(), katoms.end(), begin_cond) != katoms.end()) {
@@ -104,6 +183,7 @@ void Machine::process_cond_katoms(katom_list& katoms)
                 check_bar_count(begin, bars.size());
                 auto bar_1 = bars[0];
                 std::string predicate = to_string(begin + 1, bar_1, true);
+                warn_unrecognized_predicate(predicate, begin->m_loc);
                 katom_list true_clause {};
                 katom_list false_clause {};
                 if (bars.size() == 2) {
@@ -460,6 +540,7 @@ katom_list Machine::apply_klammer(
     Klammer& klammer, const std::string& target, katom_iter arguments_begin, katom_iter arguments_end)
 {
     (void)K::log(3, "argument substitution", *arguments_begin, *(arguments_end - 1));
+    Depth_guard depth_guard(klammer.m_name, arguments_begin->m_loc);
     m_state.replace("K_loc", arguments_begin->m_loc.str(), false);
     auto [positional, optional, rest] =
         argument_split(arguments_begin + 1, arguments_end - 1, klammer.m_parameters.m_positional.size());
@@ -563,10 +644,11 @@ void Machine::apply_klammer_registry(
     katoms.insert(end, applied_katoms.begin(), applied_katoms.end());
 }
 
-void Machine::apply(
+int Machine::apply(
     Klammer_registry& klammer_registry, katom_list& katoms, const std::string& target)
 {
     (void)K::log(3, "Klammer_registry");
+    int applied = 0;
     for (const auto& [op, cl] : find_spans(
              katoms, begin_klammer_apply, end_klammer_apply, true, command_name)) {
         auto [begin, end] = find_span_katoms(katoms, op, cl);
@@ -574,7 +656,9 @@ void Machine::apply(
             klammer_name_from_katom(begin->m_text, begin->m_loc),
             target, begin->m_loc);
         apply_klammer_registry(klammer_registry, katoms, target, begin, end);
+        ++applied;
     }
+    return applied;
 }
 
 std::string Machine::run_phase_functions()
@@ -622,7 +706,6 @@ void Machine::escape_target_characters(const Target& target, katom_list& katoms)
 std::string Machine::apply(const std::string& target_name, bool final_processing, bool escape_characters)
 {
     (void)K::log(3, "top level");
-    int recursive_limit = 5;
     m_state.set("K_target", target_name);
     m_state.subst(m_katoms.begin(), m_katoms.end());
 
@@ -634,20 +717,26 @@ std::string Machine::apply(const std::string& target_name, bool final_processing
     if (escape_characters)
         escape_target_characters(target, m_katoms);
 
+    // Reduce to a fixed point.  A pass reports how many klammers it applied;
+    // the loop ends when a pass applies none.  (It formerly ended when the
+    // katom list stopped GROWING, which is not the same thing: a klammer whose
+    // body expands to nothing is a reduction that adds no katoms.)  Exceeding
+    // the round limit is now an error rather than a message followed by
+    // rendering the unreduced document -- silently emitting a document with
+    // live klammers still in it is worse than not emitting one.  Runaway
+    // recursion is caught earlier and more precisely by the depth guard in
+    // apply_klammer(); this limit only bounds the number of ROUNDS, which is
+    // the length of a chain of klammers that generate further klammers.
     int apply_count = 0;
-    auto katom_size = m_katoms.size();
-    while (true) {
-        apply(m_klammers, m_katoms, target_name);
-
-        if (m_katoms.size() == katom_size) {
-            break;
+    while (apply(m_klammers, m_katoms, target_name) > 0) {
+        if (++apply_count > apply_round_limit) {
+            std::stringstream ss {};
+            ss << "Klammer application did not reach a fixed point after "
+               << apply_round_limit << " rounds.\n"
+               << "Each round applies every klammer present; a klammer whose "
+               << "result contains further klammers starts another round.";
+            throw Recursion_error(ss.str(), Locator(), false);
         }
-        if (++apply_count > recursive_limit) {
-            msg() << red << "Error: Recursive limit ("
-                  << recursive_limit << ") reached\n" << black;
-            break;
-        }
-        katom_size = m_katoms.size();
     }
     m_result = to_string(m_katoms.begin(), m_katoms.end());
 

mac/machine.h

@@ -79,7 +79,10 @@ public:
     void apply_klammer_registry(Klammer_registry& klammer_registry,
                katom_list& katoms, const std::string& target, katom_iter begin, katom_iter end);
 
-    void apply(Klammer_registry& klammer_registry, katom_list& katoms, const std::string& target);
+    // Returns the number of klammers applied in this pass.  The top-level
+    // fixed point loops while that count is nonzero: reduction is detected by
+    // a redex having been reduced, not by the katom list having grown.
+    int apply(Klammer_registry& klammer_registry, katom_list& katoms, const std::string& target);
 
     std::string run_phase_functions();
     std::string apply(const std::string& target_name, bool final_processing=true, bool escape_characters=true);