Continue a line comment over a trailing underscore - #5
Conversation
VB6 continues a logical line with a space followed by an underscore, and a comment is a logical line: the text on the next line is still comment, even though it does not start with its own `'`. The lexer stopped at the newline, so that next line was tokenized as code. Two things fall out of that. Prose from a comment becomes identifiers, and any character in it that is not ASCII reaches the tokenizer's fallback and is reported as an unknown token -- on the code base I tried this on, 307 lines are continuations of a comment and they accounted for 651 spurious findings, all of them accented vowels from Spanish prose. Worse, commented-out code is parsed as real code: test-data has a case where `'.Value = _` is followed by a call that was being analyzed as though it were live. Keep taking lines while the one just taken ends in a continuation, and emit the run as a single comment token. Two limits, each pinned down by one of their own snapshots: - The space before the `_` is required. `Close_` at the end of a line is an identifier ending in an underscore, not a continuation. The environment_ctlsclient1 snapshot caught this. - A continuation line that opens its own comment is left as its own token. Nothing is misread there, so there is no reason to change how source that already worked is tokenized. environment_cregistry caught this one. environment_exifread is regenerated: it holds the commented-out call described above, which now sits inside the comment token where it belongs. REM comments go through the same path and get the same treatment.
The check for a `REM` on the continuation line used `line[..4]`, which
panics when the line begins with a multi-byte character -- exactly what a
continuation line holding Spanish prose does:
end byte index 4 is not a char boundary; it is inside 'á'
Found by running the built binary over a real code base rather than only the
test suite. `str::get` returns `None` at a non-boundary instead.
|
Correction to what I wrote above about the eight failing The cause is line endings. The So: those eight are green in your CI, they are unrelated to this PR, and there is nothing here for you to fix. I should have looked at the diff before explaining it rather than after. The one thing that might be worth having is a note for contributors on Windows to set |
1 participant
A comment ending in a space and an underscore continues onto the next line, the same way any other logical line does. The lexer stopped at the newline, so that next line was tokenized as code.
The second line has no
'of its own, so it was read as code. Two consequences:test-dataalready contains a case: inenvironment/ExifRead.cls,'.Value = _is followed by a call that was being parsed as a realCallStatement.The fix
Keep taking lines while the one just taken ends in a continuation, and emit the run as a single comment token.
Two limits, and I want to flag that each was found by one of your own snapshots, not by me:
_is required.'m_oSocket.Close_is an identifier ending in an underscore, not a continuation.environment_ctlsclient1caught my first version, which got this wrong.environment_cregistrycaught this.Snapshot change
environment_exifreadis regenerated, and it is the one deliberate behaviour change here: the commented-out call described above now sits inside the comment token where it belongs. That is the point of the PR rather than a side effect, so I would rather call it out than have it noticed in review.Tests
Seven in
lexer::tests: continuation onto plain text, over several lines in a row, with trailing whitespace after the_, the no-continuation case, an underscore without a leading space, a continuation onto another comment, and the same forREM.cargo test -p vb6parse: 5656 pass. The eightclass_loadfailures are the pre-existing ones with no committed.snapbaseline (fiveaudiostation, threecdiu_beat_up_editor); they fail identically on an unmodifiedmasterhere.Independent of #2 and #4 — touches only the comment path in the lexer.