From 4306dcd4901d60667ee52378849070781a0ddea7 Mon Sep 17 00:00:00 2001 From: Andy Kopra Date: Sat, 1 Aug 2026 15:41:43 +0200 Subject: [PATCH] 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) --- README.md | 2 +- com/ktext.cpp | 19 ++- mac/Makefile | 2 +- mac/check.cpp | 298 ++++++++++++++++++++++++++++++++++++++++++ mac/check.h | 65 +++++++++ mac/error.h | 11 ++ mac/machine.cpp | 117 +++++++++++++++-- mac/machine.h | 5 +- tst/Makefile | 16 +-- tst/check_test.sh | 204 +++++++++++++++++++++++++++++ tst/recursion_test.sh | 139 ++++++++++++++++++++ 11 files changed, 849 insertions(+), 29 deletions(-) create mode 100644 mac/check.cpp create mode 100644 mac/check.h create mode 100755 tst/check_test.sh create mode 100755 tst/recursion_test.sh diff --git a/README.md b/README.md index 4074586..4e11966 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ are regenerated on each release — patches cannot be merged directly. Report problems (or send patches) to the author; accepted changes are applied to the development tree and appear in a following snapshot. -This snapshot was assembled from development commit `3b91484c1049`. +This snapshot was assembled from development commit `c27e63802406`. ## License diff --git a/com/ktext.cpp b/com/ktext.cpp index 62e81bd..1b5316a 100644 --- a/com/ktext.cpp +++ b/com/ktext.cpp @@ -1,5 +1,6 @@ #include #include "error.h" +#include "check.h" #include "command.h" #include "log.h" #include "argv.h" @@ -25,6 +26,9 @@ int main(int argc, char* argv[]) "pathname", "", "'word'"); args.flag("d", "Display the output to the screen, rather than writing files."); args.flag("m", "Show the Klammermachine state at the beginning of processing."); + args.flag("check", "Check every klammer application in the input and in the " + "body of every defined klammer, report all problems found, and exit " + "without producing output."); args.opt("v", "'verbosity'", "degree", "0", "'verbosity'"); if (show_usage(argc, argv)) { @@ -52,8 +56,13 @@ int main(int argc, char* argv[]) parse_args(input_filenames, args.as_string("t"), expand_tilde(args.as_string("o")), args.as_bool("d")); + // --check produces no output, so it needs neither a target nor -d. + // With no -t it checks every defined target, which is the more useful + // default here: "does this document hold together at all?" + bool check_only = args.as_bool("check"); + if (*(output_filename.end() - 1) == '*' - && !display_only) { + && !display_only && !check_only) { throw Argument_error( "You must specify an output target or " "display the results with the \"-d\" flag.", @@ -106,6 +115,14 @@ int main(int argc, char* argv[]) std::cout << M << "\n"; } + // --check reports statically, before anything is applied: all problems + // at once, including ones in @cond branches that are not selected and + // in klammer bodies that this render would never reach. + if (check_only) { + int errors = report_diagnostics(check_machine(M, target), std::cout); + return errors > 0 ? 1 : 0; + } + std::string result = trim(M.apply(target)); if (display_only && !result.empty()) { std::cout << result << "\n"; diff --git a/mac/Makefile b/mac/Makefile index 7c93fa4..a40321d 100644 --- a/mac/Makefile +++ b/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)) diff --git a/mac/check.cpp b/mac/check.cpp new file mode 100644 index 0000000..1bafc5f --- /dev/null +++ b/mac/check.cpp @@ -0,0 +1,298 @@ +#include +#include +#include +#include + +#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 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 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& 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& 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(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 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 check_machine(Machine& machine, const std::string& target) +{ + std::vector 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 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& 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; +} diff --git a/mac/check.h b/mac/check.h new file mode 100644 index 0000000..3aa9890 --- /dev/null +++ b/mac/check.h @@ -0,0 +1,65 @@ +#pragma once + +#include +#include +#include + +#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 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& diagnostics, std::ostream& os); diff --git a/mac/error.h b/mac/error.h index f15cf6c..c90bed3 100644 --- a/mac/error.h +++ b/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( diff --git a/mac/machine.cpp b/mac/machine.cpp index 7beb73c..478dc39 100644 --- a/mac/machine.cpp +++ b/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()); diff --git a/mac/machine.h b/mac/machine.h index e26d8ad..3b994e1 100644 --- a/mac/machine.h +++ b/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); diff --git a/tst/Makefile b/tst/Makefile index 1063458..ce284ce 100644 --- a/tst/Makefile +++ b/tst/Makefile @@ -1,22 +1,16 @@ # Klammertext distribution test suite (subset). # -# Runs the eight shell regression suites: -# cond_test.sh — @cond argument delimitation -# deftype_test.sh — the four klammer definition modes + redefinition table -# escape_test.sh — target character escaping and quoted specials -# filename_test.sh — filenames with spaces (quoting, " / " lists, rescue) -# alone_test.sh — an optional argument's three values (default, the -# argument type's :alone value, a written value) -# modulepath_test.sh — @eval finds modules beside the file that names them -# klammerset_test.sh — the @@@klammerset system command and its search path -# editor_test.sh — editor support (doc/edit): indentation and table -# alignment; needs python3, uses Emacs when installed +# Runs the shell regression suites shipped with this snapshot (the list is +# generated from the distribution manifest, so it cannot drift from the files +# actually shipped). Each suite's own header comment says what it covers. # # Requires KLAMMERTEXT_HOME set and `ktext` on PATH (build it with `make -C com`). .PHONY: test test: ./cond_test.sh + ./recursion_test.sh + ./check_test.sh ./deftype_test.sh ./escape_test.sh ./filename_test.sh diff --git a/tst/check_test.sh b/tst/check_test.sh new file mode 100755 index 0000000..c4f7725 --- /dev/null +++ b/tst/check_test.sh @@ -0,0 +1,204 @@ +#!/bin/bash +# +# check_test.sh — Static checking of klammer applications ("ktext --check"). +# +# The engine applies klammers as it meets them, so it can only complain about +# what it reaches. Two things it therefore never reaches: +# +# * the branch of a @cond that is not selected. @cond is a non-strict +# special form, so an undefined klammer or a wrong argument count sitting +# in the unselected branch is invisible -- and stays invisible until the +# day the predicate flips. +# * a klammer body that this render does not use, including a body defined +# for a target other than the one being built. +# +# Klammertext can check both without running anything, because katom structure +# is fixed when a file is read: there are no catcodes, so nothing later can +# change how text already read divides into katoms. Every application written +# literally in a document or a klammer body can be located and checked against +# the registry. That is what check_machine() (mac/check.cpp) does and what +# these tests pin down. +# +# What the checker deliberately does not see is also tested: klammers produced +# by @eval at run time, and the interiors of @eval/@read argument spans and of +# literal parameters, which are code, filenames, and raw text -- not +# applications. +# +# Engine tier: no klammer set (-k none), every klammer defined inline. +# +# Usage: ./check_test.sh +# Exit code: 0 if all tests pass, 1 otherwise. + +PASS=0 +FAIL=0 +KTEXT=ktext +K=${KLAMMERTEXT_HOME:?KLAMMERTEXT_HOME must be set} + +red=$'\033[31m' +green=$'\033[32m' +bold=$'\033[1m' +reset=$'\033[0m' + +# check_finds TEST_NAME PATTERN KTEXT_ARGS... +# --check must exit nonzero and report PATTERN. +check_finds() { + local test_name="$1" + local pattern="$2" + shift 2 + + local output status + output=$("$KTEXT" --check "$@" 2>&1) + status=$? + + if [ $status -eq 0 ]; then + echo "${red}FAIL${reset} $test_name — expected a diagnostic, got none" + FAIL=$((FAIL + 1)) + return + fi + if echo "$output" | grep -qF "$pattern"; then + echo "${green}PASS${reset} $test_name" + PASS=$((PASS + 1)) + else + echo "${red}FAIL${reset} $test_name — expected report to contain [$pattern]" + echo " output: $(echo "$output" | head -4)" + FAIL=$((FAIL + 1)) + fi +} + +# check_clean TEST_NAME KTEXT_ARGS... +# --check must exit 0 and report no diagnostics. +check_clean() { + local test_name="$1" + shift + + local output status + output=$("$KTEXT" --check "$@" 2>&1) + status=$? + + if [ $status -eq 0 ] && echo "$output" | grep -q "0 diagnostics"; then + echo "${green}PASS${reset} $test_name" + PASS=$((PASS + 1)) + else + echo "${red}FAIL${reset} $test_name — expected a clean check" + echo " output: $(echo "$output" | head -4)" + FAIL=$((FAIL + 1)) + fi +} + +# check_count TEST_NAME N KTEXT_ARGS... +# --check must report exactly N diagnostics. +check_count() { + local test_name="$1" + local expected="$2" + shift 2 + + local output got + output=$("$KTEXT" --check "$@" 2>&1) + got=$(echo "$output" | sed -nE 's/^([0-9]+) diagnostics?,.*/\1/p') + + if [ "$got" = "$expected" ]; then + echo "${green}PASS${reset} $test_name" + PASS=$((PASS + 1)) + else + echo "${red}FAIL${reset} $test_name — expected $expected diagnostics, got ${got:-none}" + echo " output: $(echo "$output" | head -4)" + FAIL=$((FAIL + 1)) + fi +} + +GREET='@@greet name : Hello, *name*. @@' + +echo "${bold}Static klammer checking tests${reset}" +echo "=============================" +echo + +# --- What the renderer cannot reach --- + +check_finds " 1. undefined klammer in an unselected @cond branch (in a body)" \ + "@nosuch is not defined" \ + -k none -s "@@pick p : @cond *p* | @nosuch x @ | ok @ @@ @pick false @" + +check_finds " 2. wrong arity in an unselected @cond branch (in a body)" \ + "is given 3" \ + -k none -s "$GREET @@pick p : @cond *p* | @greet a | b | c @ | ok @ @@ @pick false @" + +# A @cond written at the top level of a DOCUMENT is resolved when the file is +# read, so its unselected branch is gone before anything can be checked. This +# test records that limitation rather than asserting the behavior is right; see +# notes/Klammertext_improvements.md, "When @cond is resolved". +check_clean " 2a. LIMITATION: a top-level @cond branch is resolved before checking" \ + -k none -s "@cond false | @nosuch x @ | ok @" + +check_finds " 3. undefined klammer in a body that is never applied" \ + "@nosuch is not defined" \ + -k none -s "@@unused : @nosuch x @ @@ nothing applies it" + +check_finds " 4. the body it was found in is named" \ + "in body of @unused" \ + -k none -s "@@unused : @nosuch x @ @@ nothing applies it" + +# --- Arity --- + +check_finds " 5. too few positional arguments" \ + "needs 2 positional arguments but is given 1" \ + -k none -s '@@pair a | b : *a**b* @@ @pair x @' + +check_finds " 6. too many positional arguments" \ + "takes 1 positional argument but is given 2" \ + -k none -s "$GREET @greet a | b @" + +check_finds " 7. undefined optional argument" \ + 'has no optional argument ":nope"' \ + -k none -s "$GREET @greet a :nope 1 @" + +check_finds " 8. the accepted optional arguments are listed" \ + "It accepts: :loud" \ + -k none -s '@@greet name :loud : *name* @@ @greet a :nope 1 @' + +check_clean " 9. a rest argument accepts extra positional arguments" \ + -k none -s '@@many a | rest.rest : *a* @@ @many x | y | z @' + +# --- Nesting. A bar or an option name belonging to a nested klammer is not +# this klammer's; the checker counts at depth 0, as @cond does. --- + +check_clean "10. nested klammer's bars are not counted as the outer's" \ + -k none -s '@@frac a | b : *a*/*b* @@ @@one x : [*x*] @@ @one @frac 1 | 2 @ @' + +check_clean "11. nested klammer's option name is not counted as the outer's" \ + -k none -s '@@inner a :flag : *a* @@ @@outer x : [*x*] @@ @outer @inner q :flag y @ @' + +# --- Target coverage --- + +check_finds "12. klammer not defined for a target" \ + 'is not defined for the target "tex"' \ + -k none -s '@@@target html | HTML output @@@ @@@target tex | TeX output @@@ @@only.html : H @@ @only@' + +check_clean "13. defined for every target is clean" \ + -k none -s '@@@target html | HTML output @@@ @@@target tex | TeX output @@@ @@both : B @@ @both@' + +# --- What the checker deliberately does not see --- + +# In a body, so the @eval is not evaluated at read time: what is being tested +# is that the checker does not read the eval's ARGUMENT as an application. +check_clean "14. @eval argument content is code, not applications" \ + -k none -s '@@w : @eval len("@nosuch") @ @@' + +check_clean "15. a literal parameter's content is raw text" \ + -k none -s '@@lit t.literal : *t* @@ @lit @nosuch x @ lit@' + +# --- Reporting --- + +check_count "16. a target-independent fault is reported once, not per target" \ + 1 \ + -k none -s '@@@target html | HTML output @@@ @@@target tex | TeX output @@@ @@g : @nosuch@ @@' + +check_clean "17. a correct document checks clean" \ + -k none -s "$GREET @greet World @" + +check_clean "18. the Standard Klammer Set checks clean" \ + -s 'x' + +echo +echo "=============================" +echo "Results: ${green}$PASS passed${reset}, ${red}$FAIL failed${reset}" +[ $FAIL -eq 0 ] diff --git a/tst/recursion_test.sh b/tst/recursion_test.sh new file mode 100755 index 0000000..162e2a1 --- /dev/null +++ b/tst/recursion_test.sh @@ -0,0 +1,139 @@ +#!/bin/bash +# +# recursion_test.sh — The klammer application recursion guard. +# +# Before the guard, a klammer that applied itself -- directly or through a +# cycle -- descended until the C++ stack was exhausted. The process died with +# SIGSEGV: no message, no location, no indication of which klammer was at +# fault, and a core dump. For a language whose premise is that users define +# their own klammers, that was the worst available failure mode. +# +# Machine::apply_klammer() now carries a depth guard (Depth_guard in +# mac/machine.cpp) that raises a Recursion_error naming the klammer and its +# location. Separately, the top-level fixed-point loop in Machine::apply() +# ends when a pass applies no klammer -- rather than when the katom list stops +# growing -- and exceeding its round limit is an error rather than a message +# followed by rendering a document with live klammers still in it. +# +# These are engine tests: no klammer set is loaded (-k none) and every klammer +# used is defined inline as a fixture. +# +# Usage: ./recursion_test.sh +# Exit code: 0 if all tests pass, 1 otherwise. + +PASS=0 +FAIL=0 +KTEXT=ktext +K=${KLAMMERTEXT_HOME:?KLAMMERTEXT_HOME must be set} + +red=$'\033[31m' +green=$'\033[32m' +bold=$'\033[1m' +reset=$'\033[0m' + +# check_error TEST_NAME PATTERN KTEXT_ARGS... +# Runs ktext, expects a NONZERO exit status and PATTERN in the message. +# A signal death (exit >= 128) is called out separately: that is the exact +# regression this suite exists to prevent, and reporting it as "some error" +# would hide it. +check_error() { + local test_name="$1" + local pattern="$2" + shift 2 + + local output status + output=$("$KTEXT" "$@" 2>&1) + status=$? + + if [ $status -ge 128 ]; then + echo "${red}FAIL${reset} $test_name — ktext died from signal $((status - 128))" + FAIL=$((FAIL + 1)) + return + fi + if [ $status -eq 0 ]; then + echo "${red}FAIL${reset} $test_name — expected an error but ktext succeeded" + FAIL=$((FAIL + 1)) + return + fi + if echo "$output" | grep -qF "$pattern"; then + echo "${green}PASS${reset} $test_name" + PASS=$((PASS + 1)) + else + echo "${red}FAIL${reset} $test_name — expected error to contain [$pattern]" + echo " output: $(echo "$output" | head -4)" + FAIL=$((FAIL + 1)) + fi +} + +# check_eq TEST_NAME EXPECTED KTEXT_ARGS... +check_eq() { + local test_name="$1" + local expected="$2" + shift 2 + + local output status + output=$("$KTEXT" "$@" 2>/dev/null) + status=$? + output=$(printf '%s' "$output" | tr -d '\n' | sed -E 's/^[[:space:]]+//; s/[[:space:]]+$//') + + if [ $status -ne 0 ]; then + echo "${red}FAIL${reset} $test_name — ktext exited $status" + FAIL=$((FAIL + 1)) + return + fi + if [ "$output" = "$expected" ]; then + echo "${green}PASS${reset} $test_name" + PASS=$((PASS + 1)) + else + echo "${red}FAIL${reset} $test_name" + echo " expected: [$expected]" + echo " got: [$output]" + FAIL=$((FAIL + 1)) + fi +} + +echo "${bold}Klammer recursion guard tests${reset}" +echo "=============================" +echo + +# --- Non-termination is an error, not a crash --- + +check_error " 1. direct self-recursion is caught" \ + "does not terminate" \ + -k none -s '@@f : x @f@ @@ @f@' -d + +check_error " 2. the offending klammer is named" \ + 'applying "f"' \ + -k none -s '@@f : x @f@ @@ @f@' -d + +check_error " 3. mutual recursion is caught" \ + "does not terminate" \ + -k none -s '@@a : ( @b@ ) @@ @@b : [ @a@ ] @@ @a@' -d + +check_error " 4. self-recursion through an argument is caught" \ + "does not terminate" \ + -k none -s '@@w t : < *t* > @@ @@r : @w @r@ @ @@ @r@' -d + +# --- Terminating nesting is untouched --- + +check_eq " 5. deep but finite nesting still reduces" \ + "<<<<>>>>" \ + -k none -s '@@w t : <*t*> @@ @w @w @w @w @w x @ @ @ @ @' -d + +check_eq " 6. a chain of klammers generating klammers reduces" \ + "END" \ + -k none -s '@@k1 : @k2@ @@ @@k2 : @k3@ @@ @@k3 : @k4@ @@ @@k4 : @k5@ @@ @@k5 : @k6@ @@ @@k6 : @k7@ @@ @@k7 : @k8@ @@ @@k8 : END @@ @k1@' -d + +# --- The fixed point ends on "nothing was applied", not "nothing was added" --- +# +# A klammer whose body is empty reduces without adding katoms. Under the old +# size-comparison test such a klammer looked like no progress at all. + +check_eq " 7. a klammer with an empty body reduces" \ + "a b" \ + -k none -s '@@nothing : @@ a @nothing@ b' -d + +echo +echo "=============================" +echo "Results: ${green}$PASS passed${reset}, ${red}$FAIL failed${reset}" +[ $FAIL -eq 0 ]