Strict func does not catch mutation

[planetis-m]

What happened?

Line 9 mutates tree but its not caught by the compiler.

type
  Node = ref object
    name: string
    kids: seq[Node]
    parent: Node

func initParents(tree: Node) =
  for kid in tree.kids:
    kid.parent = tree
    initParents(kid)

proc process(intro: Node): Node =
  var tree = Node(name: "root", kids: @[
    intro,
    Node(name: "one", kids: @[
      Node(name: "two"),
      Node(name: "three"),
    ]),
    Node(name: "four"),
  ])
  initParents(tree)

proc main() =
  var intro = Node(name: "intro")
  var tree = process(intro)
  echo intro.parent.name

main()

Nim Version

Nim Compiler Version 1.7.3 [Linux: amd64]
Compiled at 2022-11-09
Copyright (c) 2006-2022 by Andreas Rumpf

git hash: 6894a00
active boot switches: -d:release --gc:markAndSweep

Current Standard Output Logs

compiles

Expected Standard Output Logs

Error: 'initParents' can have side effects an object reachable from 'tree' is potentially mutated

Possible Solution

No response

Additional Information

No response

[Araq]

@bung87 please investigate.

[ee7]

The below is a smaller reproduction, right?

{.experimental: "strictFuncs".}

type
  Node = ref object
    num: int
    kids: seq[Node]

func initKids(node: Node) =
  # node.num = 7 # Error: an object reachable from 'node' is potentially mutated
  for kid in node.kids:
    kid.num = 42 # No error.

proc newNode: Node =
  var kid = Node()
  result = Node(kids: @[kid])
  initKids(result)

echo newNode().kids[0].num

It looks like this problem exists in Nim devel, Nim 1.6.8 and every Nim release since 1.4.0 (which added strictFuncs).

[bung87]

reduced example, it's not handled by current implementation.

{.experimental: "strictFuncs".}

type
  Node = ref object
    num: int
    kids: seq[Node]

func initKids(node: Node) =
#   node.num = 7 # Error: an object reachable from 'node' is potentially mutated
  for kid in node.kids:
    kid.num = 42 # No error.

var kid = Node()
var a = Node(kids: @[kid])
initKids(a)

[bung87]

duno what expected here

func initKids2(kids: seq[Node]) =
  for kid in kids:
    kid.num = 42

let kids = @[kid]
initKids2(kids)

[bung87]

I believe after transforming yield in transf.nim here losing lent information which is immutableView, also the analyzer traverse kid.num = 42 seperately

[ee7]

duno what expected here

An error, right? The experimental manual says:

Since version 1.4, a stricter definition of "side effect" is available. In addition to the existing rule that a side effect is calling a function with side effects, the following rule is also enforced:

Any mutation to an object does count as a side effect if that object is reachable via a parameter that is not declared as a var parameter.

I think the minimal reproduction so far is:

{.experimental: "strictFuncs".}

type
  Node = ref object
    num: int

func mutate(nodes: seq[Node]) =
  for node in nodes:
    node.num = 42 # No error, despite mutation of an object reachable via non-`var` parameter.

let nodes = @[Node(num: 7)]
mutate(nodes)
echo nodes[0].num

Current output:

42

Expected output:

foo.nim(7, 6) Error: 'mutate' can have side effects
an object reachable from 'nodes' is potentially mutated
foo.nim(9, 9) the mutation is here

[ee7]

Note that the error does appear when we remove the for loop:

{.experimental: "strictFuncs".}

type
  Node = ref object
    num: int

func mutate(nodes: seq[Node]) =
  let node = nodes[0]
  node.num = 42 # Error, as expected.

let nodes = @[Node(num: 7)]
mutate(nodes)
echo nodes[0].num

Output:

foo.nim(7, 6) Error: 'mutate' can have side effects
an object reachable from 'nodes' is potentially mutated
foo.nim(9, 7) the mutation is here
foo.nim(8, 7) is the statement that connected the mutation to the parameter

[ee7]

But no error when we assign node via a procedure:

{.experimental: "strictFuncs".}

type
  Node = ref object
    num: int

func first(nodes: seq[Node]): Node =
  nodes[0]

func mutate(nodes: seq[Node]) =
  let node = first(nodes)
  node.num = 42 # No error, despite mutation of an object reachable via non-`var` parameter.

let nodes = @[Node(num: 7)]
mutate(nodes)
echo nodes[0].num

So it's not just due to for or the items iterator.

[planetis-m]

So the analysis works only if the return type is lent T but it doesn't work for iterators.

[Araq]

In theory it works for all cases that have been shown here. It's just an implementation bug, lent T is not important.

[ee7]

In theory it works for all cases that have been shown here. It's just an implementation bug, lent T is not important.

That's what I understood from the spec - thanks for confirming.

My understanding is that @planetis-m was just saying that (correct me if I'm wrong):

• the code in #20808 (comment) does produce the expected error when only modifying first to return lent Node
• but the below doesn't produce an error, regardless of whether myItems returns Node or lent Node

{.experimental: "strictFuncs".}

type
  Node = ref object
    num: int

iterator myItems(nodes: seq[Node]): lent Node =
  var i = 0
  let L = nodes.len
  while i < L:
    yield nodes[i]
    inc i

func mutate(nodes: seq[Node]) =
  for node in nodes.myItems:
    node.num = 42 # No error, despite mutation of an object reachable via non-`var` parameter.

let nodes = @[Node(num: 7)]
mutate(nodes)
echo nodes[0].num

[ee7]

In case it's convenient, somebody working on the write tracking might like to see if they can address #20863 at the same time.

It's another strictFuncs thing with a ref object parameter, but an error for non-mutation rather than a lack of error for a mutation.

[Araq]

Not release blocking. I also have a better design for strict functions that might make it into version 2.2.