`func` doesn't check for side effects in proc parameters

[ee7]

What's the intended behaviour here?

Example

proc echoInt(n: int) =
  echo n

func foo(n: int, bar: proc(x: int)) =
  bar(n) 

foo(2, echoInt)

See: https://play.nim-lang.org/#ix=2HiM

Current Output

2

which also occurs if you compile with --experimental:strictFuncs.

Expected Output

Either

2

in which case, I think the manual could be a little clearer, or:

foo.nim(4, 6) Error: 'foo' can have side effects

which is what happens if you call echoInt inside the func more directly:

 proc echoInt(n: int) =
   echo n

 func foo(n: int, bar: proc(x: int)) =
-  bar(n)
+  echoInt(n)

 foo(2, echoInt)

An error is produced if we write:

 proc echoInt(n: int) =
   echo n

-func foo(n: int, bar: proc(x: int)) =
+func foo(n: int, bar: proc(x: int) {.noSideEffect.}) =
  bar(n)

 foo(2, echoInt)

Additional Information

This behaviour occurs with devel and all Nim versions going back to v0.18.0, which introduced func.

The manual contains:

Nim/doc/manual.rst

Lines 6155 to 6161 in 9ce2f87

	The ``noSideEffect`` pragma is used to mark a proc/iterator to have no side
	effects. This means that the proc/iterator only changes locations that are
	reachable from its parameters and the return value only depends on the
	arguments. If none of its parameters have the type ``var T`` or ``out T``
	or ``ref T`` or ``ptr T`` this means no locations are modified. It is a static
	error to mark a proc/iterator to have no side effect if the compiler cannot
	verify this.

The procedural type is internally a pointer to a procedure, but I don't know if it's supposed to be covered by "var T or out T
or ref T or ptr T".

The experimental manual says this about strictFuncs:

Nim/doc/manual_experimental.rst

Lines 1785 to 1790 in 9ce2f87

	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.

[timotheecour]

IMO the current behavior is both the correct and the useful one, the side effect only happens via a parameter which is declared proc.

The manual just needs to be clarify that side effects are allowed via proc parameters.

[ee7]

--experimental:strictFuncs enforces that a parameter that mutates must be a var parameter.

Could it also enforce that a proc parameter does not have side effects?

[timotheecour]

Could it also enforce that a proc parameter does not have side effects?

that would make func much less useful; the fact that proc param can mutate is already encoded in the type of the proc being proc instead of func; if you want to ensure no mutation happens, use func for the proc parameter. The current func design (EDIT: excluding strictFuncs/views, refs #16305) is sane and gives API strictly more control

[ee7]

if you want to ensure no mutation happens, use func for the proc parameter.

Do you mean, "add {.noSideEffect.} to the proc parameter"? Like:
func foo(bar: proc(n: int) {.noSideEffect.}) =

Because if you write:
func foo(bar: func(n: int)) =

it produces:

Error: func keyword is not allowed in type descriptions, use proc with {.noSideEffect.} pragma instead

[timotheecour]

Do you mean, "add {.noSideEffect.} to the proc parameter"? Like:

• yes. fixes #10838 #10841 should be revisited and func keyword for proc types doesn't imply noSideEffect #10838 should be fixed in a way than amounts to adding {.noSideEffect.}

[timotheecour]

And here's an example explaining why it's safe to have a func that contains a proc param:

when true:
  func callCb(cb: proc(): int): int = result = cb()
  proc mycb1(): int = 1
  proc mycb2(): int = (echo "ok"; 1)
  func bar(): int =
    # result = callCb(mycb1) # ok: no error generated here
    result = callCb(mycb2) # ok: correctly reports Error: 'bar' can have side effects
  echo (bar(),)

callCb is legally a func, and compiler guarantees side effects can only happen through cb.
But calling callCb itself does correctly generate a side effect error if used inside a func and its param cb has side effects, as in result = callCb(mycb2)

[Araq]

Works as the spec says it does, the spec is overly brief though.

See for example:

proc echoInt(n: int) =
  echo n

func foo(n: int, bar: proc(x: int)) =
  bar(n)

func more =
  foo(2, echoInt) # error: 'more' can have side effects

more()

[timotheecour]

Works as the spec says it does, the spec is overly brief though.

the spec doesn't match the implementation, as I explained above.
https://nim-lang.github.io/Nim/manual.html#pragmas-nosideeffect-pragma says:

If none of its parameters have the type var T or out T or ref T or ptr T this means no locations are modified.

That's incorrect, see below example:
bar is neither ptr|ref nor a var (and out T shouldn't be there) and yet count is modified:

when true:
  func foo(bar: proc(): int): int = bar()
  var count = 0
  proc fn1(): int = 1
  proc fn2(): int = (count.inc; count)
  func fun1() = discard foo(fn1) # ok
  # func fun2() = discard foo(fn2) # Error: 'fun2' can have side effects

  # with callbacks, the compiler is conservative, ie that bar will have side effects
  var foo1: type(foo) = foo
  func main() =
    discard foo(fn1) # ok
    # discard foo1(fn1) # now this errors
  main()

This time, the implementation is the correct, sane, most useful behavior (see #16303 (comment)), the spec is overly restrictive.

It's easy to correct and I can volunteer a PR to fix it, along those lines:

---

Side effects in a {.noSideEffect.} proc foo, are only allowed through proc params, and when all such params are statically determined to be {.noSideEffect.}, foo will indeed be {.noSideEffect.}, in other words {.noSideEffect.}` is determined at call site, see example below:

< insert above example >.

If none of its parameters have the type var T or out T or ref T or ptr T or proc without {.noSideEffect.} this means no locations are modified.

---

In the new spec, {.noSideEffect.} tells you that side effects only happen through proc params, which is still a strong guarantee, and make these procs useful in more contexts:

• it allows uing foo inside a func (when all its params are statically {.nosideeffect.})
• but it also allows passing params with side effects (and then resulting foo won't itself be usable inside a func, and the compiler checks for this)