From: David Kastrup Date: Mon, 18 Aug 2014 16:17:05 +0000 (+0200) Subject: Issue 4070: Cyclic markup detection unable to deal with non-linear code execution X-Git-Tag: release/2.19.13-1~24 X-Git-Url: https://git.donarmstrong.com/?a=commitdiff_plain;h=dbb052ff31e25cbb21791363a58271f55d0c5be0;p=lilypond.git Issue 4070: Cyclic markup detection unable to deal with non-linear code execution This uses dynamic winding to keep the stack depth counts accurate even in the presence of non-local exits like those used of map-markup-commands. It also changes the regtest markup-cyclic-reference.ly since it seems too much trouble to trap both cyclic references via a tortoise/hare algorithm as well as limiting the maximum markup stack depth. --- diff --git a/input/regression/markup-cyclic-reference.ly b/input/regression/markup-cyclic-reference.ly index 9ca834b542..82bfe0641d 100644 --- a/input/regression/markup-cyclic-reference.ly +++ b/input/regression/markup-cyclic-reference.ly @@ -1,7 +1,7 @@ -\version "2.16.0" +\version "2.19.13" #(ly:set-option 'warning-as-error #f) -#(ly:expect-warning (ly:translate-cpp-warning-scheme "Cyclic markup detected: %s") 'cycle-markup) -#(ly:expect-warning (ly:translate-cpp-warning-scheme "Cyclic markup detected: %s") 'cycleI-markup) +#(ly:expect-warning (ly:translate-cpp-warning-scheme "Markup depth exceeds maximal value of %d; Markup: %s") 1024 'cycle-markup) +#(ly:expect-warning (ly:translate-cpp-warning-scheme "Markup depth exceeds maximal value of %d; Markup: %s") 1024 'cycleI-markup) \header { texidoc = "Cyclic markup definitions should cause a warning, but diff --git a/lily/text-interface.cc b/lily/text-interface.cc index 0dc4f36360..f45d06ac3b 100644 --- a/lily/text-interface.cc +++ b/lily/text-interface.cc @@ -95,6 +95,11 @@ Text_interface::interpret_string (SCM layout_smob, return fm->text_stencil (layout, str, is_music).smobbed_copy (); } +static size_t markup_depth = 0; + +void markup_up_depth (void *) { ++markup_depth; } +void markup_down_depth (void *) { --markup_depth; } + MAKE_SCHEME_CALLBACK_WITH_OPTARGS (Text_interface, interpret_markup, 3, 0, "Convert a text markup into a stencil." " Takes three arguments, @var{layout}, @var{props}, and @var{markup}.\n" @@ -114,28 +119,19 @@ Text_interface::interpret_markup (SCM layout_smob, SCM props, SCM markup) SCM func = scm_car (markup); SCM args = scm_cdr (markup); - /* Use a hare/tortoise algorithm to detect whether we are in a cycle, - * i.e. whether we have already encountered the same markup in the - * current branch of the markup tree structure. */ - static vector encountered_markups; - size_t depth = encountered_markups.size (); - if (depth > 0) - { - int slow = depth / 2; - if (ly_is_equal (encountered_markups[slow], markup)) - { - string name = ly_symbol2string (scm_procedure_name (func)); - // TODO: Also print the arguments of the markup! - non_fatal_error (_f ("Cyclic markup detected: %s", name)); - return Stencil ().smobbed_copy (); - } - } - /* Check for non-terminating markups, e.g. recursive calls with * changing arguments */ SCM opt_depth = ly_get_option (ly_symbol2scm ("max-markup-depth")); size_t max_depth = robust_scm2int (opt_depth, 1024); - if (depth > max_depth) + + // Don't use SCM_F_DYNWIND_REWINDABLE since it may be expensive + // without any obvious use for retaining continuations into + // markup expansion + scm_dynwind_begin ((scm_t_dynwind_flags)0); + // scm_dynwind_rewind_handler (markup_up_depth, 0, SCM_F_WIND_EXPLICITLY); + markup_up_depth (0); + scm_dynwind_unwind_handler (markup_down_depth, 0, SCM_F_WIND_EXPLICITLY); + if (markup_depth > max_depth) { string name = ly_symbol2string (scm_procedure_name (func)); // TODO: Also print the arguments of the markup! @@ -144,9 +140,8 @@ Text_interface::interpret_markup (SCM layout_smob, SCM props, SCM markup) return Stencil ().smobbed_copy (); } - encountered_markups.push_back (markup); SCM retval = scm_apply_2 (func, layout_smob, props, args); - encountered_markups.pop_back (); + scm_dynwind_end (); return retval; } else