Skip to content
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

Modification via a ref isn't detected with {.experimental: "strictFuncs".} with default GC #15096

Closed
sschwarzer opened this issue Jul 27, 2020 · 3 comments

Comments

@sschwarzer
Copy link
Contributor

A modification of a recursive structure via a ref isn't detected by the compiler.

Example

{.experimental: "strictFuncs".}
type
  XmlNode = ref object
    s: seq[XmlNode]

func processNode(node: XmlNode) =
  node.s.delete(0)

let node = XmlNode(s: @[XmlNode()])
node.processNode()

(Thanks to @Yardanico for reducing my code example further. :-) )

Current Output

Hint: used config file '/.../.choosenim/toolchains/nim-#devel/config/nim.cfg' [Conf]
Hint: used config file '/.../.choosenim/toolchains/nim-#devel/config/config.nims' [Conf]
Hint: used config file '/.../config.nims' [Conf]                                
....                                                                            
Hint:  [Link]                                                                   
Hint: 30952 lines; 0.166s; 31.805MiB peakmem; Debug build; proj: /.../strict_funcs_test.nim; out: /.../strict_funcs_test [SuccessX]

Expected Output

The modification is detected with --gc:orc.

Hint: used config file '/.../.choosenim/toolchains/nim-#devel/config/nim.cfg' [Conf]
Hint: used config file '/.../.choosenim/toolchains/nim-#devel/config/config.nims' [Conf]
Hint: used config file '/.../config.nims' [Conf]                                
.....                                                                           
/../strict_funcs_test.nim(7, 9) template/generic instantiation of `delete` from here
/../.choosenim/toolchains/nim-#devel/lib/system.nim(1268, 16) template/generic instantiation of `defaultImpl` from here
/../.choosenim/toolchains/nim-#devel/lib/system.nim(1231, 13) template/generic instantiation of `move` from here
/../.choosenim/toolchains/nim-#devel/lib/system.nim(259, 10) template/generic instantiation of `setLen` from here
/../.choosenim/toolchains/nim-#devel/lib/system/seqs_v2.nim(112, 6) Error: 'setLen' can have side effects
an object reachable from 'newlen' is potentially mutated                        
/../.choosenim/toolchains/nim-#devel/lib/system/seqs_v2.nim(121, 11) the mutation is here
/../.choosenim/toolchains/nim-#devel/lib/system/seqs_v2.nim(122, 9) is the statement that connected the mutation to the parameter

Additional Information

$ nim -v
Nim Compiler Version 1.3.5 [Linux: amd64]
Compiled at 2020-07-26
Copyright (c) 2006-2020 by Andreas Rumpf

active boot switches: -d:release
@Araq
Copy link
Member

Araq commented Jul 27, 2020

The problem here is that you enable the strict checking for your own code and not for delete.

@bung87
Copy link
Collaborator

bung87 commented Jul 27, 2020

with default GC nimSeqsV2 not enabled , it will not call setLen in seqs_v2.nim which declared as {.noSideEffect.}

@sschwarzer
Copy link
Contributor Author

The problem here is that you enable the strict checking for your own code and not for delete.

Thank you.

Does that mean I should get the compiler error for

$ nim c --experimental:strictFuncs strictfuncs_test.nim

? I don't. :)

And according to what you said, --gc:orc shouldn't have given me an error either, if I understood correctly. :)

Araq added a commit that referenced this issue Jul 29, 2020
@Araq Araq closed this as completed in d130175 Jul 30, 2020
mildred pushed a commit to mildred/Nim that referenced this issue Jan 11, 2021
* fixes nim-lang#15110
* fixes nim-lang#15096

* prepare varpartitions for cursor inference
* new cursor inference begins to work
* make tests green
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants