times.parse causes incorrect proveInit warning

[auxym]

Example

import seqUtils
import times

proc parseMyDates(line: string): DateTime =
    result = parse(line, "yyyy-MM-dd")

var dateStrings = @["2018-12-01", "2018-12-02", "2018-12-03"]
var parsed = dateStrings.map(parseMyDates)

Current Output

Compiler warnings:

scratch.nim(9, 25) template/generic instantiation of `map` from here
/usr/lib/nim/pure/collections/sequtils.nim(238, 10) Warning: Cannot prove that 'result' is initialized. This will become a compile time error in the future. [ProveInit]

Expected Output

I'm new to Nim, so I was not expecting this code to output a warning. Quick question to IRC yielded a suggestion that this was likely a bug in times or seqUtils, so here I am.

If the warning is not a bug, then may I suggest clearer warning, or suggestion of solution? The current compiler output has me thoroughly confused.

Possible Solution

Well, the solution I am using at the moment is ignore the warning. As discussed above, my solution would be either to fix whatever is causing the warning, or if the warning should stay, make it be easier to understand.

Additional Information

[GULPF]

I think sequtils.map should use newSeqOfCap instead of newSeq.

The reason you get the warning is that DateTime has a field which is not valid when uninitialized. DateTime.month only accepts the values 1-12, so the default value (0) is invalid. When using newSeq(n), a seq with length n filled with the default values will be created, which will be invalid. By using newSeqOfCap(n) instead the issue should be avoided.

[timotheecour]

but map assigns all values in the seq greedily, so maybe its a bug with proveInit; newSeqOfcap could be a tad less efficient due to len updates required after each add (especially in release mode)

[andreaferretti]

@GULPF A solution would be to store the month internally as 0-11 and add/subtract 1 in each function of the public API (same for day). Or maybe map should push proveInit: off and then pop it at the end?

[timotheecour]

Or maybe map should push proveInit: off and then pop it at the end?

maybe as a workaround for false positives of proveInit algorithm, but what would be effect of that if proveInit is actually useful INSIDE the argument given to toSeq?

iterator myiter():A=
  # we don't  want `proveInit` in toSeq to change behavior inside this
  yield some_expr

let a = toSeq(myIter())

[GULPF]

A solution would be to store the month internally as 0-11 and add/subtract 1 in each function of the public API (same for day)

It fixes this particular issue, but every other non-zero type will still give warnings. This issue isn't only for map either:

type T = range[1..2]

import tables

var t = initTable[int, T]()
# lib/pure/collections/tables.nim(325, 3) Warning: Cannot prove that 'result' is initialized. This will become a compile time error in the future. [ProveInit]

var s = (1.T)..(2.T)
# lib/system.nim(325, 3) Warning: Cannot prove that 'result' is initialized. This will become a compile time error in the future. [ProveInit]

Non-zero types are generally badly supported in the language. No idea how realistic it is, but I sometimes which that these types would be initialized to their lowest value instead of 0.

[andreaferretti]

@timotheecour Well, proveinit could be disabled just around this line. So we would still have proveInit active while initializing the actual objects that end up in the result

@GULPF Right - but maybe a quicker win would be to push/pop proveInit:off around some lines in initTable as well