Re: Texinfo command nesting and syntax checking: nested @ref

[Gavin Smith]

On Thu, Jan 19, 2023 at 04:52:07AM +0100, Patrice Dumas wrote:
> It is not completly clear to me that the performance cost would be lower
> than looking at the commands stack and checking that the nesting is
> valid, as there is, in general, no deep nesting, and the performance
> penalty will probably be more important for your option when there are a
> lot of non-nested commands.  Your proposal, however, avoids a performance
> penalty increasing with nesting, as it would be simply proportional to
> the number of commands, I believe.  I do not think that we can know
> in advance which one is better, though, as it depends on the manual.
> 
> In any case, your proposal looks good to me, I can follow-up with
> changes in the pure perl parser if you want me to.

I have started work on this, incrementing a counter at the
@footnote open brace and decrement it at the closing brace (following
the places in the code where ct_brace_command is popped from the
context stack), but there are three different places in the code where
the context is popped at the closing brace.  It would be clearer if this
code could be unified.

I expect that this new code could take over the existing use of the
context brace context (for @caption, @shortcaption and @footnote),
so I suggest that we could add this new code first, then try to simplify
afterwards.

I have not committed the change below yet as this leads to a change in the
test results.  I believe it should be straightforward to implement this
in the pure Perl parser.  I would be happy to write this myself but I
may not have any more time to do this this weekend.

diff --git a/ChangeLog b/ChangeLog
index 61ada4eed1..2b16489992 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,24 @@
+2023-01-21  Gavin Smith  <gavinsmith0123@gmail.com>
+
+       Nesting context for XS parser
+
+       * tp/Texinfo/XS/parsetexi/context_stack.h (NESTING_CONTEXT):
+       Define new structure.
+       * tp/Texinfo/XS/parsetexi/context_stack.c (nesting_context): Define.
+       * tp/Texinfo/XS/parsetexi/api.c (reset_parser_except_conf):
+       Wipe nesting_context.
+
+       * tp/Texinfo/XS/parsetexi/parser.c (check_valid_nesting_context):
+       New function to check nesting_context.
+       (process_remaining_on_line) <@-command>:
+       Call it together with check_valid_nesting.
+
+       * tp/Texinfo/XS/parsetexi/separator.c (handle_open_brace)
+       Increment nesting_context.footnote for @footnote.
+       * tp/Texinfo/XS/parsetexi/close.c (close_current),
+       * tp/Texinfo/XS/parsetexi/separator.c (handle_close_brace),
+       Decrement nesting_context.footnote for @footnote.
+
 2023-01-19  Gavin Smith  <gavinsmith0123@gmail.com>
 
        * tp/Texinfo/XS/parsetexi/commands.h: Remove unused flags
diff --git a/tp/Texinfo/XS/parsetexi/api.c b/tp/Texinfo/XS/parsetexi/api.c
index 7210b8c5e9..9e69040c78 100644
--- a/tp/Texinfo/XS/parsetexi/api.c
+++ b/tp/Texinfo/XS/parsetexi/api.c
@@ -129,6 +129,7 @@ reset_parser_except_conf (void)
   wipe_errors ();
   reset_context_stack ();
   reset_region_stack ();
+  memset (&nesting_context, 0, sizeof (nesting_context));
   reset_floats ();
   wipe_global_info ();
   set_input_encoding ("utf-8");
diff --git a/tp/Texinfo/XS/parsetexi/close.c b/tp/Texinfo/XS/parsetexi/close.c
index 141fe2672a..3dff0e03e5 100644
--- a/tp/Texinfo/XS/parsetexi/close.c
+++ b/tp/Texinfo/XS/parsetexi/close.c
@@ -281,6 +281,9 @@ close_current (ELEMENT *current,
               else if (pop_context () != ct_brace_command)
                 fatal ("context brace command context expected");
             }
+
+          if (current->cmd == CM_footnote)
+            nesting_context.footnote--;
           current = close_brace_command (current, closed_block_command,
                                          interrupting_command);
         }
diff --git a/tp/Texinfo/XS/parsetexi/context_stack.c 
b/tp/Texinfo/XS/parsetexi/context_stack.c
index 0ecd1dec6e..e05a9b5829 100644
--- a/tp/Texinfo/XS/parsetexi/context_stack.c
+++ b/tp/Texinfo/XS/parsetexi/context_stack.c
@@ -140,6 +140,15 @@ current_region (void)
   return region_stack[region_top - 1];
 }
 
+
+
+
+
+/* Command nesting context. */
+
+NESTING_CONTEXT nesting_context;
+
+
 
 /* used for @kbd */
 int
diff --git a/tp/Texinfo/XS/parsetexi/context_stack.h 
b/tp/Texinfo/XS/parsetexi/context_stack.h
index 3d8444e657..321e79d03f 100644
--- a/tp/Texinfo/XS/parsetexi/context_stack.h
+++ b/tp/Texinfo/XS/parsetexi/context_stack.h
@@ -51,6 +51,15 @@ enum command_id current_region_cmd (void);
 
 void reset_region_stack (void);
 
+
+
+/* Used to check indirect nesting, e.g. @footnote{@emph{@footnote{...}}} */
+typedef struct {
+    int footnote;
+} NESTING_CONTEXT;
+
+extern NESTING_CONTEXT nesting_context;
+
 
 int in_preformatted_context_not_menu(void);
 #endif
diff --git a/tp/Texinfo/XS/parsetexi/parser.c b/tp/Texinfo/XS/parsetexi/parser.c
index bdb8d78904..ef26491e9f 100644
--- a/tp/Texinfo/XS/parsetexi/parser.c
+++ b/tp/Texinfo/XS/parsetexi/parser.c
@@ -1138,6 +1138,23 @@ check_valid_nesting (ELEMENT *current, enum command_id 
cmd)
     }
 }
 
+void
+check_valid_nesting_context (enum command_id cmd)
+{
+  enum command_id invalid_context = 0;
+  if (cmd == CM_footnote && nesting_context.footnote > 0)
+    {
+      invalid_context = CM_footnote;
+    }
+
+  if (invalid_context)
+    {
+      line_warn ("@%s should not appear anywhere inside @%s",
+        command_name(cmd),
+        command_name(invalid_context));
+    }
+}
+
 /* *LINEP is a pointer into the line being processed.  It is advanced past any
    bytes processed.
    Return STILL_MORE_TO_PROCESS when there is more to process on the line
@@ -1913,7 +1930,10 @@ value_invalid:
         }
 
       if (current->parent)
-        check_valid_nesting (current, cmd);
+        {
+          check_valid_nesting (current, cmd);
+          check_valid_nesting_context (cmd);
+        }
 
       if (def_line_continuation)
         {
diff --git a/tp/Texinfo/XS/parsetexi/separator.c 
b/tp/Texinfo/XS/parsetexi/separator.c
index d94e61ba4a..0aa3385556 100644
--- a/tp/Texinfo/XS/parsetexi/separator.c
+++ b/tp/Texinfo/XS/parsetexi/separator.c
@@ -99,7 +99,11 @@ handle_open_brace (ELEMENT *current, char **line_inout)
                     }
                 }
 #undef float
-        }
+            }
+          else if (command == CM_footnote)
+            {
+              nesting_context.footnote++;
+            }
 
           /* Add to context stack. */
           switch (command)
@@ -248,6 +252,8 @@ handle_close_brace (ELEMENT *current, char **line_inout)
             }
           else if (pop_context () != ct_brace_command)
             fatal ("context brace command context expected");
+          if (current->parent->cmd == CM_footnote)
+            nesting_context.footnote--;
         }
       /* determine if trailing spaces are ignored */
       else if (command_data(current->parent->cmd).data == BRACE_arguments)
@@ -526,6 +532,9 @@ handle_close_brace (ELEMENT *current, char **line_inout)
           closed_command = current->parent->cmd;
           counter_pop (&count_remaining_args);
 
+          if (closed_command == CM_footnote)
+            nesting_context.footnote--;
+
           register_global_command (current->parent);
           current = current->parent->parent;
           if (close_preformatted_command(closed_command))
diff --git a/tp/t/results/info_tests/nested_footnotes_separate.pl 
b/tp/t/results/info_tests/nested_footnotes_separate.pl
index 4450af8fef..e175959c53 100644
--- a/tp/t/results/info_tests/nested_footnotes_separate.pl
+++ b/tp/t/results/info_tests/nested_footnotes_separate.pl
@@ -200,7 +200,17 @@ $result_menus{'nested_footnotes_separate'} = {
   'info' => {}
 };
 
-$result_errors{'nested_footnotes_separate'} = [];
+$result_errors{'nested_footnotes_separate'} = [
+  {
+    'error_line' => 'warning: @footnote should not appear anywhere inside 
@footnote
+',
+    'file_name' => '',
+    'line_nr' => 6,
+    'macro' => '',
+    'text' => '@footnote should not appear anywhere inside @footnote',
+    'type' => 'warning'
+  }
+];
 
 
 $result_floats{'nested_footnotes_separate'} = {};