fix line attribution - #38
Merged
Merged
Conversation
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.
`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.
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).
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.
good lord Claude is a bad writer Signed-off-by: Michael Greenberg <michael@greenberg.science>
mgree
marked this pull request as ready for review
September 1, 2026 15:50
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.