On Thu, May 22, 2025 at 07:21:17AM -0500, Eric Blake wrote:
> I still plan to see if m4p can run all of the examples in m4/checks;
> it's a bit tricky in places where the command line compatibility is
> not yet working.
$ cd m4-1.4/checks
$ ./check-them -I ../examples -m m4p [0-9][0-9][0-9].*
shows that you have a number of command-line compatibility issues. Right off
the bat:
001.preprocess shows you lack -s synclines
002.debugging_ and 003.command_li both fail because you aren't
allowing interleaving of options with input files (POSIX requires this
to work, making m4 one of the few programs where options can't be
blindly reshuffled to the front):
$ cat foo
bar
$ m4 -Dbar=hello foo -Dbar=world foo
hello
world
005.command_li is the first successful test. The next failure, in
021.macro_argu, actually highlights two issues:
$ ./check-them -I ../examples/ -m m4p 021.macro_argu
m4p 0.7.0
Checking 021.macro_argu
@ ../doc/m4.texi:1609: Origin of test
021.macro_argu: stderr mismatch
--- m4-tmp.2211840/m4-xerr 2025-05-22 08:11:58.521794160 -0500
+++ m4-tmp.2211840/m4-err 2025-05-22 08:11:58.489793857 -0500
@@ -1,2 +1 @@
-usage: m4p [-h] [-P] [-v] [-I DIRECTORY] [-D NAME[=VALUE]] [-U NAME] [-F
FILE]:stdin:1: Warning: too few arguments to builtin `index'
-usage: m4p [-h] [-P] [-v] [-I DIRECTORY] [-D NAME[=VALUE]] [-U NAME] [-F
FILE]:stdin:3: Warning: excess arguments to builtin `index' ignored
+m4:stdin:1: Warning: too few arguments to builtin `index'
Failed checks were:
021.macro_argu:err
First, the obvious issue of a missing warning:
$ echo 'index(a,1,)' | m4
m4:stdin:1: Warning: excess arguments to builtin `index' ignored
-1
$ echo 'index(a,1,)' | m4p
-1
Then, the more insidious issue: the check-them script was thoroughly
confused on what error messages it should expect. Stepping back, I
see that the check-them script is trying to assume that it can
correlate the output in 'm4 --help' with the prefix that will appear
in error messages:
m4name=`"$m4" --help | ${SED} -e 's/Usage: \(.*\) \[OPTION.*/\1/' \
-e 's/\\\\/\\\\\\\\/g' -e 1q`
And for the C version of m4, a symlink test proves the point:
$ ln -s /bin/m4 m4s
$ ./m4s --help | grep Usage
Usage: ./m4s [OPTION]... [FILE]...
$ echo 'eval()' | ./m4s
./m4s:stdin:1: empty string treated as 0 in builtin `eval'
0
Note how "./m4s" appeared in both places where "m4" used to appear.
But m4p currently does:
$ m4p --help | grep Usage
$ m4p --help | greap sage
usage: m4p [-h] [-P] [-v] [-I DIRECTORY] [-D NAME[=VALUE]] [-U NAME] [-F FILE]
$
which confuses check-them for several reasons: it is starting with
"usage:" rather than "Usage:" (GNU Coding Standards recommend the
upper-case spelling in this case), and the first option is [-h]
instead of [OPTIONS]. And even if I relax the sed statement to pick
out "m4p" instead of the empty string, there's STILL the matter that
m4p hard-codes "m4p" in --help but "m4:" in error messages (easy
enough to prove with another symlink test to see that it is not
honoring argv[0]). There's a reason that the GNU Coding Standards
recommend that error messages start with argv[0] instead of the
canonical program name: so that you can rename your binary and still
quickly determine which binary printed the message you saw. Yes, it
means that comparing stderr of two programs is harder (if they don't
share the same basename, you have to instead munge stderr - but that's
what check-them is doing).
048.undefine shows a weakness in m4p's tracing (in fact, it exists to
expose a use-after-free bug in 1.4.19 that was fixed in 1.4.20).
Simplifying that shows that debugmode(t) is not yet tracing everywhere
in m4p that it does in m4:
$ echo 'changequote([,])define(a,[popdef([a])])debugmode(t)a' | m4
m4trace: -1- a
m4trace: -1- popdef
$ echo 'changequote([,])define(a,[popdef([a])])debugmode(t)a' | m4p
$
054.defn shows a weakness in the use of defn on builtins in a spot
where a macro name is expected (define, indir, ...); here's a simpler
case:
$ echo 'define(,1)indir(defn(defn))' | m4
m4:stdin:1: Warning: indir: invalid macro name ignored
$ echo 'define(,1)indir(defn(defn))' | m4p
1
$
The failure in 095.dumpdef is because debugmode(q) isn't affecting
dumpdef yet (here, using -d to trigger debugmode(aeq):
$ echo 'changequote([,])define(a,1)dumpdef([a])dnl' | m4 -d
a: [1]
$ echo 'changequote([,])define(a,1)dumpdef([a])dnl' | m4p -d
a: 1
108.dnl shows a corner case in dnl vs m4wrap (the parser intentionally
ends any hunt for a newline at the end of first-level m4wrap text,
rather than letting it spill over into affecting the nested m4wrap);
several other m4wrap tests fail, so you must consider changecom, nested
quoting, and argument collection at the end of m4wrap as well:
$ m4 108.dnl
0 HI m4:108.dnl:10: Warning: end of file treated as newline
2 HI
$ m4p 108.dnl
0 HI $
114.changequot picks up oddities in trying to use potential macro
names as quote characters (POSIX says this is undefined, but if you
are aiming for m4 compatibility, you should consider accepting macro
names with higher precedence than quotes, even though you match m4 in
accepting comments with higher precedence than macro names).
$ echo 'define(hi,HI)changequote(q,Q)q hi Q' | m4
q HI Q
$ echo 'define(hi,HI)changequote(q,Q)q hi Q' | m4p
hi
118.changequot shows an oddity when the start- and end-quote
delimiters are identical (awkward in practice since you can't nest
quotes, but worth being compatible on - here, the nested invocation of
q() expands to ""aa"",""a"", so that the outer invocation of q() sees
an empty string, the expansion of aa, an empty string, a comma, an
empty string, a single a, and another empty string):
$ echo 'define(aa,A)define(q,"$@")changequote(",")q(q("aa","a"))' | m4
A,a
$ echo 'define(aa,A)define(q,"$@")changequote(",")q(q("aa","a"))' | m4p
m4:stdin:1: ERROR: end of file in string
156.undivert shows that undivert must treat the empty string the same
as 0, rather than a filename (no-op either way):
$ echo 'undivert()' | m4
$ echo 'undivert()' | m4p
m4:stdin:1: cannot undivert `': No such file or directory
170.regexp shows that you need to handle missing back-ref more
gracefully:
$ echo 'regexp(abc,b,\1)' | m4
m4:stdin:1: Warning: sub-expression 1 not present
$ echo 'regexp(abc,b,\1)' | m4p
Traceback (most recent call last):
File "/home/eblake/m4p/.venv/bin/m4p", line 8, in <module>
...
return regexp_replacement(arg_string, arg_replacement, p.span())
File "/home/eblake/m4p/m4p/parser.py", line 96, in regexp_replacement
first, last = span[d]
~~~~^^^
IndexError: list index out of range
and 171.regexp shows a missing warning on the implicit empty regex:
$ echo 'regexp(abc)' | m4
m4:stdin:1: Warning: too few arguments to builtin `regexp'
0
$ echo 'regexp(abc)' | m4p
0
183.format shows you don't handle %A:
$ echo 'format(%A,1)' | m4
0X1P+0
$ echo 'format(%A,1)' | m4p
Traceback (most recent call last):
File "/home/eblake/m4p/.venv/bin/m4p", line 8, in <module>
...
File "/home/eblake/m4p/m4p/parser.py", line 1179, in m4_format
out += m.group() % tuple(xs)
~~~~~~~~~~^~~~~~~~~~~
ValueError: unsupported format character 'A' (0x41) at index 1
You already know about various eval failures.
208.location shows some stress tests on the parser's notion of the
current line; a simple demo:
$ echo __file__ | m4
stdin
$ echo __file__ | m4
$ echo 'include(foo)' | m4 -Dbar=__file__ -I ../examples//
../examples/foo
$ echo 'include(foo)' | m4p -Dbar=__file__ -I ../examples//
../examples//foo
212.using_froz shows that frozen file support is still incomplete:
$ echo 'traceon(undefined)' | m4p -d -F /dev/null
Traceback (most recent call last):
File "/home/eblake/m4p/.venv/bin/m4p", line 8, in <module>
sys.exit(main())
...
File "msgpack/_packer.pyx", line 257, in msgpack._cmsgpack.Packer._pack_inner
TypeError: can not serialize 'DebugLevel' object
Not showing as a failure in check-them, but 146.diversions (creating a
large macro to output into diversions) highlights several performance
issues - what took 70ms on m4 instead 2m48s on m4p. In fact, I can
parameterize that test to show that your scaling is approaching
quadratic, just in defining the macro (not even involving diversions):
$ cat test.m4
ifdef(n,,`define(n,5)')define(t,.)dnl
define(scale,`ifelse($1,0,,`define(`t',defn(`t',`t'))$0(decr($1))')')dnl
scale(n)len(t)
$ /bin/time -f%E m4 -Dn=22 test.m4
4194304
0:00.13
$ /bin/time -f%E m4 -Dn=24 test.m4 # O(n): 4 times bigger is ~4 times longer
16777216
0:00.43
$ /bin/time -f%E m4p -Dn=17 test.m4
131072
0:02.04
$ /bin/time -f%E m4p -Dn=18 test.m4 # >3x: this is the breaking point
262144
0:06.53
$ /bin/time -f%E m4p -Dn=19 test.m4 # O(n^2): 2 times bigger is ~4 times longer
524288
0:24.48
When merely doubling the size of the string quadruples execution time,
you are doing something wrong with memory management. Small strings
(under 64k) aren't showing the penalties, probably because of default
buffer sizes and fewer reallocations going on.
Rule of thumb: any time you are appending to an arbitrarily growing
buffer, extending the buffer size in an arithmetic progression (if
every time you hit the buffer size, you add a constant amount like
64k) WILL scale O(n^2) (since each time you double in size, you have
doubled the number of reallocs to account for the larger sizing, and
each realloc is averaging double the number of bytes to move into the
new spot). Instead, grow the buffer size in a geometric progression
(every time you hit the buffer size, you double the allocation; it
even works if you just grow the allocation by 1.5x).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
