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

fixes #19162; enable strictEffects for v2 #19380

Merged
merged 36 commits into from
Oct 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
02a451d
enable stricteffects
ringabout Jan 17, 2022
946c5da
Merge remote-tracking branch 'upstream/devel' into pr_effects
ringabout Jan 19, 2022
0f9a446
add gcsafe
ringabout Jan 19, 2022
f8f31ab
fix tests
ringabout Jan 20, 2022
fdf82e6
fix ci
ringabout Jan 20, 2022
b0a9ea6
use func
ringabout Jan 20, 2022
4771b78
Merge branch 'devel' into pr_effects
ringabout Mar 9, 2022
687b92b
Merge branch 'devel' into pr_effects
ringabout Apr 12, 2022
12905be
Merge branch 'devel' into pr_effects
ringabout Apr 25, 2022
a55e2d0
Merge branch 'devel' into pr_effects
ringabout Aug 8, 2022
f202b8b
Merge branch 'devel' into pr_effects
ringabout Aug 25, 2022
f96dce0
Merge branch 'devel' into pr_effects
ringabout Sep 23, 2022
792771b
Merge branch 'devel' into pr_effects
ringabout Oct 6, 2022
9fcbf62
fixes pegs tests
ringabout Oct 6, 2022
5842184
explicitly mark repr related procs with noSideEffect
ringabout Oct 7, 2022
a6b5300
Merge branch 'devel' into pr_effects
ringabout Oct 7, 2022
086ff44
Merge branch 'devel' into pr_effects
ringabout Oct 10, 2022
75ec9b5
Merge branch 'devel' into pr_effects
ringabout Oct 14, 2022
617d48c
add nimLegacyEffects
ringabout Oct 14, 2022
c23e510
Merge remote-tracking branch 'upstream/devel' into pr/19380
ringabout Oct 14, 2022
29f1d71
change URL
ringabout Oct 14, 2022
9194f7c
fixes docopt
ringabout Oct 14, 2022
86470a7
add `raises: []` to repr
ringabout Oct 14, 2022
4a1c8a6
fixes weave
ringabout Oct 14, 2022
1b7d473
fixes nimyaml
ringabout Oct 14, 2022
94fa914
mistake
ringabout Oct 14, 2022
e5b06b9
fixes glob
ringabout Oct 14, 2022
b858391
fixes parsetoml
ringabout Oct 14, 2022
afda060
Update testament/important_packages.nim
ringabout Oct 14, 2022
2ebcfff
Apply suggestions from code review
ringabout Oct 14, 2022
57253c9
Update testament/important_packages.nim
ringabout Oct 14, 2022
daf21b4
fixes more
ringabout Oct 15, 2022
bd1b7a9
more no raises
ringabout Oct 15, 2022
ca5cda0
change URL
ringabout Oct 15, 2022
204f51a
try repr
ringabout Oct 15, 2022
0f771a2
add legacy:laxEffects
ringabout Oct 15, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@
- ORC is now the default memory management strategy. Use
`--mm:refc` for a transition period.

- `strictEffects` are no longer experimental.
Use `legacy:laxEffects` to keep backward compatibility.

- The `gorge`/`staticExec` calls will now return a descriptive message in the output
if the execution fails for whatever reason. To get back legacy behaviour use `-d:nimLegacyGorgeErrors`.

Expand Down
5 changes: 0 additions & 5 deletions compiler/nim.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@ define:useStdoutAsStdmsg
warning[ObservableStores]: off
@end

@if nimHasEffectsOf:
experimental:strictEffects
warningAsError:Effect:on
@end

@if nimHasWarningAsError:
warningAsError:GcUnsafe2:on
@end
2 changes: 2 additions & 0 deletions compiler/options.nim
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ type
## Historically and especially in version 1.0.0 of the language
## conversions to unsigned numbers were checked. In 1.0.4 they
## are not anymore.
laxEffects
## Lax effects system prior to Nim 2.0.

SymbolFilesOption* = enum
disabledSf, writeOnlySf, readOnlySf, v2Sf, stressTest
Expand Down
8 changes: 4 additions & 4 deletions compiler/sempass2.nim
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ proc isIndirectCall(tracked: PEffects; n: PNode): bool =
if n.kind != nkSym:
result = true
elif n.sym.kind == skParam:
if strictEffects in tracked.c.features:
if laxEffects notin tracked.c.config.legacyFeatures:
if tracked.owner == n.sym.owner and sfEffectsDelayed in n.sym.flags:
result = false # it is not a harmful call
else:
Expand Down Expand Up @@ -581,7 +581,7 @@ proc isOwnedProcVar(tracked: PEffects; n: PNode): bool =
tracked.owner == n.sym.owner
#if result and sfPolymorphic notin n.sym.flags:
# echo tracked.config $ n.info, " different here!"
if strictEffects in tracked.c.features:
if laxEffects notin tracked.c.config.legacyFeatures:
result = result and sfEffectsDelayed in n.sym.flags

proc isNoEffectList(n: PNode): bool {.inline.} =
Expand All @@ -598,7 +598,7 @@ proc trackOperandForIndirectCall(tracked: PEffects, n: PNode, formals: PType; ar
# assume indirect calls are taken here:
if op != nil and op.kind == tyProc and n.skipConv.kind != nkNilLit and
not isTrival(caller) and
((param != nil and sfEffectsDelayed in param.flags) or strictEffects notin tracked.c.features):
((param != nil and sfEffectsDelayed in param.flags) or laxEffects in tracked.c.config.legacyFeatures):

internalAssert tracked.config, op.n[0].kind == nkEffectList
var effectList = op.n[0]
Expand Down Expand Up @@ -844,7 +844,7 @@ proc trackCall(tracked: PEffects; n: PNode) =
assumeTheWorst(tracked, n, op)
gcsafeAndSideeffectCheck()
else:
if strictEffects in tracked.c.features and a.kind == nkSym and
if laxEffects notin tracked.c.config.legacyFeatures and a.kind == nkSym and
a.sym.kind in routineKinds:
propagateEffects(tracked, n, a.sym)
else:
Expand Down
1 change: 1 addition & 0 deletions config/config.nims
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ when defined(windows) and not defined(booting):
switch("define", "nimRawSetjmp")

switch("define", "nimVersion:" & NimVersion)

4 changes: 1 addition & 3 deletions doc/manual.md
Original file line number Diff line number Diff line change
Expand Up @@ -4952,8 +4952,7 @@ Effect system
=============

**Note**: The rules for effect tracking changed with the release of version
1.6 of the Nim compiler. This section describes the new rules that are activated
via `--experimental:strictEffects`.
1.6 of the Nim compiler.


Exception tracking
Expand Down Expand Up @@ -5073,7 +5072,6 @@ conservative in its effect analysis:

```nim test = "nim c $1" status = 1
{.push warningAsError[Effect]: on.}
{.experimental: "strictEffects".}

import algorithm

Expand Down
2 changes: 1 addition & 1 deletion lib/pure/pegs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ template matchOrParse(mopProc: untyped) =
# procs. For the former, *enter* and *leave* event handler code generators
# are provided which just return *discard*.

proc mopProc(s: string, p: Peg, start: int, c: var Captures): int =
proc mopProc(s: string, p: Peg, start: int, c: var Captures): int {.gcsafe.} =
proc matchBackRef(s: string, p: Peg, start: int, c: var Captures): int =
# Parse handler code must run in an *of* clause of its own for each
# *PegKind*, so we encapsulate the identical clause body for
Expand Down
2 changes: 1 addition & 1 deletion lib/system.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1701,7 +1701,7 @@ proc pop*[T](s: var seq[T]): T {.inline, noSideEffect.} =
result = s[L]
setLen(s, L)

proc `==`*[T: tuple|object](x, y: T): bool =
func `==`*[T: tuple|object](x, y: T): bool =
## Generic `==` operator for tuples that is lifted from the components.
## of `x` and `y`.
for a, b in fields(x, y):
Expand Down
16 changes: 8 additions & 8 deletions lib/system/repr_v2.nim
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ proc repr*(x: bool): string {.magic: "BoolToStr", noSideEffect.}
## repr for a boolean argument. Returns `x`
## converted to the string "false" or "true".

proc repr*(x: char): string {.noSideEffect.} =
proc repr*(x: char): string {.noSideEffect, raises: [].} =
## repr for a character argument. Returns `x`
## converted to an escaped string.
##
Expand All @@ -47,7 +47,7 @@ proc repr*(x: char): string {.noSideEffect.} =
result.add x
result.add '\''

proc repr*(x: string | cstring): string {.noSideEffect.} =
proc repr*(x: string | cstring): string {.noSideEffect, raises: [].} =
## repr for a string argument. Returns `x`
## converted to a quoted and escaped string.
result.add '\"'
Expand All @@ -63,7 +63,7 @@ proc repr*(x: string | cstring): string {.noSideEffect.} =
result.add x[i]
result.add '\"'

proc repr*[Enum: enum](x: Enum): string {.magic: "EnumToStr", noSideEffect.}
proc repr*[Enum: enum](x: Enum): string {.magic: "EnumToStr", noSideEffect, raises: [].}
## repr for an enumeration argument. This works for
## any enumeration type thanks to compiler magic.
##
Expand Down Expand Up @@ -100,7 +100,7 @@ template repr*(x: distinct): string =

template repr*(t: typedesc): string = $t

proc reprObject[T: tuple|object](res: var string, x: T) =
proc reprObject[T: tuple|object](res: var string, x: T) {.noSideEffect, raises: [].} =
res.add '('
var firstElement = true
const isNamed = T is object or isNamedTuple(T)
Expand All @@ -121,7 +121,7 @@ proc reprObject[T: tuple|object](res: var string, x: T) =
res.add(')')


proc repr*[T: tuple|object](x: T): string =
proc repr*[T: tuple|object](x: T): string {.noSideEffect, raises: [].} =
## Generic `repr` operator for tuples that is lifted from the components
## of `x`. Example:
##
Expand All @@ -133,7 +133,7 @@ proc repr*[T: tuple|object](x: T): string =
result = $typeof(x)
reprObject(result, x)

proc repr*[T](x: ref T | ptr T): string =
proc repr*[T](x: ref T | ptr T): string {.noSideEffect, raises: [].} =
if isNil(x): return "nil"
when T is object:
result = $typeof(x)
Expand All @@ -142,7 +142,7 @@ proc repr*[T](x: ref T | ptr T): string =
result = when typeof(x) is ref: "ref " else: "ptr "
result.add repr(x[])

proc collectionToRepr[T](x: T, prefix, separator, suffix: string): string =
proc collectionToRepr[T](x: T, prefix, separator, suffix: string): string {.noSideEffect, raises: [].} =
result = prefix
var firstElement = true
for value in items(x):
Expand Down Expand Up @@ -189,4 +189,4 @@ proc repr*[T](x: openArray[T]): string =
##
## .. code-block:: Nim
## $(@[23, 45].toOpenArray(0, 1)) == "[23, 45]"
collectionToRepr(x, "[", ", ", "]")
collectionToRepr(x, "[", ", ", "]")
2 changes: 1 addition & 1 deletion nimsuggest/sexp.nim
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ macro convertSexp*(x: untyped): untyped =
## `%` for every element.
result = toSexp(x)

proc `==`* (a, b: SexpNode): bool {.noSideEffect.} =
func `==`* (a, b: SexpNode): bool =
## Check two nodes for equality
if a.isNil:
if b.isNil: return true
Expand Down
16 changes: 8 additions & 8 deletions testament/important_packages.nim
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,15 @@ pkg "criterion", allowFailure = true # pending https://github.com/disruptek/crit
pkg "datamancer"
pkg "dashing", "nim c tests/functional.nim"
pkg "delaunay", url = "https://github.com/nim-lang/DelaunayNim", useHead = true
pkg "docopt"
pkg "docopt", url = "https://github.com/nim-lang/docopt.nim", useHead = true
pkg "easygl", "nim c -o:egl -r src/easygl.nim", "https://github.com/jackmott/easygl"
pkg "elvis"
pkg "fidget"
pkg "fragments", "nim c -r fragments/dsl.nim", allowFailure = true # pending https://github.com/nim-lang/packages/issues/2115
pkg "fusion"
pkg "gara"
pkg "glob"
pkg "ggplotnim", "nim c -d:noCairo -r tests/tests.nim"
pkg "glob", url = "https://github.com/nim-lang/glob", useHead = true
pkg "ggplotnim", "nim c -d:noCairo -r tests/tests.nim", url = "https://github.com/nim-lang/ggplotnim", useHead = true
pkg "gittyup", "nimble test", "https://github.com/disruptek/gittyup", allowFailure = true
pkg "gnuplot", "nim c gnuplot.nim"
# pkg "gram", "nim c -r --gc:arc --define:danger tests/test.nim", "https://github.com/disruptek/gram"
Expand Down Expand Up @@ -109,7 +109,7 @@ pkg "nimongo", "nimble test_ci", allowFailure = true
pkg "nimph", "nimble test", "https://github.com/disruptek/nimph", allowFailure = true
pkg "nimPNG", useHead = true
pkg "nimpy", "nim c -r tests/nimfrompy.nim"
pkg "nimquery"
pkg "nimquery", url = "https://github.com/nim-lang/nimquery", useHead = true
pkg "nimsl"
pkg "nimsvg"
pkg "nimterop", "nimble minitest"
Expand All @@ -121,7 +121,7 @@ pkg "npeg", "nimble testarc"
pkg "numericalnim", "nimble nimCI"
pkg "optionsutils"
pkg "ormin", "nim c -o:orminn ormin.nim"
pkg "parsetoml"
pkg "parsetoml", url = "https://github.com/nim-lang/parsetoml", useHead = true
pkg "patty"
pkg "pixie"
pkg "plotly", "nim c examples/all.nim"
Expand All @@ -146,7 +146,7 @@ pkg "strslice"
pkg "strunicode", "nim c -r --mm:refc src/strunicode.nim"
pkg "supersnappy"
pkg "synthesis"
pkg "telebot", "nim c -o:tbot -r src/telebot.nim"
pkg "telebot", "nim c -o:tbot -r src/telebot.nim", url = "https://github.com/nim-lang/telebot.nim", useHead = true
pkg "tempdir"
pkg "templates"
pkg "tensordsl", "nim c -r --mm:refc tests/tests.nim", "https://[email protected]/krux02/tensordslnim.git"
Expand All @@ -158,11 +158,11 @@ pkg "tiny_sqlite"
pkg "unicodedb", "nim c -d:release -r tests/tests.nim"
pkg "unicodeplus", "nim c -d:release -r tests/tests.nim"
pkg "unpack"
pkg "weave", "nimble install -y cligen synthesis;nimble test_gc_arc"
pkg "weave", "nimble install -y cligen synthesis;nimble test_gc_arc", url = "https://github.com/nim-lang/weave", useHead = true
pkg "websocket", "nim c websocket.nim"
pkg "winim", "nim c winim.nim"
pkg "with"
pkg "ws", allowFailure = true
pkg "yaml", "nim c -r test/tserialization.nim"
pkg "yaml", "nim c -r test/tserialization.nim", url = "https://github.com/nim-lang/NimYAML", useHead = true
pkg "zero_functional", "nim c -r -d:nimNoLentIterators test.nim"
pkg "zippy"
2 changes: 1 addition & 1 deletion tests/closure/tnested.nim
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ py
py
px
6
proc (){.closure, gcsafe.}
proc (){.closure, noSideEffect, gcsafe.}
'''
"""

Expand Down
2 changes: 1 addition & 1 deletion tests/effects/teffects6.nim
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ createMenuItem(s, "Go to definition...",
)


proc noRaise(x: proc()) {.raises: [].} =
proc noRaise(x: proc()) {.raises: [], effectsOf: x.} =
# unknown call that might raise anything, but valid:
x()

Expand Down
4 changes: 2 additions & 2 deletions tests/effects/tgcsafe.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
discard """
errormsg: "'mainUnsafe' is not GC-safe"
errormsg: "'mainUnsafe' is not GC-safe as it performs an indirect call here"
line: 26
cmd: "nim $target --hints:on --threads:on $options $file"
"""
Expand All @@ -13,7 +13,7 @@ proc myproc(i: int) {.gcsafe.} =
if isNil(global_proc):
return

proc mymap(x: proc ()) =
proc mymap(x: proc ()) {.effectsOf: x.} =
x()

var
Expand Down
2 changes: 1 addition & 1 deletion tests/effects/tnosideeffect.nim
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
block: # `.noSideEffect`
func foo(bar: proc(): int): int = bar()
func foo(bar: proc(): int): int {.effectsOf: bar.} = bar()
var count = 0
proc fn1(): int = 1
proc fn2(): int = (count.inc; count)
Expand Down
6 changes: 3 additions & 3 deletions tests/stdlib/tpegs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,9 @@ block:

block:
var
pStack: seq[string] = @[]
valStack: seq[float] = @[]
opStack = ""
pStack {.threadvar.}: seq[string]
valStack {.threadvar.}: seq[float]
opStack {.threadvar.}: string
let
parseArithExpr = pegAst.eventParser:
pkNonTerminal:
Expand Down