From aa7ccb1214c2ec454829a64d05a52ecc8f32aa01 Mon Sep 17 00:00:00 2001 From: Dan Liu Date: Sun, 30 Aug 2026 19:55:02 -0400 Subject: [PATCH 1/7] Add python/dump.py to dump and audit parser nodes Prints each node's attributed line range, source lines, printed form and (with -a) the AST. Audits attribution across nodes and exits nonzero on OVERLAP/GAP/EMPTY, which round_trip.sh cannot catch -- rt.py never looks at the line ranges. --ranges gives golden-comparable output; --which reports which libdash was imported, since dump.py falls back to the working tree. --- python/dump.py | 210 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 210 insertions(+) create mode 100755 python/dump.py diff --git a/python/dump.py b/python/dump.py new file mode 100755 index 0000000..8874ab0 --- /dev/null +++ b/python/dump.py @@ -0,0 +1,210 @@ +#!/usr/bin/env python3 +"""Dump the parser nodes libdash detects in a script. + +Per node: attributed line range, source lines, printed form, and the AST (-a). + +Also audits attribution across nodes, reporting OVERLAP (range runs into the +next node), GAP (lines belong to no node), EMPTY, INVERTED, OVERRUN and +UNCOVERED. Exits nonzero on any, so it doubles as a regression test. +""" + +import argparse +import os +import pprint +import sys + +try: + import libdash +except ModuleNotFoundError: + # fall back to the working tree, so libdash/*.py edits need no reinstall + sys.path.insert (0, os.path.dirname (os.path.dirname (os.path.abspath (__file__)))) + import libdash + +sys.setrecursionlimit (9001) + + +class Problem: + def __init__ (self, kind, index, detail): + self.kind = kind + self.index = index + self.detail = detail + + def __str__ (self): + return "{}: node {}: {}".format (self.kind, self.index, self.detail) + + +def read_lines (path): + with open (path, 'r') as fp: + return fp.readlines () + + +def collect (path, init): + """Parse `path`, returning (nodes, error) where nodes is a list of + (ast, lines, linno_before, linno_after).""" + nodes = [] + + try: + for node in libdash.parse (path, init): + nodes.append (node) + except Exception as e: + return (nodes, e) + + return (nodes, None) + + +def is_inert (line): + """A blank line or a whole-line comment yields no node, so it may legitimately + fall between two nodes.""" + stripped = line.strip () + return stripped == "" or stripped.startswith ("#") + + +def audit (nodes, lines): + """Check the line ranges the parser handed back for overlaps and gaps.""" + problems = [] + prev_after = 0 + + for (i, (_ast, _text, before, after)) in enumerate (nodes): + if after < before: + problems.append (Problem ("INVERTED", i, + "range [{}, {}) runs backwards".format (before, after))) + elif after == before: + problems.append (Problem ("EMPTY", i, + "range [{}, {}) covers no lines".format (before, after))) + + if before < prev_after: + problems.append (Problem ("OVERLAP", i, + "starts at line {} but the previous node ran through line {}" + .format (before, prev_after - 1))) + elif before > prev_after: + # Blank lines and comments produce no node, so only flag a gap that + # swallowed something the parser should have accounted for. + skipped = lines [prev_after:before] + + if not all (is_inert (line) for line in skipped): + problems.append (Problem ("GAP", i, + "lines {}..{} belong to no node: {}" + .format (prev_after + 1, before, + ", ".join (repr (l) for l in skipped + if not is_inert (l))))) + + prev_after = max (prev_after, after) + + if nodes and prev_after > len (lines): + problems.append (Problem ("OVERRUN", len (nodes) - 1, + "last node ends at line {} but the file has {} lines" + .format (prev_after, len (lines)))) + + trailing = lines [prev_after:] + + if nodes and not all (is_inert (line) for line in trailing): + problems.append (Problem ("UNCOVERED", len (nodes) - 1, + "lines {}..{} after the last node belong to no node: {}" + .format (prev_after + 1, len (lines), + ", ".join (repr (l) for l in trailing + if not is_inert (l))))) + + return problems + + +def show (path, nodes, err, problems, args): + lines = read_lines (path) + + print ("=== {} ({} lines) ===".format (path, len (lines))) + + for (i, (ast, text, before, after)) in enumerate (nodes): + print ("--- node {} lines [{}, {}) {}" + .format (i, before, after, + "src lines {}..{}".format (before + 1, after) if after > before + else "no source lines")) + + if text is None: + print (" src | ") + else: + for (n, line) in enumerate (text.splitlines ()): + print (" src | {:>4} | {}".format (before + n + 1, line)) + + for line in libdash.to_string (ast).splitlines (): + print (" print | {}".format (line)) + + if args.ast: + for line in pprint.pformat (ast, width = 100).splitlines (): + print (" ast | {}".format (line)) + + if err is not None: + print ("!!! {}: {}".format (type (err).__name__, err)) + + for p in problems: + print ("!!! {}".format (p)) + + print ("--- {} node(s), {} problem(s)".format (len (nodes), len (problems))) + print () + + +def show_ranges (path, nodes, err): + """One line per node, stable enough to diff against a golden file.""" + for (i, (_ast, text, before, after)) in enumerate (nodes): + print ("node {} [{}, {}) {!r}".format (i, before, after, text)) + + if err is not None: + print ("error {}: {}".format (type (err).__name__, err)) + + +def main (): + ap = argparse.ArgumentParser (description = __doc__, + formatter_class = argparse.RawDescriptionHelpFormatter) + ap.add_argument ("files", nargs = "*", help = "shell scripts to dump") + ap.add_argument ("-a", "--ast", action = "store_true", + help = "also dump the raw AST for each node") + ap.add_argument ("-q", "--quiet", action = "store_true", + help = "only show files with problems") + ap.add_argument ("-r", "--ranges", action = "store_true", + help = "terse one-line-per-node output, for golden-file comparison") + ap.add_argument ("-w", "--which", action = "store_true", + help = "print which libdash was imported, then exit") + args = ap.parse_args () + + if args.which: + print (libdash.__file__) + return 0 + + if not args.files: + ap.error ("no files given") + + init = True + bad = 0 + + for path in args.files: + try: + lines = read_lines (path) + except OSError as e: + print ("=== {}: cannot read: {}".format (path, e)) + bad += 1 + continue + + (nodes, err) = collect (path, init) + init = False + + if args.ranges: + show_ranges (path, nodes, err) + continue + + problems = audit (nodes, lines) + + if problems: + bad += 1 + + if problems or not args.quiet: + show (path, nodes, err, problems, args) + elif err is not None: + print ("=== {}: {}: {}".format (path, type (err).__name__, err)) + print () + + if bad: + print ("{} file(s) with line-attribution problems".format (bad)) + + return 1 if bad else 0 + + +if __name__ == "__main__": + sys.exit (main ()) From 08fde7126c2aff706eabf2a33ad3b5dfa5e9968e Mon Sep 17 00:00:00 2001 From: Dan Liu Date: Sun, 30 Aug 2026 19:55:23 -0400 Subject: [PATCH 2/7] Fix bug so next line isn't included in the line range `nleft` is bytes left in the read buffer, so `nleft_after != 0` just means "not at EOF". Adding a line whenever it holds makes every command but the last swallow the first line of its successor: 1231 overlapping ranges across 216 of 320 test files. No adjustment is needed here at all -- dash advances `linno` when it consumes the newline. The block began as an `assert nleft_after == 0` sanity check and should have been dropped when it started failing; why it started failing is a separate bug, fixed next. Solution: drop the `elif nleft_after != 0` block. round_trip.sh cannot catch this: rt.py passes only `ast` to to_string, so parsedLines/linno_before/linno_after never reach the comparison. Adds 8 fixtures under test/line_mapping/ with goldens -- simple, comments_and_blanks, semi_one_line, multiline_construct, nested, function, continuation, heredoc. All 8 fail against 0.5.0. --- libdash/parser.py | 3 - python/Makefile | 10 ++- test/README.md | 12 +++- test/line_mapping.sh | 61 +++++++++++++++++++ .../line_mapping/comments_and_blanks.expected | 2 + test/line_mapping/comments_and_blanks.sh | 7 +++ test/line_mapping/continuation.expected | 2 + test/line_mapping/continuation.sh | 4 ++ test/line_mapping/function.expected | 2 + test/line_mapping/function.sh | 4 ++ test/line_mapping/heredoc.expected | 2 + test/line_mapping/heredoc.sh | 5 ++ .../line_mapping/multiline_construct.expected | 2 + test/line_mapping/multiline_construct.sh | 5 ++ test/line_mapping/nested.expected | 2 + test/line_mapping/nested.sh | 5 ++ test/line_mapping/semi_one_line.expected | 2 + test/line_mapping/semi_one_line.sh | 2 + test/line_mapping/simple.expected | 3 + test/line_mapping/simple.sh | 3 + 20 files changed, 132 insertions(+), 6 deletions(-) create mode 100755 test/line_mapping.sh create mode 100644 test/line_mapping/comments_and_blanks.expected create mode 100644 test/line_mapping/comments_and_blanks.sh create mode 100644 test/line_mapping/continuation.expected create mode 100644 test/line_mapping/continuation.sh create mode 100644 test/line_mapping/function.expected create mode 100644 test/line_mapping/function.sh create mode 100644 test/line_mapping/heredoc.expected create mode 100644 test/line_mapping/heredoc.sh create mode 100644 test/line_mapping/multiline_construct.expected create mode 100644 test/line_mapping/multiline_construct.sh create mode 100644 test/line_mapping/nested.expected create mode 100644 test/line_mapping/nested.sh create mode 100644 test/line_mapping/semi_one_line.expected create mode 100644 test/line_mapping/semi_one_line.sh create mode 100644 test/line_mapping/simple.expected create mode 100644 test/line_mapping/simple.sh diff --git a/libdash/parser.py b/libdash/parser.py index 4db7808..ff236d8 100644 --- a/libdash/parser.py +++ b/libdash/parser.py @@ -83,9 +83,6 @@ def parse(inputPath, init=True): # Last line did not have a newline assert (len (lines [-1]) > 0 and (lines [-1][-1] != '\n')) - elif nleft_after != 0: - # we formerly asserted that `nleft_after != 0`, but this no longer holds - linno_after = linno_after + 1; # The last line wasn't counted n_ptr = cast (n_ptr_C, POINTER (union_node)) new_ast = of_node (n_ptr) diff --git a/python/Makefile b/python/Makefile index b446cce..b3233c7 100644 --- a/python/Makefile +++ b/python/Makefile @@ -1,9 +1,15 @@ -.PHONY: test clean +.PHONY: test test-roundtrip test-line-mapping clean -test: rt.py ../libdash/*.py +test: test-roundtrip test-line-mapping + +test-roundtrip: rt.py ../libdash/*.py @find ../test/tests ../test/pash_tests -type f | while read f; do ../test/round_trip.sh ./rt.py "$$f"; done | tee python.log @cat python.log | egrep '^[A-Z0-9_]+:' | cut -d ':' -f 1 | sort | uniq -c @grep ':' python.log && echo "FAILED" && exit 1 || exit 0 +# Per-node line mapping; round_trip.sh cannot see it (rt.py uses only the AST). +test-line-mapping: dump.py ../libdash/*.py + @../test/line_mapping.sh + clean: rm *.o *.so *.log diff --git a/test/README.md b/test/README.md index c3b7f08..f1ae577 100644 --- a/test/README.md +++ b/test/README.md @@ -1,7 +1,17 @@ -There are three directories of tests: +There are four directories of tests: - `tests` are the original libdash tests, mostly handwritten - `pash_tests` are shell scripts taken from [`pash`](https://github.com/binpash/pash) - `failing` are shell scripts that aren't working right now (which is probably a bug) + - `line_mapping` holds fixtures for the per-node source line mapping, each with a + `.expected` golden Both OCaml and Python bindings use the `round_trip.sh` to test round tripping. The `test_ocaml_python.sh` script compares the output from Python and OCaml. + +`round_trip.sh` checks that `print . parse` reaches a fixpoint. It cannot +compare against the original source, since the AST drops comments, whitespace +and quoting style -- which leaves the source line mapping (`parsedLines`, +`linno_before`, `linno_after`) uncovered, as `rt.py` discards it and prints only +the AST. `line_mapping.sh` covers that: it runs `python/dump.py --ranges` over +each fixture and diffs against the golden. `REGEN=1 ./line_mapping.sh` updates +the goldens after an intentional change. diff --git a/test/line_mapping.sh b/test/line_mapping.sh new file mode 100755 index 0000000..aea56ed --- /dev/null +++ b/test/line_mapping.sh @@ -0,0 +1,61 @@ +#!/bin/sh +# +# Checks the per-node source line mapping (parsedLines, linno_before, +# linno_after) -- invisible to round_trip.sh, which only sees the AST. +# +# Each fixture in line_mapping/ has a .expected golden, one line per node. +# REGEN=1 rewrites the goldens after an intentional change. + +: ${DUMP=../python/dump.py} + +cd "$(dirname "$0")" || exit 2 + +status=0 + +# dump.py falls back to the working tree, so a stale PYTHONPATH can swap the +# library under test; say which one we got. +echo "# libdash: $("$DUMP" --which 2>/dev/null || echo '')" + +for tgt in line_mapping/*.sh +do + expected="${tgt%.sh}.expected" + actual=$(mktemp) + + if ! "$DUMP" --ranges "$tgt" >"$actual" 2>&1 + then + echo "LINE_MAPPING_ABORT: '$tgt'" + cat "$actual" >&2 + rm -f "$actual" + status=1 + continue + fi + + if [ "$REGEN" ] + then + cp "$actual" "$expected" + echo "REGEN '$tgt'" + rm -f "$actual" + continue + fi + + if [ ! -f "$expected" ] + then + echo "LINE_MAPPING_NO_GOLDEN: '$tgt' (run REGEN=1 $0)" + rm -f "$actual" + status=1 + continue + fi + + if diff "$expected" "$actual" >/dev/null + then + echo "PASS '$tgt'" + else + echo "LINE_MAPPING_FAIL: '$tgt'" + diff -u "$expected" "$actual" + status=1 + fi + + rm -f "$actual" +done + +exit $status diff --git a/test/line_mapping/comments_and_blanks.expected b/test/line_mapping/comments_and_blanks.expected new file mode 100644 index 0000000..561ca7b --- /dev/null +++ b/test/line_mapping/comments_and_blanks.expected @@ -0,0 +1,2 @@ +node 0 [2, 3) 'echo first\n' +node 1 [5, 6) 'echo second\n' diff --git a/test/line_mapping/comments_and_blanks.sh b/test/line_mapping/comments_and_blanks.sh new file mode 100644 index 0000000..988ca95 --- /dev/null +++ b/test/line_mapping/comments_and_blanks.sh @@ -0,0 +1,7 @@ +# leading comment + +echo first + +# middle comment +echo second +# trailing comment diff --git a/test/line_mapping/continuation.expected b/test/line_mapping/continuation.expected new file mode 100644 index 0000000..53bd341 --- /dev/null +++ b/test/line_mapping/continuation.expected @@ -0,0 +1,2 @@ +node 0 [0, 3) 'echo one \\\n two \\\n three\n' +node 1 [3, 4) 'echo next\n' diff --git a/test/line_mapping/continuation.sh b/test/line_mapping/continuation.sh new file mode 100644 index 0000000..d74f2f4 --- /dev/null +++ b/test/line_mapping/continuation.sh @@ -0,0 +1,4 @@ +echo one \ + two \ + three +echo next diff --git a/test/line_mapping/function.expected b/test/line_mapping/function.expected new file mode 100644 index 0000000..394ef50 --- /dev/null +++ b/test/line_mapping/function.expected @@ -0,0 +1,2 @@ +node 0 [0, 3) 'greet() {\n echo hello\n}\n' +node 1 [3, 4) 'greet\n' diff --git a/test/line_mapping/function.sh b/test/line_mapping/function.sh new file mode 100644 index 0000000..a25a7f7 --- /dev/null +++ b/test/line_mapping/function.sh @@ -0,0 +1,4 @@ +greet() { + echo hello +} +greet diff --git a/test/line_mapping/heredoc.expected b/test/line_mapping/heredoc.expected new file mode 100644 index 0000000..d373214 --- /dev/null +++ b/test/line_mapping/heredoc.expected @@ -0,0 +1,2 @@ +node 0 [0, 4) 'cat < Date: Sun, 30 Aug 2026 19:55:38 -0400 Subject: [PATCH 3/7] Count a final line that has no terminating newline Separate bug, same origin: the bindings never followed dash. `EOF_NLEFT = -99` no longer exists -- preadbuffer sets `nleft = 0` at EOF (dash dcf4ee3, 2021) -- so the branch counting a final unterminated line was dead code. test/tests/escaping is one line long and parsed to [0, 0). 25 corpus files are affected, all lacking a trailing newline. That same dash change broke the assert removed in the previous commit. EOF now comes from `parsefile->eof & 2`, the sticky bit preadbuffer sets on PEOF and pungetc never clears. The fixup extends to len(lines) only when the last line really has no newline, so it cannot fire mid-file. Doing this in pure Python instead -- checking `linno_after == len(lines) - 1` -- is wrong: in a multi-command file with no final newline the second-to-last command also ends there and swallows the last line (test/pash_tests/expand-u.sh). Only a real EOF signal separates the cases. Reading that field needs the ctypes mirrors to match src/input.h again. dash gained `eof` and `spfree` and moved `lleft` after `spfree`; the `lastc[2]` arrays are long gone. Both structs kept their LP64 size, so nothing crashed -- fields were just silently misaligned: parsefile.lleft read parsefile.eof parsefile.lastc read parsefile.spfree parsefile.unget read parsefile.lleft strpush.lastc read strpush.spfree `linno`, `fd` and `nleft` precede the drift and were always correct. Adds 3 fixtures: no_trailing_newline, multiline_at_eof and multi_no_trailing_newline (the case the pure-Python shortcut gets wrong). --- libdash/_dash.py | 21 ++++++++++------ libdash/parser.py | 25 +++++++++++-------- .../multi_no_trailing_newline.expected | 3 +++ .../line_mapping/multi_no_trailing_newline.sh | 3 +++ test/line_mapping/multiline_at_eof.expected | 1 + test/line_mapping/multiline_at_eof.sh | 4 +++ .../line_mapping/no_trailing_newline.expected | 1 + test/line_mapping/no_trailing_newline.sh | 1 + 8 files changed, 40 insertions(+), 19 deletions(-) create mode 100644 test/line_mapping/multi_no_trailing_newline.expected create mode 100644 test/line_mapping/multi_no_trailing_newline.sh create mode 100644 test/line_mapping/multiline_at_eof.expected create mode 100644 test/line_mapping/multiline_at_eof.sh create mode 100644 test/line_mapping/no_trailing_newline.expected create mode 100644 test/line_mapping/no_trailing_newline.sh diff --git a/libdash/_dash.py b/libdash/_dash.py index bb15190..2261b50 100644 --- a/libdash/_dash.py +++ b/libdash/_dash.py @@ -223,8 +223,8 @@ class strpush (Structure): # struct alias *ap; /* if push was associated with an alias */ # char *string; /* remember the string since it may change */ # -# /* Remember last two characters for pungetc. */ -# int lastc[2]; +# /* Delay freeing so we can stop nested aliases. */ +# struct strpush *spfree; # # /* Number of outstanding calls to pungetc. */ # int unget; @@ -234,7 +234,7 @@ class strpush (Structure): ("prevnleft", c_int), ("ap", c_void_p), ("string", c_char_p), - ("lastc", 2 * c_int), + ("spfree", POINTER (strpush)), ("unget", c_int)]; class parsefile (Structure): @@ -245,14 +245,18 @@ class parsefile (Structure): # int linno; /* current line */ # int fd; /* file descriptor (or -1 if string) */ # int nleft; /* number of chars left in this line */ -# int lleft; /* number of chars left in this buffer */ +# int eof; /* do not read again once we hit EOF */ # char *nextc; /* next char in buffer */ # char *buf; /* input buffer */ # struct strpush *strpush; /* for pushing strings at this level */ # struct strpush basestrpush; /* so pushing one is fast */ # -# /* Remember last two characters for pungetc. */ -# int lastc[2]; +# /* Delay freeing so we can stop nested aliases. */ +# struct strpush *spfree; +# +# #ifndef SMALL +# int lleft; /* number of chars left in this buffer */ +# #endif # # /* Number of outstanding calls to pungetc. */ # int unget; @@ -261,12 +265,13 @@ class parsefile (Structure): ("linno", c_int), ("fd", c_int), ("nleft", c_int), - ("lleft", c_int), + ("eof", c_int), ("nextc", POINTER (c_char)), # NOT c_char_p! ("buf", c_char_p), ("strpush", POINTER (strpush)), ("basestrpush", strpush), - ("lastc", 2 * c_int), + ("spfree", POINTER (strpush)), + ("lleft", c_int), ("unget", c_int)]; diff --git a/libdash/parser.py b/libdash/parser.py index ff236d8..4fcd30e 100644 --- a/libdash/parser.py +++ b/libdash/parser.py @@ -16,7 +16,8 @@ def libdash_library_path(): LIBDASH_LIBRARY_PATH = os.path.join(FILE_PATH, "libdash.so") return LIBDASH_LIBRARY_PATH -EOF_NLEFT = -99; # libdash/src/input.c +# parsefile->eof bit 1: set by preadbuffer at EOF, never cleared by pungetc. +PARSEFILE_EOF = 2; class ParsingException(Exception): def __init__(self, message='ParseError'): @@ -64,7 +65,7 @@ def parse(inputPath, init=True): n_ptr_C = parsecmd_safe (libdash, False) linno_after = parsefile_var.contents.linno - 1; # libdash is 1-indexed - nleft_after = parsefile_var.contents.nleft + eof_after = parsefile_var.contents.eof if (n_ptr_C == None): # Dash.Null pass @@ -73,16 +74,18 @@ def parse(inputPath, init=True): elif (n_ptr_C == NERR): # Dash.Error raise ParsingException() else: - if (nleft_after == EOF_NLEFT): - linno_after = linno_after + 1; # The last line wasn't counted + ## dash bumps `linno` on newline, so an unterminated last line is + ## never counted. Only adjust at EOF; otherwise every node would + ## swallow the next one's first line. + if (eof_after & PARSEFILE_EOF): + if (inputPath == "-"): + ## no lines to check against; trust the flag + linno_after = linno_after + 1 + elif (lines and (lines [-1][-1:] != '\n')): + linno_after = len (lines) - if (inputPath != "-"): - ## Both of these assertions check "our" assumption with respect to the final parser state - ## and are therefore not necessary if they become an issue. - assert ((linno_after == len (lines)) or (linno_after == len (lines) + 1)) - - # Last line did not have a newline - assert (len (lines [-1]) > 0 and (lines [-1][-1] != '\n')) + if (inputPath != "-"): + linno_after = min (linno_after, len (lines)) n_ptr = cast (n_ptr_C, POINTER (union_node)) new_ast = of_node (n_ptr) diff --git a/test/line_mapping/multi_no_trailing_newline.expected b/test/line_mapping/multi_no_trailing_newline.expected new file mode 100644 index 0000000..ede9af5 --- /dev/null +++ b/test/line_mapping/multi_no_trailing_newline.expected @@ -0,0 +1,3 @@ +node 0 [0, 1) 'echo one\n' +node 1 [1, 2) 'echo two\n' +node 2 [2, 3) 'echo three' diff --git a/test/line_mapping/multi_no_trailing_newline.sh b/test/line_mapping/multi_no_trailing_newline.sh new file mode 100644 index 0000000..204dfb4 --- /dev/null +++ b/test/line_mapping/multi_no_trailing_newline.sh @@ -0,0 +1,3 @@ +echo one +echo two +echo three \ No newline at end of file diff --git a/test/line_mapping/multiline_at_eof.expected b/test/line_mapping/multiline_at_eof.expected new file mode 100644 index 0000000..0e79186 --- /dev/null +++ b/test/line_mapping/multiline_at_eof.expected @@ -0,0 +1 @@ +node 0 [0, 4) 'if true\nthen\n echo x\nfi' diff --git a/test/line_mapping/multiline_at_eof.sh b/test/line_mapping/multiline_at_eof.sh new file mode 100644 index 0000000..a81a6db --- /dev/null +++ b/test/line_mapping/multiline_at_eof.sh @@ -0,0 +1,4 @@ +if true +then + echo x +fi \ No newline at end of file diff --git a/test/line_mapping/no_trailing_newline.expected b/test/line_mapping/no_trailing_newline.expected new file mode 100644 index 0000000..95305b8 --- /dev/null +++ b/test/line_mapping/no_trailing_newline.expected @@ -0,0 +1 @@ +node 0 [0, 1) 'echo only' diff --git a/test/line_mapping/no_trailing_newline.sh b/test/line_mapping/no_trailing_newline.sh new file mode 100644 index 0000000..7380412 --- /dev/null +++ b/test/line_mapping/no_trailing_newline.sh @@ -0,0 +1 @@ +echo only \ No newline at end of file From 3dd0c39b05153bfeb60ee5fe5eb5f732953beacd Mon Sep 17 00:00:00 2001 From: Dan Liu Date: Sun, 30 Aug 2026 19:55:48 -0400 Subject: [PATCH 4/7] Check the ctypes struct mirrors against dash's headers The root cause was not the EOF logic: _dash.py hand-copies dash's struct layouts and nothing ever verified the copy. libdash sat on dash 0.5.11.5, where the mirror was correct. Upstream then changed the input layer three times -- dcf4ee3 2021-09-05 remove EOF_NLEFT special case (nleft = 0 at EOF) 2c92409 2024-06-02 remove lastc[2] from both structs 69786bc 2024-06-09 add `eof`, move `lleft` after `spfree` -- and f58fffc crossed all three in one hop rewriting history onto dash master. The vendored C followed; _dash.py did not. check_structs.py compiles a probe against src/*.h and compares sizeof and every field offset with ctypes, for all 19 mirrored types. A field C no longer has is a compile error; a moved field is an offset mismatch. Both paths verified against the 0.5.0 mirror and a synthetic field swap. Wired into `make -C python test`. Needs a C compiler and a built tree. The OCaml bindings need no equivalent -- ocaml/dune's ctypes stanza computes offsets from the headers at build time, which is why only the Python side broke. --- python/Makefile | 8 ++- test/README.md | 7 ++ test/check_structs.py | 163 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 176 insertions(+), 2 deletions(-) create mode 100755 test/check_structs.py diff --git a/python/Makefile b/python/Makefile index b3233c7..9e5091c 100644 --- a/python/Makefile +++ b/python/Makefile @@ -1,6 +1,10 @@ -.PHONY: test test-roundtrip test-line-mapping clean +.PHONY: test test-structs test-roundtrip test-line-mapping clean -test: test-roundtrip test-line-mapping +test: test-structs test-roundtrip test-line-mapping + +# _dash.py hand-copies dash's struct layouts; nothing else checks them. +test-structs: ../test/check_structs.py ../libdash/_dash.py + @../test/check_structs.py test-roundtrip: rt.py ../libdash/*.py @find ../test/tests ../test/pash_tests -type f | while read f; do ../test/round_trip.sh ./rt.py "$$f"; done | tee python.log diff --git a/test/README.md b/test/README.md index f1ae577..12936d8 100644 --- a/test/README.md +++ b/test/README.md @@ -8,6 +8,13 @@ There are four directories of tests: Both OCaml and Python bindings use the `round_trip.sh` to test round tripping. The `test_ocaml_python.sh` script compares the output from Python and OCaml. +`check_structs.py` checks the ctypes mirrors in `libdash/_dash.py` against +dash's real structs, comparing `sizeof` and every field offset via a probe +compiled from `src/*.h`. Needs a C compiler and a built tree; skips otherwise. +`_dash.py` hand-copies those layouts, so a moved field is otherwise read +silently at the wrong offset. The OCaml bindings need no equivalent -- +`ocaml/dune`'s ctypes stanza computes offsets from the headers at build time. + `round_trip.sh` checks that `print . parse` reaches a fixpoint. It cannot compare against the original source, since the AST drops comments, whitespace and quoting style -- which leaves the source line mapping (`parsedLines`, diff --git a/test/check_structs.py b/test/check_structs.py new file mode 100755 index 0000000..9d2bc57 --- /dev/null +++ b/test/check_structs.py @@ -0,0 +1,163 @@ +#!/usr/bin/env python3 +"""Check libdash/_dash.py's ctypes mirrors against dash's real C structs. + +_dash.py hand-copies dash's struct layouts and nothing else verifies the copy, +so a moved field is read at the old offset and silently returns a different +member -- as happened when parsefile gained `eof` and Python's `lleft` started +returning it. + +Compiles a probe against src/*.h and compares sizeof and every field offset +with ctypes. Needs a built tree (src/nodes.h is generated). + +The OCaml bindings need no equivalent: ocaml/dune's ctypes stanza computes +offsets from the headers at build time. +""" + +import os +import subprocess +import sys +import tempfile + +from ctypes import sizeof + +ROOT = os.path.dirname (os.path.dirname (os.path.abspath (__file__))) +sys.path.insert (0, ROOT) + +from libdash import _dash + +# (python class, C type, check field offsets too?) +# stackmark is opaque in _dash.py -- only its size is claimed correct. +STRUCTS = [ + ("stackmark", "struct stackmark", False), + ("nodelist", "struct nodelist", True), + ("union_node", "union node", True), + ("ncmd", "struct ncmd", True), + ("npipe", "struct npipe", True), + ("nredir", "struct nredir", True), + ("nbinary", "struct nbinary", True), + ("nif", "struct nif", True), + ("nfor", "struct nfor", True), + ("ncase", "struct ncase", True), + ("nclist", "struct nclist", True), + ("ndefun", "struct ndefun", True), + ("narg", "struct narg", True), + ("nfile", "struct nfile", True), + ("ndup", "struct ndup", True), + ("nhere", "struct nhere", True), + ("nnot", "struct nnot", True), + ("strpush", "struct strpush", True), + ("parsefile", "struct parsefile", True), +] + +PROBE_HEADER = """\ +#include +#include +#include "shell.h" +#include "nodes.h" +#include "input.h" +#include "memalloc.h" + +int main (void) +{ +""" + + +def probe_source (ctype, fields): + body = [' printf ("sizeof %zu\\n", sizeof ({}));'.format (ctype)] + + for name in fields: + body.append (' printf ("{0} %zu\\n", offsetof ({1}, {0}));' + .format (name, ctype)) + + return PROBE_HEADER + "\n".join (body) + "\n return 0;\n}\n" + + +def run_probe (ctype, fields, workdir): + """Returns (offsets dict, None) or (None, error string).""" + src = os.path.join (workdir, "probe.c") + exe = os.path.join (workdir, "probe") + + with open (src, "w") as f: + f.write (probe_source (ctype, fields)) + + cc = os.environ.get ("CC", "cc") + compiled = subprocess.run ([cc, "-I", os.path.join (ROOT, "src"), "-I", ROOT, + "-o", exe, src], + capture_output = True, text = True) + + if compiled.returncode != 0: + # a field C no longer has lands here, e.g. "no member named 'lastc'" + notes = [l.strip () for l in compiled.stderr.splitlines () if "error:" in l] + return (None, "; ".join (notes) or "probe failed to compile") + + ran = subprocess.run ([exe], capture_output = True, text = True) + + if ran.returncode != 0: + return (None, "probe failed to run") + + out = {} + + for line in ran.stdout.split ("\n"): + if line.strip (): + (key, value) = line.rsplit (" ", 1) + out [key] = int (value) + + return (out, None) + + +def resolve (pyname): + """_dash.py's `def nodelist` shadows `class nodelist`; reach the class + through a field pointing at it.""" + found = getattr (_dash, pyname) + + if pyname == "nodelist" and not hasattr (found, "_fields_"): + return dict (_dash.narg._fields_) ["backquote"]._type_ + + return found + + +def main (): + if not os.path.exists (os.path.join (ROOT, "src", "nodes.h")): + print ("SKIP: src/nodes.h not generated; build libdash first") + return 0 + + status = 0 + + with tempfile.TemporaryDirectory () as workdir: + for (pyname, ctype, check_fields) in STRUCTS: + cls = resolve (pyname) + fields = [n for (n, _t) in cls._fields_] if check_fields else [] + + (c, err) = run_probe (ctype, fields, workdir) + + if err is not None: + print ("STRUCT_MISMATCH: '{}' ({}): {}".format (ctype, pyname, err)) + status = 1 + continue + + problems = [] + + if sizeof (cls) != c ["sizeof"]: + problems.append ("sizeof: python {}, C {}" + .format (sizeof (cls), c ["sizeof"])) + + for name in fields: + mine = getattr (cls, name).offset + + if mine != c [name]: + problems.append ("field {}: python offset {}, C offset {}" + .format (name, mine, c [name])) + + if problems: + for p in problems: + print ("STRUCT_MISMATCH: '{}' ({}): {}".format (ctype, pyname, p)) + status = 1 + else: + print ("PASS '{}' (sizeof {}, {} field(s))" + .format (ctype, c ["sizeof"], len (fields))) + + return status + + +if __name__ == "__main__": + sys.exit (main ()) From e908cdbe16292f858148d6239142aded7305ca60 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Tue, 1 Sep 2026 11:43:42 -0400 Subject: [PATCH 5/7] case from @nad2040's original bug report --- test/line_mapping/cases.expected | 5 +++++ test/line_mapping/cases.sh | 8 ++++++++ 2 files changed, 13 insertions(+) create mode 100644 test/line_mapping/cases.expected create mode 100644 test/line_mapping/cases.sh diff --git a/test/line_mapping/cases.expected b/test/line_mapping/cases.expected new file mode 100644 index 0000000..ed461b2 --- /dev/null +++ b/test/line_mapping/cases.expected @@ -0,0 +1,5 @@ +node 0 [0, 1) 'echo A\n' +node 1 [1, 2) 'echo B; echo C\n' +node 2 [2, 5) 'for i in 1 2; do\n echo "loop $i"\ndone\n' +node 3 [5, 7) 'echo D | \\\n cat\n' +node 4 [7, 8) 'echo E\n' diff --git a/test/line_mapping/cases.sh b/test/line_mapping/cases.sh new file mode 100644 index 0000000..983fddc --- /dev/null +++ b/test/line_mapping/cases.sh @@ -0,0 +1,8 @@ +echo A +echo B; echo C +for i in 1 2; do + echo "loop $i" +done +echo D | \ + cat +echo E From bacbfea645c90abe387960d0207a36cddffbf878 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Tue, 1 Sep 2026 11:49:34 -0400 Subject: [PATCH 6/7] clarify readme good lord Claude is a bad writer Signed-off-by: Michael Greenberg --- test/README.md | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/README.md b/test/README.md index 12936d8..f958e28 100644 --- a/test/README.md +++ b/test/README.md @@ -6,19 +6,19 @@ There are four directories of tests: - `line_mapping` holds fixtures for the per-node source line mapping, each with a `.expected` golden -Both OCaml and Python bindings use the `round_trip.sh` to test round tripping. The `test_ocaml_python.sh` script compares the output from Python and OCaml. +Both OCaml and Python bindings use the `round_trip.sh` to test round tripping, i.e., that `print . parse` reaches a fixpoint. +It cannot compare against the original source, since the AST drops comments, whitespace and quoting style. +The `test_ocaml_python.sh` script ensures that Python and OCaml round-trip to the same output (or fail together). + +Round trip tests, however, can hide important bugs. +Two other tests check parses more concretely. `check_structs.py` checks the ctypes mirrors in `libdash/_dash.py` against dash's real structs, comparing `sizeof` and every field offset via a probe -compiled from `src/*.h`. Needs a C compiler and a built tree; skips otherwise. -`_dash.py` hand-copies those layouts, so a moved field is otherwise read -silently at the wrong offset. The OCaml bindings need no equivalent -- -`ocaml/dune`'s ctypes stanza computes offsets from the headers at build time. +compiled from `src/*.h`. +This test needs a C compiler and a built tree. +This test only runs on the Python code, as OCaml's ctypes automatically computes offsets, but `_dash.py` hand-writes them. -`round_trip.sh` checks that `print . parse` reaches a fixpoint. It cannot -compare against the original source, since the AST drops comments, whitespace -and quoting style -- which leaves the source line mapping (`parsedLines`, -`linno_before`, `linno_after`) uncovered, as `rt.py` discards it and prints only -the AST. `line_mapping.sh` covers that: it runs `python/dump.py --ranges` over -each fixture and diffs against the golden. `REGEN=1 ./line_mapping.sh` updates -the goldens after an intentional change. +`line_mapping.sh` covers source line mapping (`parsedLines`, `linno_before`, `linno_after`). +It runs `python/dump.py --ranges` over each fixture and diffs against the `.expected` output. +Run `REGEN=1 ./line_mapping.sh` to automatically update the expected output after an intentional change. From 86f6de56c1b5cab7480346cd7cbb7db5957e4655 Mon Sep 17 00:00:00 2001 From: Michael Greenberg Date: Tue, 1 Sep 2026 17:27:03 -0400 Subject: [PATCH 7/7] bump versions --- dune-project | 2 +- pyproject.toml | 2 +- setup.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dune-project b/dune-project index b59caa9..b5c9ed8 100644 --- a/dune-project +++ b/dune-project @@ -10,7 +10,7 @@ (package (name libdash) (synopsis "Bindings to the dash shell's parser") - (version 0.5.0) + (version 0.5.1) (depends ("ctypes" (>= "0.21.1")) ("ctypes-foreign" (>= "0.21.1")) diff --git a/pyproject.toml b/pyproject.toml index 8904bfd..3e35d97 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "libdash" -version = "0.5.0" +version = "0.5.1" authors = [ { name="Michael Greenberg", email="michael@greenberg.science" }, { name="PaSh contributors" }, diff --git a/setup.py b/setup.py index f9c8387..eaf7397 100644 --- a/setup.py +++ b/setup.py @@ -61,7 +61,7 @@ def run(self): setup(name='libdash', packages=['libdash'], cmdclass={'build_py': libdash_build_py}, - version='0.5.0', + version='0.5.1', long_description=long_description, long_description_content_type='text/markdown', include_package_data=True,