[PATCH 0/2] gas: .irp/.irpc handling

[PATCH 0/2] gas: .irp/.irpc handling

[Jan Beulich]

1: adjust handling of quotes for .irpc
2: extend \+ support to .irp / .irpc

See notes / questions in the individual patches.

Jan

[PATCH 1/2] gas: adjust handling of quotes for .irpc

[Jan Beulich]

The present handling of inner double quotes can lead to very strange
diagnostics. Follow one of the two possible interpretations of the doc:
@dots{} referring to possibly multiple white space separated
@var{values}, each of which may be quoted. The original implementation,
prior to 465e5617233f ("PR gas/3856"), hints at the other possible
interpretation: When quoted there's only a single @var{values}, with
inner quotes taken as ordinary characters. That, however, seems overall
less useful to me.

While touching the documentation, mirror the (inverse) spelling
correction (@section line inconsistent with actual description) to .irp
as well.
---
The other interpretation of the doc would also require adjustments:
in_quotes then would need to be left alone when encountering inner
quotes.

If quotes need to be possible to specify in a @var{values}, I think they
ought to be backslash-escaped, much like elsewhere when quoted entities
come into play. Adding support for that ought to be a separate change,
though.

--- a/gas/doc/as.texi
+++ b/gas/doc/as.texi
@@ -5859,7 +5859,7 @@ processing must also be performed upon t
 @end ifset
 
 @node Irp
-@section @code{.irp @var{symbol},@var{values}}@dots{}
+@section @code{.irp @var{symbol},@var{value}}@dots{}
 
 @cindex @code{irp} directive
 Evaluate a sequence of statements assigning different values to @var{symbol}.
@@ -5893,12 +5893,13 @@ For some caveats with the spelling of @v
 
 @cindex @code{irpc} directive
 Evaluate a sequence of statements assigning different values to @var{symbol}.
-The sequence of statements starts at the @code{.irpc} directive, and is
-terminated by an @code{.endr} directive.  For each character in @var{value},
-@var{symbol} is set to the character, and the sequence of statements is
-assembled.  If no @var{value} is listed, the sequence of statements is
-assembled once, with @var{symbol} set to the null string.  To refer to
-@var{symbol} within the sequence of statements, use @var{\symbol}.
+The sequence of statements starts at the line following the @code{.irpc}
+directive, and is terminated by an @code{.endr} directive.  For each character
+in each (possibly double quoted) @var{values}, @var{symbol} is set to the
+character, and the sequence of statements is assembled.  If no @var{values} is
+listed, the sequence of statements is assembled once, with @var{symbol} set to
+the null string.  To refer to @var{symbol} within the sequence of statements,
+use @var{\symbol}.
 
 For example, assembling
 
--- a/gas/macro.c
+++ b/gas/macro.c
@@ -1369,12 +1369,6 @@ expand_irp (int irpc, size_t idx, sb *in
     {
       bool in_quotes = false;
 
-      if (irpc && in->ptr[idx] == '"')
-	{
-	  in_quotes = true;
-	  ++idx;
-	}
-
       while (idx < in->len)
 	{
 	  if (!irpc)
@@ -1383,16 +1377,14 @@ expand_irp (int irpc, size_t idx, sb *in
 	    {
 	      if (in->ptr[idx] == '"')
 		{
-		  size_t nxt;
-
-		  if (irpc)
-		    in_quotes = ! in_quotes;
+		  in_quotes = ! in_quotes;
+		  ++idx;
 
-		  nxt = sb_skip_white (idx + 1, in);
-		  if (nxt >= in->len)
+		  if (! in_quotes)
 		    {
-		      idx = nxt;
-		      break;
+		      idx = sb_skip_white (idx, in);
+		      if (idx >= in->len)
+			break;
 		    }
 		}
 	      sb_reset (&f.actual);
--- /dev/null
+++ b/gas/testsuite/gas/macros/irpc-quote.l
@@ -0,0 +1,18 @@
+# This should match the output of gas irpc-quote.s.
+#...
+> <
+>a<
+>b<
+>c<
+>d<
+> <
+>e<
+>f<
+>1<
+>2<
+> <
+>3<
+>4<
+>5<
+>6<
+> <
--- /dev/null
+++ b/gas/testsuite/gas/macros/irpc-quote.s
@@ -0,0 +1,6 @@
+	.irpc c, " ab" cd " ef"
+	.print ">\c<"
+	.endr
+	.irpc c, "12 " 34 "56 "
+	.print ">\c<"
+	.endr
--- a/gas/testsuite/gas/macros/macros.exp
+++ b/gas/testsuite/gas/macros/macros.exp
@@ -109,3 +109,5 @@ run_list_test count
 # AIX targets need an extended regexp to match "\+".
 setup_xfail "avr-*-*" "cris*-*-*" "msp430-*-*" "z80-*-*" "*-*-aix*"
 run_list_test irp-count
+
+run_list_test irpc-quote

[PATCH 2/2] gas: extend \+ support to .irp / .irpc

[Jan Beulich]

These are effectively macro-like, without any separate macro definition.
They already support \@, so they would better also support \+. This
allows, where desired, to get away without maintaining an explicit count
variable in source code.

With this the recently introduced testcase doesn't need any xfails
anymore.
---
Perhaps .rep/.rept would benefit even more, but how to (tidily) support
the expansion there isn't quite clear to me, yet. While there is support
for some kind of expansion, that doesn't quite fit here: We can't just
strstr() for "\+", but would need to take into account earlier
backslashes as well, to properly deal with uses in e.g. nested .rept.
It's also not clear to me whether MIPS'es re-use of s_rept() should then
also do the expansion, or whether that ought to remain unchanged in
behavior.

Unlike for macros, the counter is incremented regardless of errors. I
think that's better (and macros may want switching to that, too), in
particular when conidering the effect on listings: For macros, as it
stands, there may be multiple (partial) macro expansions with the same
value, which may end up misleading.

--- a/gas/NEWS
+++ b/gas/NEWS
@@ -3,9 +3,10 @@
 * In x86 Intel syntax undue mnemonic suffixes are now warned about.  This is
   a first step towards rejecting their use where unjustified.
 
-* Assembler macros can now use the syntax \+ to access the number of times a
-  given macro has been executed.  This is similar to the already existing \@
-  syntax, except that the count is maintained on a per-macro basis.
+* Assembler macros as well as the bodies of .irp / .irpc can now use the
+  syntax \+ to access the number of times a given macro has been executed.
+  This is similar to the already existing \@ syntax, except that the count is
+  maintained on a per-macro basis.
   
 * Support the NF feature in Intel APX.
 
--- a/gas/macro.c
+++ b/gas/macro.c
@@ -798,7 +798,8 @@ sub_actual (size_t start, sb *in, sb *t,
 
 static const char *
 macro_expand_body (sb *in, sb *out, formal_entry *formals,
-		   struct htab *formal_hash, const macro_entry *macro)
+		   struct htab *formal_hash, const macro_entry *macro,
+		   unsigned int instance)
 {
   sb t;
   size_t src = 0;
@@ -854,13 +855,13 @@ macro_expand_body (sb *in, sb *out, form
 	      sprintf (buffer, "%u", macro_number);
 	      sb_add_string (out, buffer);
 	    }
-	  else if (macro && src < in->len && in->ptr[src] == '+')
+	  else if (src < in->len && in->ptr[src] == '+')
 	    {
 	      /* Sub in the current macro invocation number.  */
 
 	      char buffer[12];
 	      src++;
-	      sprintf (buffer, "%d", macro->count);
+	      sprintf (buffer, "%d", instance);
 	      sb_add_string (out, buffer);
 	    }
 	  else if (src < in->len && in->ptr[src] == '&')
@@ -1213,7 +1214,8 @@ macro_expand (size_t idx, sb *in, macro_
 	    }
 	}
 
-      err = macro_expand_body (&m->sub, out, m->formals, m->formal_hash, m);
+      err = macro_expand_body (&m->sub, out, m->formals, m->formal_hash, m,
+			       m->count);
     }
 
   /* Discard any unnamed formal arguments.  */
@@ -1363,11 +1365,12 @@ expand_irp (int irpc, size_t idx, sb *in
   if (idx >= in->len)
     {
       /* Expand once with a null string.  */
-      err = macro_expand_body (&sub, out, &f, h, 0);
+      err = macro_expand_body (&sub, out, &f, h, NULL, 0);
     }
   else
     {
       bool in_quotes = false;
+      unsigned int instance = 0;
 
       while (idx < in->len)
 	{
@@ -1392,7 +1395,8 @@ expand_irp (int irpc, size_t idx, sb *in
 	      ++idx;
 	    }
 
-	  err = macro_expand_body (&sub, out, &f, h, 0);
+	  err = macro_expand_body (&sub, out, &f, h, NULL, instance);
+	  ++instance;
 	  if (err != NULL)
 	    break;
 	  if (!irpc)
--- a/gas/testsuite/gas/macros/irp-count.d
+++ /dev/null
@@ -1,2 +0,0 @@
-#name: Macro counters inside IRP commands (irp-count.d)
-# Tests that \+ does not trip up IRP commands
--- a/gas/testsuite/gas/macros/irp-count.l
+++ b/gas/testsuite/gas/macros/irp-count.l
@@ -1,3 +1,7 @@
 #...
-\+
-\+
+a0
+a1
+b2
+x0
+x1
+y2
--- a/gas/testsuite/gas/macros/irp-count.s
+++ b/gas/testsuite/gas/macros/irp-count.s
@@ -1,7 +1,7 @@
-	.irp i,1
-	.print "\+"
+	.irp i,a,a,b
+	.print "\i\+"
 	.endr
 	
-	.irpc i,1
-	.print "\+"
+	.irpc i,xxy
+	.print "\i\+"
 	.endr
--- a/gas/testsuite/gas/macros/macros.exp
+++ b/gas/testsuite/gas/macros/macros.exp
@@ -103,11 +103,5 @@ gas_test_error "exit.s" "" ".exitm outsi
 
 run_list_test altmacro
 run_list_test count
-
-# The AVR, CRIS, MSP430 and Z80 targets define ONLY_STANDARD_ESCAPES,
-#  so \+ is rejected.
-# AIX targets need an extended regexp to match "\+".
-setup_xfail "avr-*-*" "cris*-*-*" "msp430-*-*" "z80-*-*" "*-*-aix*"
 run_list_test irp-count
-
 run_list_test irpc-quote