Skip to content

strictFuncs: incorrect error: multiple assignment involving ref object parameter #20863

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ee7 opened this issue Nov 16, 2022 · 0 comments
Closed

Comments

@ee7
Copy link
Contributor

ee7 commented Nov 16, 2022

Example

{.experimental: "strictFuncs".}

type
  Foo = ref object

func twice(foo: Foo) =
  var a = newSeq[Foo](2)
  a[0] = foo # No error.
  a[1] = foo # Error: 'twice' can have side effects.

let foo = Foo()
twice(foo)

Current behavior

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

Expected behavior

Compiles without error.

Nim version

Occurs in Nim devel (3d692d0), Nim 1.6.8, and every Nim version back to 1.4.0 (which introduced strictFuncs).

Additional information

This issue is a follow-up to #18998 (comment) and #16305. I'm attempting to make the latter more actionable by tracking the maybe-simplest subset of problems in a separate issue, with a single clear reduction. But feel free to close this as a duplicate if we prefer a single issue that's probably harder to close.

We see no error if we make Foo a non-ref object, or change twice to any of the below:

func twice(foo: Foo) =
  var a = newSeq[Foo](2)
  a[0] = foo
func twice(foo: Foo) =
  var a = newSeq[Foo](2)
  a[0] = foo
  var b = foo
func twice(foo: Foo) =
  var a = newSeqOfCap[Foo](2)
  a.add foo
  a.add foo

The example at the top of this post is a reduction of the problem that keeps procedures like sequtils.zip,sequtils.unzip, and sequtils.repeat as a proc:

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).
##
runnableExamples:
let
total = repeat(5, 3)
assert total == @[5, 5, 5]
result = newSeq[T](n)
for i in 0 ..< n:
result[i] = x

Because when we made each a func, something like the below would produce an error:

{.experimental: "strictFuncs".}

import std/sequtils

type
  Foo = ref object

let foo = Foo()
discard repeat(foo, 3) # Error: 'repeat' can have side effects

Background

The docs for strictFuncs say:

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.

So we shouldn't see an error in any of the above cases, because there is no mutation.

Araq added a commit that referenced this issue Dec 11, 2022
@Araq Araq closed this as completed in 3812d91 Dec 11, 2022
survivorm pushed a commit to survivorm/Nim that referenced this issue Feb 28, 2023
…g#21066)

* alternative, much simpler algorithm for strict func checking

* forgot to git add new compiler module

* new spec is incredibly simple to describe

* fixes bigints regression

* typos

* closes nim-lang#16305; closes nim-lang#17387; closes nim-lang#20863
capocasa pushed a commit to capocasa/Nim that referenced this issue Mar 31, 2023
…g#21066)

* alternative, much simpler algorithm for strict func checking

* forgot to git add new compiler module

* new spec is incredibly simple to describe

* fixes bigints regression

* typos

* closes nim-lang#16305; closes nim-lang#17387; closes nim-lang#20863
bung87 pushed a commit to bung87/Nim that referenced this issue Jul 29, 2023
…g#21066)

* alternative, much simpler algorithm for strict func checking

* forgot to git add new compiler module

* new spec is incredibly simple to describe

* fixes bigints regression

* typos

* closes nim-lang#16305; closes nim-lang#17387; closes nim-lang#20863
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants