nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.55k stars 1.47k forks source link

Regression bug in lines() #8961

Closed simonkrauter closed 6 years ago

simonkrauter commented 6 years ago

Commit 7896903fd09ee5c855112660179b6f4ec57a1977 introduced this bug.

lines() omits last line under some conditions.

Test program:

for line in lines("test.txt"):
  echo line

test.txt:

1
2aaaaaaaa
3bbbbbbb

with Windows line breaks, no line break at the last line.

Output:

1
2aaaaaaaa

Expected output, output with older version of Nim and output of readFile("test.txt").splitLines():

1
2aaaaaaaa
3bbbbbbb

Reproducible with:

Nim Compiler Version 0.18.1 [Windows: amd64] Compiled at 2018-09-13 git hash: 382fe446c3926f7976de09b7a1d8ad131912c7b6 active boot switches: -d:release

Nim Compiler Version 0.18.1 [Linux: amd64] Compiled at 2018-09-03 git hash: b53531ee31558484304df4552c2e66cf61842eb8 active boot switches: -d:release

dm1try commented 6 years ago

UNRELATED to this issue ~btw on Mac OS the behaviour is opposite splitLines returns extra empty line~

# test.txt

some line
another one
and another

# hex repr
00000000: 736f 6d65 206c 696e 650a 616e 6f74 6865  some line.anothe
00000010: 7220 6f6e 650a 616e 6420 616e 6f74 6865  r one.and anothe
00000020: 720a                                     r.
echo readFile("test.txt").splitLines().repr

# 0x10202f048[0x10202d088"some line", 0x10202d118"another one", 0x10202d178"and another", ""]

ruby

pp File.read("test.txt").lines
# ["some line\n", "another one\n", "and another\n"]

~reproducible on 0.18.0 and on devel branch~

skilchen commented 6 years ago

The condition triggering this regression is the fact that the last line is exactly one character shorter than the previous one. (can you verify this on windows?) A quick fix would be to change the top of the procedure readLine from:

  # Use the currently reserved space for a first try
  var sp = line.string.len
  if sp == 0:
    sp = 80
    line.string.setLen(sp)

to something more like the previous version:

  # Use the currently reserved space for a first try
  var sp = line.string.len
  if sp == 0:
    sp = 80
    line.string.setLen(sp)
  else:
    when not defined(nimscript):
      sp = cint(cast[PGenericSeq](line.string).space)
    else:
      line.string.setLen(sp + 1)
dm1try commented 6 years ago

oops, sorry guys =) ignore my comment above about splitLines (which also looks like a bug, but independent from this issue)

simonkrauter commented 6 years ago

Commit 7896903fd09ee5c855112660179b6f4ec57a1977 broke it, I have just tested it.