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

[Gavin Smith]

On Sun, Jan 29, 2023 at 06:57:33PM +0000, Gavin Smith wrote:

> > > Perhaps we should also keep a stack of which commands incremented this
> > > counter so we can report the invalid nesting in an error message?
> > 
> > It is a necessity, in my opinion.
> 
> I've started work on this in the patch below.
> 
> When I have time, I will make the corresponding change to the XS code.
> Then it should be straightforward to extend this to all commands
> that should only contain basic inline content.

My current work is at the end of this message.

I have still have to work out a few discrepancies between the XS
and pure Perl results, but I don't have time to do this tonight.



diff --git a/ChangeLog b/ChangeLog
index 25a47eb166..9eeabd22e4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,33 @@
+2023-01-30  Gavin Smith <gavinsmith0123@gmail.com>
+
+       Nesting context basic inline stack
+
+       * tp/Texinfo/XS/parsetexi/context_stack.h
+       (COMMAND_STACK): New type.
+       * tp/Texinfo/XS/parsetexi/context_stack.c
+       (reset_command_stack, push_command, pop_command):
+       New functions to operate on COMMAND_STACK.
+       (reset_context_stack, push_context, pop_context)
+       (current_context_command):
+       Use COMMAND_STACK for the command part of the context stack.
+
+       * tp/Texinfo/XS/parsetexi/context_stack.h (NESTING_CONTEXT):
+       Replace xref counter with basic_inline_stack, which can be used
+       more generally for any command that should only contain basic
+       inline content.
+       * tp/Texinfo/XS/parsetexi/api.c (reset_parser_except_conf):
+       Call reset_command_stack on nesting_context.basic_inline_stack.
+       * tp/Texinfo/XS/parsetexi/separator.c
+       (handle_open_brace, handle_close_brace) <ref command>:
+       Call push_command and pop_command respectively on
+       nesting_context.basic_inline_stack
+
+       * tp/Texinfo/ParserNonXS.pm: Corresponding changes.
+       (parser): Initialise basic_inline_stack.
+       (_process_remaining_on_line) <ref commands>:
+       Push and pop basic_inline_stack.
+       (_check_valid_nesting_context): Check basic_inline_stack.
+
 2023-01-29  Patrice Dumas  <pertusus@free.fr>
 
        * tp/Texinfo/XS/parsetexi/macro.c (expand_macro_arguments): do not
diff --git a/tp/Texinfo/ParserNonXS.pm b/tp/Texinfo/ParserNonXS.pm
index 2198bdb491..2094e6b591 100644
--- a/tp/Texinfo/ParserNonXS.pm
+++ b/tp/Texinfo/ParserNonXS.pm
@@ -626,7 +626,6 @@ foreach my $no_paragraph_context ('math', 'preformatted', 
'rawpreformatted',
 my %nesting_context_init = (
                          'footnote' => 0,
                          'caption' => 0,
-                         'xref' => 0
 );
 
 # Interface and internal functions for input management
@@ -657,6 +656,7 @@ sub parser(;$$)
   # other initializations
   $parser->{'definfoenclose'} = {};
   $parser->{'nesting_context'} = {%nesting_context_init};
+  $parser->{'nesting_context'}->{'basic_inline_stack'} = [];
 
   # handle user provided state.
 
@@ -4323,11 +4323,11 @@ sub _check_valid_nesting_context
     $self->_line_warn(sprintf(
         __("\@%s should not appear anywhere inside caption"),
           $command), $source_info);
-  } elsif ($Texinfo::Commands::ref_commands{$command}
-         and $self->{'nesting_context'}->{'xref'}) {
-    $self->_line_warn(sprintf(
-        __("\@%s should not appear anywhere inside cross-reference"),
-          $command), $source_info);
+  } elsif (defined($self->{'nesting_context'}->{'basic_inline_stack'})
+             and @{$self->{'nesting_context'}->{'basic_inline_stack'}} > 0
+             and !$in_basic_inline_commands{$command}) {
+    $invalid_context
+      = $self->{'nesting_context'}->{'basic_inline_stack'}->[-1];
   }
   $self->_line_warn(sprintf(
         __("\@%s should not appear anywhere inside \@%s"),
@@ -5814,7 +5814,8 @@ sub _process_remaining_on_line($$$$)
           }
           $self->_push_context('ct_inlineraw', $command)
             if ($command eq 'inlineraw');
-          $self->{'nesting_context'}->{'xref'} += 1
+          push @{$self->{'nesting_context'}->{'basic_inline_stack'}},
+               $command
             if ($Texinfo::Commands::ref_commands{$command});
         }
         print STDERR "OPENED \@$current->{'parent'}->{'cmdname'}, remaining: "
@@ -5914,7 +5915,7 @@ sub _process_remaining_on_line($$$$)
           }
         } elsif ($ref_commands{$current->{'parent'}->{'cmdname'}}) {
           my $ref = $current->{'parent'};
-          $self->{'nesting_context'}->{'xref'} -= 1;
+          pop @{$self->{'nesting_context'}->{'basic_inline_stack'}};
           if (@{$ref->{'args'}}) {
             my @args;
             for $a (@{$ref->{'args'}}) {
diff --git a/tp/Texinfo/XS/parsetexi/api.c b/tp/Texinfo/XS/parsetexi/api.c
index 74b26dbde7..9d0dbf396d 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 ();
+  reset_command_stack (&nesting_context.basic_inline_stack);
   memset (&nesting_context, 0, sizeof (nesting_context));
   reset_floats ();
   wipe_global_info ();
diff --git a/tp/Texinfo/XS/parsetexi/context_stack.c 
b/tp/Texinfo/XS/parsetexi/context_stack.c
index e05a9b5829..175470da94 100644
--- a/tp/Texinfo/XS/parsetexi/context_stack.c
+++ b/tp/Texinfo/XS/parsetexi/context_stack.c
@@ -18,26 +18,72 @@
 
 #include "parser.h"
 
-static enum context *stack;
-static enum command_id *commands_stack;
+static enum context *context_stack;
 static size_t top; /* One above last pushed context. */
 static size_t space;
 
+/* Kept in sync with context_stack. */
+static COMMAND_STACK command_stack;
+
+/* Generic command stack functions */
+
+void
+reset_command_stack (COMMAND_STACK *stack)
+{
+  stack->top = 0;
+  stack->space = 0;
+  free (stack->stack);
+  stack->stack = 0;
+}
+
+void
+push_command (COMMAND_STACK *stack, enum command_id cmd)
+{
+  if (stack->top >= stack->space)
+    {
+      stack->stack
+        = realloc (stack->stack,
+                   (stack->space += 5) * sizeof (enum command_id));
+    }
+
+  stack->stack[stack->top] = cmd;
+  stack->top++;
+}
+
+enum command_id
+pop_command (COMMAND_STACK *stack)
+{
+  if (stack->top == 0)
+    fatal ("command stack empty");
+
+  return stack->stack[--stack->top];
+}
+
+enum command_id
+top_command (COMMAND_STACK *stack)
+{
+  if (stack->top == 0)
+    fatal ("command stack empty");
+
+  return stack->stack[stack->top - 1];
+}
+
+
+/* Context stacks */
+
 void
 reset_context_stack (void)
 {
   top = 0;
+  reset_command_stack (&command_stack);
 }
 
 void
 push_context (enum context c, enum command_id cmd)
 {
   if (top >= space)
-    {
-      stack = realloc (stack, (space += 5) * sizeof (enum context));
-      commands_stack
-        = realloc (commands_stack, (space += 5) * sizeof (enum command_id));
-    }
+    context_stack = realloc (context_stack,
+                             (space += 5) * sizeof (enum context));
 
   debug (">>>>>>>>>>>>>>>>>PUSHING STACK AT %d  -- %s @%s", top,
          c == ct_preformatted ? "preformatted"
@@ -45,9 +91,10 @@ push_context (enum context c, enum command_id cmd)
          : c == ct_def ? "def"
          : c == ct_brace_command ? "brace_command"
          : "", command_name(cmd));
-  stack[top] = c;
-  commands_stack[top] = cmd;
+  context_stack[top] = c;
   top++;
+
+  push_command (&command_stack, cmd);
 }
 
 enum context
@@ -56,8 +103,10 @@ pop_context ()
   if (top == 0)
     fatal ("context stack empty");
 
+  (void) pop_command (&command_stack);
+
   debug (">>>>>>>>>>>>>POPPING STACK AT %d", top - 1);
-  return stack[--top];
+  return context_stack[--top];
 }
 
 enum context
@@ -66,7 +115,7 @@ current_context (void)
   if (top == 0)
     return ct_NONE;
 
-  return stack[top - 1];
+  return context_stack[top - 1];
 }
 
 enum command_id
@@ -78,8 +127,8 @@ current_context_command (void)
     return CM_NONE;
   for (i = top -1; i >= 0; i--)
     {
-      if (commands_stack[i] != CM_NONE)
-        return commands_stack[i];
+      if (command_stack.stack[i] != CM_NONE)
+        return command_stack.stack[i];
     }
   return CM_NONE;
 }
@@ -162,10 +211,10 @@ in_preformatted_context_not_menu()
     {
       enum context ct;
       enum command_id cmd;
-      ct = stack[i];
+      ct = context_stack[i];
       if (ct != ct_line && ct != ct_preformatted)
         return 0;
-      cmd = commands_stack[i];
+      cmd = command_stack.stack[i];
       if (command_data(cmd).flags & CF_block
           && command_data(cmd).data != BLOCK_menu
           && ct == ct_preformatted)
diff --git a/tp/Texinfo/XS/parsetexi/context_stack.h 
b/tp/Texinfo/XS/parsetexi/context_stack.h
index 6bed3b23d6..2e382bf2b0 100644
--- a/tp/Texinfo/XS/parsetexi/context_stack.h
+++ b/tp/Texinfo/XS/parsetexi/context_stack.h
@@ -53,11 +53,23 @@ void reset_region_stack (void);
 
 
 
+typedef struct {
+  enum command_id *stack;
+  size_t top;   /* One above last pushed context. */
+  size_t space;
+} COMMAND_STACK;
+
+void reset_command_stack (COMMAND_STACK *stack);
+void push_command (COMMAND_STACK *stack, enum command_id cmd);
+enum command_id pop_command (COMMAND_STACK *stack);
+enum command_id top_command (COMMAND_STACK *stack);
+
+
 /* Used to check indirect nesting, e.g. @footnote{@emph{@footnote{...}}} */
 typedef struct {
     int footnote;
     int caption;
-    int xref;
+    COMMAND_STACK basic_inline_stack;
 } NESTING_CONTEXT;
 
 extern NESTING_CONTEXT nesting_context;
diff --git a/tp/Texinfo/XS/parsetexi/parser.c b/tp/Texinfo/XS/parsetexi/parser.c
index 0c369ffc9c..5cd43b039b 100644
--- a/tp/Texinfo/XS/parsetexi/parser.c
+++ b/tp/Texinfo/XS/parsetexi/parser.c
@@ -1153,11 +1153,21 @@ check_valid_nesting_context (enum command_id cmd)
       line_warn ("@%s should not appear anywhere inside caption",
         command_name(cmd));
     }
-  else if ((command_data(cmd).flags & CF_ref) && nesting_context.xref > 0)
+  else if (nesting_context.basic_inline_stack.top > 0)
     {
-      line_warn
-        ("@%s should not appear anywhere inside cross-reference",
-         command_name(cmd));
+      unsigned long flags = command_data(cmd).flags;
+      if (!((flags & (CF_brace | CF_nobrace)
+            || cmd == CM_c || cmd == CM_comment))
+          || (flags & CF_ref)
+          || cmd == CM_titlefont
+          || cmd == CM_anchor
+          || cmd == CM_footnote
+          || cmd == CM_verb)
+        invalid_context = top_command (&nesting_context.basic_inline_stack);
+
+      /* FIXME: This may not match exactly the definition of a basic inline
+         command in check_valid_nesting.  We should only need to
+         define what these commands are in one place. */
     }
 
   if (invalid_context)
diff --git a/tp/Texinfo/XS/parsetexi/separator.c 
b/tp/Texinfo/XS/parsetexi/separator.c
index 30d2a64f32..b48bdc351a 100644
--- a/tp/Texinfo/XS/parsetexi/separator.c
+++ b/tp/Texinfo/XS/parsetexi/separator.c
@@ -157,7 +157,7 @@ handle_open_brace (ELEMENT *current, char **line_inout)
             }
           if (command_data(command).flags & CF_ref)
             {
-              nesting_context.xref++;
+              push_command (&nesting_context.basic_inline_stack, command);
             }
         }
       debug ("OPENED");
@@ -285,7 +285,7 @@ handle_close_brace (ELEMENT *current, char **line_inout)
       else if (command_data(closed_command).flags & CF_ref)
         {
           ELEMENT *ref = current->parent;
-          nesting_context.xref--;
+          (void) pop_command (&nesting_context.basic_inline_stack);
           if (ref->args.number > 0)
             {
               if ((closed_command == CM_inforef