Skip to content

Commit

Permalink
sequtils: fix errors from strictFuncs use (nim-lang#18998)
Browse files Browse the repository at this point in the history
Nim 1.4.x compiled the below code without error when using
`--experimental:strictFuncs`

    import std/sequtils

    type Foo = ref object

    let foo1 = Foo()
    let foo2 = Foo()
    let foos = @[foo1, foo2]
    let fooTuples = @[(foo1, 1), (foo2, 2)]

    discard repeat(foo1, 3)
    discard zip(foos, foos)
    discard unzip(fooTuples)

However, since 2020-12-09, devel Nim produced errors like

    /tmp/bar.nim(11, 15) template/generic instantiation of `repeat` from here
    /foo/nim/pure/collections/sequtils.nim(172, 6) Error: 'repeat' can have side effects
    an object reachable from 'x' is potentially mutated
    /foo/nim/pure/collections/sequtils.nim(183, 15) the mutation is here
    /foo/nim/pure/collections/sequtils.nim(183, 15) is the statement that connected the mutation to the parameter

This commit reverts some `proc` to `func` changes so that code that:

- calls `repeat`, `zip`, or `unzip`
- and instantiates them with types containing `ref`

can once again be compiled with `strictFuncs`. Otherwise, a user might
be forced to drop or alter their `strictFuncs` use when upgrading from
Nim 1.4.x, or when writing new code that uses these procedures (at least
for now, with the current `strictFuncs` implementation).

This commit also adds tests to assert that the remaining funcs in this
module can be compiled with `strictFuncs` when used with types
containing `ref`.

The original batch of `proc` to `func` changes in `sequtils.nim` was in
commit 6f57eba, which was partially reverted in 38eb021.

See also: nim-lang#16305
  • Loading branch information
ee7 authored Oct 16, 2021
1 parent f4525ef commit 3b1a601
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 3 deletions.
6 changes: 3 additions & 3 deletions lib/pure/collections/sequtils.nim
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func cycle*[T](s: openArray[T], n: Natural): seq[T] =
result[o] = e
inc o

func repeat*[T](x: T, n: Natural): seq[T] =
proc repeat*[T](x: T, n: Natural): seq[T] =
## Returns a new sequence with the item `x` repeated `n` times.
## `n` must be a non-negative number (zero or more).
##
Expand Down Expand Up @@ -246,7 +246,7 @@ func maxIndex*[T](s: openArray[T]): int {.since: (1, 1).} =


template zipImpl(s1, s2, retType: untyped): untyped =
func zip*[S, T](s1: openArray[S], s2: openArray[T]): retType =
proc zip*[S, T](s1: openArray[S], s2: openArray[T]): retType =
## Returns a new sequence with a combination of the two input containers.
##
## The input containers can be of different types.
Expand Down Expand Up @@ -289,7 +289,7 @@ when (NimMajor, NimMinor) <= (1, 0):
else:
zipImpl(s1, s2, seq[(S, T)])

func unzip*[S, T](s: openArray[(S, T)]): (seq[S], seq[T]) {.since: (1, 1).} =
proc unzip*[S, T](s: openArray[(S, T)]): (seq[S], seq[T]) {.since: (1, 1).} =
## Returns a tuple of two sequences split out from a sequence of 2-field tuples.
runnableExamples:
let
Expand Down
26 changes: 26 additions & 0 deletions tests/stdlib/tsequtils.nim
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ from algorithm import sorted

{.experimental: "strictEffects".}
{.push warningAsError[Effect]: on.}
{.experimental: "strictFuncs".}

# helper for testing double substitution side effects which are handled
# by `evalOnceAs`
Expand Down Expand Up @@ -456,6 +457,31 @@ block:
# xxx: obscure CT error: basic_types.nim(16, 16) Error: internal error: symbol has no generated name: true
doAssert: iter(3).mapIt(2*it).foldl(a + b) == 6

block: # strictFuncs tests with ref object
type Foo = ref object

let foo1 = Foo()
let foo2 = Foo()
let foos = @[foo1, foo2]

# Procedures that are `func`
discard concat(foos, foos)
discard count(foos, foo1)
discard cycle(foos, 3)
discard deduplicate(foos)
discard minIndex(foos)
discard maxIndex(foos)
discard distribute(foos, 2)
var mutableFoos = foos
mutableFoos.delete(0..1)
mutableFoos.insert(foos)

# Some procedures that are `proc`, but were reverted from `func`
discard repeat(foo1, 3)
discard zip(foos, foos)
let fooTuples = @[(foo1, 1), (foo2, 2)]
discard unzip(fooTuples)

template main =
# xxx move all tests here
block: # delete tests
Expand Down

0 comments on commit 3b1a601

Please sign in to comment.