Skip to content

Commit

Permalink
Merge #523
Browse files Browse the repository at this point in the history 
523: perform `.closure` inference in `sempass2` r=zerbina a=zerbina

## Summary
While the `lambdalifting` transformation is responsible for figuring
out what inner procedures capture the content of which memory
locations, semantic analysis already needs to know whether an inner
procedure accesses its environment, as whether it does affects its
calling convention (which is part of a procedure's type).

`semCaptureSym` was responsible for detecting if usage of a symbol
required a capture and for changing all relevant enclosing procedures
between the to use the `.closure` calling convention, but the
implementation had multiple issues.

Instead, whether a procedure captures something is now detected during
`sempass2`. Setting the calling convention is performed there too now.
This fixes:
- inner procedures using the `.closure` calling convention even if
  explicitly specified otherwise
- tentative semantic analysis (such as `compiles` or analysis of
  argument expression during overload resolution) having side-effects
  because of `semCaptureSym`
- symbols resulting from typed template/macro parameter expansion
  weren't considered by `semCaptureSym`, which in some cases caused
  procedures to not be updated to use the `.closure` calling convention,
  which then caused errors in the code-generator or backend compiler

Detecting illegal captures now also happens in `sempass2` (instead of
in `lambdalifting`), which means that they're now *always* reported and
not only for alive code.

A hint is now reported if a procedure uses the `.closure` calling
convention but doesn't capture anything.

## Details

- move illegal capture detection from `lambdalifting` into `sempass2`
- detect whether a procedure captures something during `sempass2`
  instead of in `lambdalifting`
- remove `semCaptureSym` and its usages
- remove the `tfCapturesEnv` type flag. It's use case was detecting
  top-level closure procedures (can only be achieved via anonymous
  procedures), but a hidden environment parameter is added for them
  now, instead
- report a dedicated error when attempting to capture a run-time value
  in a compile-time context

Changes to `lambdalifting`:
- remove the unused "in container" analysis
- remove all logic for permanently changing a procedure's calling
  convention
- run the pass on all non-`.closure` procedures (including top-level
  closure iterators) instead of only on top-level procedures. While
  this doesn't affect the result (all inner procedures that capture
  something are still lifted), it means less recursion depth and a
  smaller working set for each run, as well as less calls to
  `transformBody` (which translates to less work).

### Misc

- change `rsemIllegalMemoryCapture` to use the `sym` field instead of
  `symbols`
- move `isOwnedBy` from `vmgen.nim` to `ast_query.nim`



Co-authored-by: zerbina <[email protected]>
  • Loading branch information
@bors @zerbina
bors[bot] and zerbina authored Feb 9, 2023
2 parents f3b3bde + 9929164 commit f175f5f
Show file tree
Hide file tree
Showing 17 changed files with 414 additions and 129 deletions.
8 changes: 8 additions & 0 deletions compiler/ast/ast_query.nim
View file
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,14 @@ proc skipGenericOwner*(s: PSym): PSym =
else:
s.owner

func isOwnedBy*(a, b: PSym): bool =
## Tests if `b` is the transitive owner of `a`, returns true if `a` got
## owned! :)
var a = a.owner
while a != nil and a.kind != skModule:
if a == b: return true
a = a.owner

proc originatingModule*(s: PSym): PSym =
result = s.owner
while result.kind != skModule: result = result.owner
Expand Down
1 change: 0 additions & 1 deletion compiler/ast/ast_types.nim
View file
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,6 @@ type
## concrete type (lastSon becomes the concrete type)
tfRetType, ## marks return types in proc (used to detect type classes
## used as return types for return type inference)
tfCapturesEnv, ## whether proc really captures some environment
tfByCopy, ## pass object/tuple by copy (C backend)
tfByRef, ## pass object/tuple by reference (C backend)
tfIterator, ## type is really an iterator, not a tyProc
Expand Down
3 changes: 2 additions & 1 deletion compiler/ast/report_enums.nim
View file
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,7 @@ type
rsemNotABaseMethod
rsemIllegalCallconvCapture
rsemIllegalMemoryCapture
rsemIllegalCompTimeCapture
rsemIgnoreInvalidForLoop
rsemMissingGenericParamsForTemplate
rsemMisplacedMagicType
Expand Down Expand Up @@ -842,6 +843,7 @@ type
rsemXCannotRaiseY = "XCannotRaiseY"
rsemConvToBaseNotNeeded = "ConvToBaseNotNeeded"
rsemConvFromXtoItselfNotNeeded = "ConvFromXtoItselfNotNeeded"
rsemClosureWithoutEnv = "ClosureWithoutEnv"

rsemProcessing = "Processing" ## Processing module
rsemProcessingStmt = "ProcessingStmt" ## Processing toplevel statement
Expand Down Expand Up @@ -1146,7 +1148,6 @@ const
rsemUnexpectedPragmaInDefinitionOf,
rsemDoubleCompletionOf,

rsemIllegalMemoryCapture,
rsemOverrideSafetyMismatch,
rsemOverrideLockMismatch
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/backend/cgen.nim
View file
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,7 @@ proc closeNamespaceNim(): Rope =
result.add("}\L")

proc closureSetup(p: BProc, prc: PSym) =
if tfCapturesEnv notin prc.typ.flags: return
if prc.typ.callConv != ccClosure: return
# prc.ast[paramsPos].last contains the type we're after:
var ls = lastSon(prc.ast[paramsPos])
p.config.internalAssert(ls.kind == nkSym, prc.info, "closure generation failed")
Expand Down
11 changes: 8 additions & 3 deletions compiler/front/cli_reporter.nim
View file
Original file line number Diff line number Diff line change
Expand Up @@ -458,12 +458,15 @@ proc reportBody*(conf: ConfigRef, r: SemReport): string =
result = "duplicate case label"

of rsemIllegalMemoryCapture:
let s = r.symbols[0]
let s = r.sym
result = (
"'$1' is of type <$2> which cannot be captured as it would violate memory" &
" safety, declared here: $3; using '-d:nimNoLentIterators' helps in some cases"
) % [s.name.s, typeToString(s.typ), conf $ s.info]

of rsemIllegalCompTimeCapture:
result = "'$1' is inaccesible because run-time values cannot be" &
" captured in a compile-time context"
result = result % [r.sym.name.s]
of rsemUnavailableTypeBound:
result.add(
"'",
Expand Down Expand Up @@ -1414,7 +1417,9 @@ proc reportBody*(conf: ConfigRef, r: SemReport): string =

of rsemConvFromXtoItselfNotNeeded:
result = "conversion from $1 to itself is pointless" % r.typ.render

of rsemClosureWithoutEnv:
result = "procedure explicitly uses the 'closure' calling convention," &
" but it doesn't capture anything"
of rsemIllegalConversion:
result = "illegal conversion from '$1' to '$2'" % [
r.actualType.render, r.formalType.render
Expand Down
1 change: 0 additions & 1 deletion compiler/plugins/itersgen.nim
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ proc iterToProcImpl*(c: PContext, n: PNode): PNode =

let prc = newSym(skProc, n[3].ident, nextSymId c.idgen, iter.sym.owner, iter.sym.info)
prc.typ = copyType(iter.sym.typ, nextTypeId c.idgen, prc)
excl prc.typ.flags, tfCapturesEnv
prc.typ.n.add newSymNode(getEnvParam(iter.sym))
prc.typ.rawAddSon t
let orig = iter.sym.ast
Expand Down
163 changes: 62 additions & 101 deletions compiler/sem/lambdalifting.nim
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,14 @@ import
msgs
],
compiler/sem/[
typeallowed,
liftdestructors,
transf,
lowerings
]

# xxx: reports are a code smell meaning data types are misplaced
from compiler/ast/reports_sem import SemReport,
reportSem,
reportSymbols
reportSem
from compiler/ast/report_enums import ReportKind

discard """
Expand Down Expand Up @@ -192,7 +190,6 @@ proc addHiddenParam(routine: PSym, param: PSym) =
# some nkEffect node:
param.position = routine.typ.n.len-1
params.add newSymNode(param)
#incl(routine.typ.flags, tfCapturesEnv)
assert sfFromGeneric in param.flags
#echo "produced environment: ", param.id, " for ", routine.id

Expand All @@ -219,9 +216,6 @@ proc interestingVar(s: PSym): bool {.inline.} =
sfGlobal notin s.flags and
s.typ.kind notin {tyStatic, tyTypeDesc}

proc illegalCapture(s: PSym): bool {.inline.} =
result = classifyViewType(s.typ) != noView or s.kind == skResult

proc isInnerProc(s: PSym): bool =
if s.kind in {skProc, skFunc, skMethod, skConverter, skIterator} and s.magic == mNone:
result = s.skipGenericOwner.kind in routineKinds
Expand Down Expand Up @@ -310,23 +304,6 @@ proc freshVarForClosureIter*(g: ModuleGraph; s: PSym; idgen: IdGenerator; owner:

# ------------------ new stuff -------------------------------------------

proc markAsClosure(g: ModuleGraph; owner: PSym; n: PNode) =
let s = n.sym
if illegalCapture(s):
localReport(g.config, n.info, reportSymbols(
rsemIllegalMemoryCapture, @[s, owner]))

elif not (
owner.typ.callConv == ccClosure or
owner.typ.callConv == ccNimCall and
tfExplicitCallConv notin owner.typ.flags
):
localReport(g.config, n.info, reportSymbols(
rsemIllegalCallconvCapture, @[s, owner]))

incl(owner.typ.flags, tfCapturesEnv)
owner.typ.callConv = ccClosure

type
DetectionPass = object
processed, capturedVars: IntSet
Expand Down Expand Up @@ -428,6 +405,7 @@ Consider:
"""

proc addClosureParam(c: var DetectionPass; fn: PSym; info: TLineInfo) =
assert fn.typ.callConv == ccClosure
var cp = getEnvParam(fn)
let owner = if fn.kind == skIterator: fn else: fn.skipGenericOwner
let t = c.getEnvTypeForOwner(owner, info)
Expand All @@ -443,20 +421,24 @@ proc addClosureParam(c: var DetectionPass; fn: PSym; info: TLineInfo) =
proc detectCapturedVars(n: PNode; owner: PSym; c: var DetectionPass) =
case n.kind
of nkSym:
let s = n.sym
if s.kind in {skProc, skFunc, skMethod, skConverter, skIterator} and
s.typ != nil and s.typ.callConv == ccClosure:
# this handles the case that the inner proc was declared as
# .closure but does not actually capture anything:
addClosureParam(c, s, n.info)
let
s = n.sym
innerProc = isInnerProc(s)

if innerProc and s.typ.callConv == ccClosure:
# something that uses an environment parameter (even if it doesn't
# capture anything) is used that takes a closure is used -> the
# transformation pass is required
c.somethingToDo = true

let innerProc = isInnerProc(s)
if innerProc:
if s.isIterator: c.somethingToDo = true
if not c.processed.containsOrIncl(s.id):
# add the hidden parameter to it and process the inner procedure
addClosureParam(c, s, n.info)
# note that during ``transformBody``, the lambda lifting pass is run
# for the procedure
let body = transformBody(c.graph, c.idgen, s, cache = true)
detectCapturedVars(body, s, c)

let ow = s.skipGenericOwner
if ow == owner:
if owner.isIterator:
Expand All @@ -473,26 +455,17 @@ proc detectCapturedVars(n: PNode; owner: PSym; c: var DetectionPass) =
addField(obj, s, c.graph.cache, c.idgen)
# direct or indirect dependency:
elif (innerProc and s.typ.callConv == ccClosure) or interestingVar(s):
discard """
proc outer() =
var x: int
proc inner() =
proc innerInner() =
echo x
innerInner()
inner()
# inner() takes a closure too!
"""
# mark 'owner' as taking a closure:
# an interesting entity that's not owned by the current `owner` (and
# thus needs to be captured in some form)
c.somethingToDo = true
markAsClosure(c.graph, owner, n)
addClosureParam(c, owner, n.info)
#echo "capturing ", n.info
# variable 's' is actually captured:

if interestingVar(s) and not c.capturedVars.containsOrIncl(s.id):
# we haven't seen the entity yet. Remember it and add a field for it
# into the environment object
let obj = c.getEnvTypeForOwner(ow, n.info).skipTypes({tyRef, tyPtr})
#getHiddenParam(owner).typ.skipTypes({tyRef, tyPtr})
addField(obj, s, c.graph.cache, c.idgen)

# create required upFields:
var w = owner.skipGenericOwner
if isInnerProc(w) or owner.isIterator:
Expand All @@ -510,7 +483,6 @@ proc detectCapturedVars(n: PNode; owner: PSym; c: var DetectionPass) =
"""
let up = w.skipGenericOwner
#echo "up for ", w.name.s, " up ", up.name.s
markAsClosure(c.graph, w, n)
addClosureParam(c, w, n.info) # , ow
createUpField(c, w, up, n.info)
w = up
Expand All @@ -532,7 +504,6 @@ type
LiftingPass = object
processed: IntSet
envVars: Table[int, PNode]
inContainer: int

proc initLiftingPass(fn: PSym): LiftingPass =
result.processed = initIntSet()
Expand Down Expand Up @@ -708,18 +679,15 @@ proc liftCapturedVars(n: PNode; owner: PSym; d: var DetectionPass;
of nkSym:
let s = n.sym
if isInnerProc(s):
if not c.processed.containsOrIncl(s.id):
if s.typ.callConv == ccClosure and not c.processed.containsOrIncl(s.id):
#if s.name.s == "temp":
# echo renderTree(s.getBody, {renderIds})
let oldInContainer = c.inContainer
c.inContainer = 0
var body = transformBody(d.graph, d.idgen, s, cache = false)
body = liftCapturedVars(body, s, d, c)
if c.envVars.getOrDefault(s.id).isNil:
s.transformedBody = body
else:
s.transformedBody = newTree(nkStmtList, rawClosureCreation(s, d, c, n.info), body)
c.inContainer = oldInContainer

if s.typ.callConv == ccClosure:
result = symToClosure(n, owner, d, c)
Expand All @@ -746,12 +714,9 @@ proc liftCapturedVars(n: PNode; owner: PSym; d: var DetectionPass;
n[1] = x[1]
of nkLambdaKinds, nkIteratorDef:
if n.typ != nil and n[namePos].kind == nkSym:
let oldInContainer = c.inContainer
c.inContainer = 0
let m = newSymNode(n[namePos].sym)
m.typ = n.typ
result = liftCapturedVars(m, owner, d, c)
c.inContainer = oldInContainer
of nkHiddenStdConv:
if n.len == 2:
n[1] = liftCapturedVars(n[1], owner, d, c)
Expand All @@ -773,51 +738,11 @@ proc liftCapturedVars(n: PNode; owner: PSym; d: var DetectionPass;
n[1] = liftCapturedVars(n[1], owner, d, c)
return

let inContainer = n.kind in {nkObjConstr, nkBracket}
if inContainer: inc c.inContainer
for i in 0..<n.len:
n[i] = liftCapturedVars(n[i], owner, d, c)
if inContainer: dec c.inContainer

# ------------------ old stuff -------------------------------------------

proc semCaptureSym*(s, owner: PSym) =
discard """
proc outer() =
var x: int
proc inner() =
proc innerInner() =
echo x
innerInner()
inner()
# inner() takes a closure too!
"""
proc propagateClosure(start, last: PSym) =
var o = start
while o != nil and o.kind != skModule:
if o == last: break
o.typ.callConv = ccClosure
o = o.skipGenericOwner

if interestingVar(s) and s.kind != skResult:
if owner.typ != nil and not isGenericRoutine(owner):
# XXX: is this really safe?
# if we capture a var from another generic routine,
# it won't be consider captured.
var o = owner.skipGenericOwner
while o != nil and o.kind != skModule:
if s.owner == o:
if owner.typ.callConv == ccClosure or owner.kind == skIterator or
owner.typ.callConv == ccNimCall and tfExplicitCallConv notin owner.typ.flags:
owner.typ.callConv = ccClosure
propagateClosure(owner.skipGenericOwner, s.owner)
else:
discard "do not produce an error here, but later"
#echo "computing .closure for ", owner.name.s, " because of ", s.name.s
o = o.skipGenericOwner
# since the analysis is not entirely correct, we don't set 'tfCapturesEnv'
# here

proc liftIterToProc*(g: ModuleGraph; fn: PSym; body: PNode; ptrType: PType;
idgen: IdGenerator): PNode =
var d = initDetectionPass(g, fn, idgen)
Expand All @@ -833,6 +758,23 @@ proc liftIterToProc*(g: ModuleGraph; fn: PSym; body: PNode; ptrType: PType;
fn.transitionRoutineSymKind(oldKind)
fn.typ.callConv = oldCC

func dontLift(prc: PSym): bool {.inline.} =
## Returns whether the lifting pass should not run for the given procedure
## `prc`. The pass needs to only be run for routines that don't capture
## anything
if prc.typ != nil:
prc.typ.callConv == ccClosure and
# don't run the pass for closure routines (they might capture
# something) ...
(prc.kind != skIterator or prc.skipGenericOwner.kind != skModule)
# ... except if it's a top-level closure iterator (they can't capture
# anything)
else:
# some compiler-inserted magic with no type information -> don't run the
# pass
true


proc liftLambdas*(g: ModuleGraph; fn: PSym, body: PNode; tooEarly: var bool;
idgen: IdGenerator): PNode =
# XXX backend == backendJs does not suffice! The compiletime stuff needs
Expand All @@ -841,19 +783,38 @@ proc liftLambdas*(g: ModuleGraph; fn: PSym, body: PNode; tooEarly: var bool;
# However we can do lifting for the stuff which is *only* compiletime.
let isCompileTime = sfCompileTime in fn.flags or fn.kind == skMacro

if body.kind == nkEmpty or (
g.config.backend == backendJs and not isCompileTime) or
fn.skipGenericOwner.kind != skModule:
if body.kind == nkEmpty or # forward declaration
(g.config.backend == backendJs and not isCompileTime) or
dontLift(fn):
# HACK: anonymous procedures defined outside of procedures (i.e. owned by
# a module), need a hidden environment parameter too, even though
# `nil` will always be passed. We add the parameter here, but
# non-nested anonymous closure procedures should just be disallowed
# instead
if fn.skipGenericOwner.kind == skModule and
fn.typ != nil and fn.typ.callConv == ccClosure:
# duplicates the code of ``addClosureParam``, but that's okay
let cp = newSym(skParam, getIdent(g.cache, paramName), nextSymId(idgen), fn, fn.info)
incl(cp.flags, sfFromGeneric)

# setup the type:
cp.typ = newType(tyRef, nextTypeId(idgen), fn)
rawAddSon(cp.typ, createEnvObj(g, idgen, fn, fn.info))

addHiddenParam(fn, cp)

# ignore forward declaration:
result = body
tooEarly = true
else:
var d = initDetectionPass(g, fn, idgen)
detectCapturedVars(body, fn, d)
if not d.somethingToDo and fn.isIterator:
# the "lift captures" pass needs to always run either directly or
# indirectly for closure iterators, as it's also responsible for
# lifting locals inside a closure iterator into its environment
addClosureParam(d, fn, body.info)
d.somethingToDo = true

if d.somethingToDo:
var c = initLiftingPass(fn)
result = liftCapturedVars(body, fn, d, c)
Expand Down
Loading

0 comments on commit f175f5f

Please sign in to comment.