-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
algorithm.nim: Use func
#16304
algorithm.nim: Use func
#16304
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, I guess tests fail...
2020-12-09T17:51:16.8819350Z /Users/runner/work/1/s/lib/pure/algorithm.nim(434, 6) Error: 'sorted' can have side effects
2020-12-09T17:51:16.8836130Z an object reachable from 'a' is potentially mutated
2020-12-09T17:51:16.8863760Z /Users/runner/work/1/s/lib/pure/algorithm.nim(454, 8) the mutation is here
Yeah, it's due to: Nim/tests/views/tcan_compile_nim.nim Line 2 in f344a70
Because this: bin/nim c --experimental:strictFuncs compiler/nim.nim doesn't work with Let's see what happens with |
I've reduced the previous CI failure to #16305 |
No, it was an oversight by me. We don't want that. Callbacks can have effects. |
That's the crux of #16303 and IMO we do in fact want that.
|
This commit changes the funcs that take a `proc` parameter back to procs. This reverts some of commit 6f57eba: sequtils.nim: Use `func` (nim-lang#16293) See also: - nim-lang#16303 - nim-lang#16304
I see. I should've asked explicitly in the sequtils PR description, rather than this one. Sorry. Questions:
|
This commit changes the funcs that take a `proc` parameter back to procs. This reverts some of commit 6f57eba: sequtils.nim: Use `func` (nim-lang#16293) See also: - nim-lang#16303 - nim-lang#16304
This commit changes the funcs that take a `proc` parameter back to procs. This reverts some of commit 6f57eba: sequtils.nim: Use `func` (nim-lang#16293) See also: - nim-lang#16303 - nim-lang#16304
This commit changes the funcs that take a `proc` parameter back to procs. This reverts some of commit 6f57eba: sequtils.nim: Use `func` (nim-lang#16293) See also: - nim-lang#16303 - nim-lang#16304
If you want to continue with this, the next steps would be:
|
This pull request has been automatically marked as stale because it has not had recent activity. If you think it is still a valid PR, please rebase it on the latest devel; otherwise it will be closed. Thank you for your contributions. |
To confirm: do we want to change a
proc
that has aproc
parameter into afunc
? (See the first commit in this PR).#16293 was merged, so I assume the answer is "yes".
For example, this PR changes
sorted
from aproc
to afunc
. However, the below code will still compile without error even when using--experimental:strictFuncs
.It produces:
I just want to make sure that this is what we want.
See also: #16303