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

'lock levels' are deprecated, now a noop #20539

Merged
merged 2 commits into from
Oct 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@
- `macros.getImpl` for `const` symbols now returns the full definition node
(as `nnkConstDef`) rather than the AST of the constant value.

- Lock levels are deprecated, now a noop.

- ORC is now the default memory management strategy. Use
`--mm:refc` for a transition period.

Expand Down
12 changes: 0 additions & 12 deletions compiler/ast.nim
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,6 @@ type
allUsages*: seq[TLineInfo]

TTypeSeq* = seq[PType]
TLockLevel* = distinct int16

TTypeAttachedOp* = enum ## as usual, order is important here
attachedDestructor,
Expand Down Expand Up @@ -963,7 +962,6 @@ type
# -1 means that the size is unkwown
align*: int16 # the type's alignment requirements
paddingAtEnd*: int16 #
lockLevel*: TLockLevel # lock level as required for deadlock checking
loc*: TLoc
typeInst*: PType # for generic instantiations the tyGenericInst that led to this
# type.
Expand Down Expand Up @@ -1499,17 +1497,9 @@ proc newProcNode*(kind: TNodeKind, info: TLineInfo, body: PNode,
pragmas, exceptions, body]

const
UnspecifiedLockLevel* = TLockLevel(-1'i16)
MaxLockLevel* = 1000'i16
UnknownLockLevel* = TLockLevel(1001'i16)
AttachedOpToStr*: array[TTypeAttachedOp, string] = [
"=destroy", "=copy", "=sink", "=trace", "=deepcopy"]

proc `$`*(x: TLockLevel): string =
if x.ord == UnspecifiedLockLevel.ord: result = "<unspecified>"
elif x.ord == UnknownLockLevel.ord: result = "<unknown>"
else: result = $int16(x)

proc `$`*(s: PSym): string =
if s != nil:
result = s.name.s & "@" & $s.id
Expand All @@ -1519,7 +1509,6 @@ proc `$`*(s: PSym): string =
proc newType*(kind: TTypeKind, id: ItemId; owner: PSym): PType =
result = PType(kind: kind, owner: owner, size: defaultSize,
align: defaultAlignment, itemId: id,
lockLevel: UnspecifiedLockLevel,
uniqueId: id)
when false:
if result.itemId.module == 55 and result.itemId.item == 2:
Expand All @@ -1544,7 +1533,6 @@ proc assignType*(dest, src: PType) =
dest.n = src.n
dest.size = src.size
dest.align = src.align
dest.lockLevel = src.lockLevel
# this fixes 'type TLock = TSysLock':
if src.sym != nil:
if dest.sym != nil:
Expand Down
17 changes: 0 additions & 17 deletions compiler/cgmeth.nim
Original file line number Diff line number Diff line change
Expand Up @@ -147,23 +147,6 @@ proc fixupDispatcher(meth, disp: PSym; conf: ConfigRef) =
disp.ast[resultPos].kind == nkEmpty:
disp.ast[resultPos] = copyTree(meth.ast[resultPos])

# The following code works only with lock levels, so we disable
# it when they're not available.
when declared(TLockLevel):
proc `<`(a, b: TLockLevel): bool {.borrow.}
proc `==`(a, b: TLockLevel): bool {.borrow.}
if disp.typ.lockLevel == UnspecifiedLockLevel:
disp.typ.lockLevel = meth.typ.lockLevel
elif meth.typ.lockLevel != UnspecifiedLockLevel and
meth.typ.lockLevel != disp.typ.lockLevel:
message(conf, meth.info, warnLockLevel,
"method has lock level $1, but another method has $2" %
[$meth.typ.lockLevel, $disp.typ.lockLevel])
# XXX The following code silences a duplicate warning in
# checkMethodeffects() in sempass2.nim for now.
if disp.typ.lockLevel < meth.typ.lockLevel:
disp.typ.lockLevel = meth.typ.lockLevel

proc methodDef*(g: ModuleGraph; idgen: IdGenerator; s: PSym) =
var witness: PSym
for i in 0..<g.methods.len:
Expand Down
4 changes: 2 additions & 2 deletions compiler/ic/ic.nim
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ proc storeType(t: PType; c: var PackedEncoder; m: var PackedModule): PackedItemI

var p = PackedType(kind: t.kind, flags: t.flags, callConv: t.callConv,
size: t.size, align: t.align, nonUniqueId: t.itemId.item,
paddingAtEnd: t.paddingAtEnd, lockLevel: t.lockLevel)
paddingAtEnd: t.paddingAtEnd)
storeNode(p, t, n)
p.typeInst = t.typeInst.storeType(c, m)
for kid in items t.sons:
Expand Down Expand Up @@ -891,7 +891,7 @@ proc typeHeaderFromPacked(c: var PackedDecoder; g: var PackedModuleGraph;
t: PackedType; si, item: int32): PType =
result = PType(itemId: ItemId(module: si, item: t.nonUniqueId), kind: t.kind,
flags: t.flags, size: t.size, align: t.align,
paddingAtEnd: t.paddingAtEnd, lockLevel: t.lockLevel,
paddingAtEnd: t.paddingAtEnd,
uniqueId: ItemId(module: si, item: item),
callConv: t.callConv)

Expand Down
1 change: 0 additions & 1 deletion compiler/ic/packed_ast.nim
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ type
size*: BiggestInt
align*: int16
paddingAtEnd*: int16
lockLevel*: TLockLevel # lock level as required for deadlock checking
# not serialized: loc*: TLoc because it is backend-specific
typeInst*: PackedItemId
nonUniqueId*: int32
Expand Down
5 changes: 3 additions & 2 deletions compiler/lineinfos.nim
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ type
warnUnreachableElse = "UnreachableElse", warnUnreachableCode = "UnreachableCode",
warnStaticIndexCheck = "IndexCheck", warnGcUnsafe = "GcUnsafe", warnGcUnsafe2 = "GcUnsafe2",
warnUninit = "Uninit", warnGcMem = "GcMem", warnDestructor = "Destructor",
warnLockLevel = "LockLevel", warnResultShadowed = "ResultShadowed",
warnLockLevel = "LockLevel", # deadcode
warnResultShadowed = "ResultShadowed",
warnInconsistentSpacing = "Spacing", warnCaseTransition = "CaseTransition",
warnCycleCreated = "CycleCreated", warnObservableStores = "ObservableStores",
warnStrictNotNil = "StrictNotNil",
Expand Down Expand Up @@ -161,7 +162,7 @@ const
warnUninit: "use explicit initialization of '$1' for clarity",
warnGcMem: "'$1' uses GC'ed memory",
warnDestructor: "usage of a type with a destructor in a non destructible context. This will become a compile time error in the future.",
warnLockLevel: "$1",
warnLockLevel: "$1", # deadcode
warnResultShadowed: "Special variable 'result' is shadowed.",
warnInconsistentSpacing: "Number of spaces around '$#' is not consistent",
warnCaseTransition: "Potential object case transition, instantiate new object instead",
Expand Down
1 change: 0 additions & 1 deletion compiler/main.nim
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,6 @@ proc mainCommand*(graph: ModuleGraph) =
of cmdDoc0: docLikeCmd commandDoc(cache, conf)
of cmdDoc:
docLikeCmd():
conf.setNoteDefaults(warnLockLevel, false) # issue #13218
conf.setNoteDefaults(warnRstRedefinitionOfLabel, false) # issue #13218
# because currently generates lots of false positives due to conflation
# of labels links in doc comments, e.g. for random.rand:
Expand Down
19 changes: 1 addition & 18 deletions compiler/pragmas.nim
Original file line number Diff line number Diff line change
Expand Up @@ -684,23 +684,6 @@ proc pragmaLockStmt(c: PContext; it: PNode) =
for i in 0..<n.len:
n[i] = c.semExpr(c, n[i])

proc pragmaLocks(c: PContext, it: PNode): TLockLevel =
if it.kind notin nkPragmaCallKinds or it.len != 2:
invalidPragma(c, it)
else:
case it[1].kind
of nkStrLit, nkRStrLit, nkTripleStrLit:
if it[1].strVal == "unknown":
result = UnknownLockLevel
else:
localError(c.config, it[1].info, "invalid string literal for locks pragma (only allowed string is \"unknown\")")
else:
let x = expectIntLit(c, it)
if x < 0 or x > MaxLockLevel:
localError(c.config, it[1].info, "integer must be within 0.." & $MaxLockLevel)
else:
result = TLockLevel(x)

proc typeBorrow(c: PContext; sym: PSym, n: PNode) =
if n.kind in nkPragmaCallKinds and n.len == 2:
let it = n[1]
Expand Down Expand Up @@ -1196,7 +1179,7 @@ proc singlePragma(c: PContext, sym: PSym, n: PNode, i: var int,
of wLocks:
if sym == nil: pragmaLockStmt(c, it)
elif sym.typ == nil: invalidPragma(c, it)
else: sym.typ.lockLevel = pragmaLocks(c, it)
else: warningDeprecated(c.config, n.info, "'Lock levels' are deprecated, now a noop")
of wBitsize:
if sym == nil or sym.kind != skField:
invalidPragma(c, it)
Expand Down
3 changes: 0 additions & 3 deletions compiler/semcall.nim
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,6 @@ proc effectProblem(f, a: PType; result: var string; c: PContext) =
of efTagsUnknown:
result.add "\n The `.tags` requirements differ. Annotate the " &
"proc with {.tags: [].} to get extended error information."
of efLockLevelsDiffer:
result.add "\n The `.locks` requirements differ. Annotate the " &
"proc with {.locks: 0.} to get extended error information."
of efEffectsDelayed:
result.add "\n The `.effectsOf` annotations differ."
of efTagsIllegal:
Expand Down
78 changes: 1 addition & 77 deletions compiler/sempass2.nim
Original file line number Diff line number Diff line change
Expand Up @@ -77,19 +77,13 @@ type
gcUnsafe, isRecursive, isTopLevel, hasSideEffect, inEnforcedGcSafe: bool
hasDangerousAssign, isInnerProc: bool
inEnforcedNoSideEffects: bool
maxLockLevel, currLockLevel: TLockLevel
currOptions: TOptions
config: ConfigRef
graph: ModuleGraph
c: PContext
escapingParams: IntSet
PEffects = var TEffects

proc `<`(a, b: TLockLevel): bool {.borrow.}
proc `<=`(a, b: TLockLevel): bool {.borrow.}
proc `==`(a, b: TLockLevel): bool {.borrow.}
proc max(a, b: TLockLevel): TLockLevel {.borrow.}

proc createTypeBoundOps(tracked: PEffects, typ: PType; info: TLineInfo) =
if typ == nil: return
when false:
Expand All @@ -107,33 +101,12 @@ proc isLocalVar(a: PEffects, s: PSym): bool =
s.typ != nil and (s.kind in {skVar, skResult} or (s.kind == skParam and isOutParam(s.typ))) and
sfGlobal notin s.flags and s.owner == a.owner

proc getLockLevel(t: PType): TLockLevel =
var t = t
# tyGenericInst(TLock {tyGenericBody}, tyStatic, tyObject):
if t.kind == tyGenericInst and t.len == 3: t = t[1]
if t.kind == tyStatic and t.n != nil and t.n.kind in {nkCharLit..nkInt64Lit}:
result = t.n.intVal.TLockLevel

proc lockLocations(a: PEffects; pragma: PNode) =
if pragma.kind != nkExprColonExpr:
localError(a.config, pragma.info, "locks pragma without argument")
return
var firstLL = TLockLevel(-1'i16)
for x in pragma[1]:
let thisLL = getLockLevel(x.typ)
if thisLL != 0.TLockLevel:
if thisLL < 0.TLockLevel or thisLL > MaxLockLevel.TLockLevel:
localError(a.config, x.info, "invalid lock level: " & $thisLL)
elif firstLL < 0.TLockLevel: firstLL = thisLL
elif firstLL != thisLL:
localError(a.config, x.info,
"multi-lock requires the same static lock level for every operand")
a.maxLockLevel = max(a.maxLockLevel, firstLL)
a.locked.add x
if firstLL >= 0.TLockLevel and firstLL != a.currLockLevel:
if a.currLockLevel > 0.TLockLevel and a.currLockLevel <= firstLL:
localError(a.config, pragma.info, "invalid nested locking")
a.currLockLevel = firstLL

proc guardGlobal(a: PEffects; n: PNode; guard: PSym) =
# check whether the corresponding lock is held:
Expand Down Expand Up @@ -438,8 +411,6 @@ proc listEffects(a: PEffects) =
for e in items(a.exc): message(a.config, e.info, hintUser, typeToString(e.typ))
for e in items(a.tags): message(a.config, e.info, hintUser, typeToString(e.typ))
for e in items(a.forbids): message(a.config, e.info, hintUser, typeToString(e.typ))
#if a.maxLockLevel != 0:
# message(e.info, hintUser, "lockLevel: " & a.maxLockLevel)

proc catches(tracked: PEffects, e: PType) =
let e = skipTypes(e, skipPtrs)
Expand Down Expand Up @@ -551,25 +522,6 @@ proc importedFromC(n: PNode): bool =
# when imported from C, we assume GC-safety.
result = n.kind == nkSym and sfImportc in n.sym.flags

proc getLockLevel(s: PSym): TLockLevel =
result = s.typ.lockLevel
if result == UnspecifiedLockLevel:
if {sfImportc, sfNoSideEffect} * s.flags != {} or
tfNoSideEffect in s.typ.flags:
result = 0.TLockLevel
else:
result = UnknownLockLevel
#message(??.config, s.info, warnUser, "FOR THIS " & s.name.s)

proc mergeLockLevels(tracked: PEffects, n: PNode, lockLevel: TLockLevel) =
if lockLevel >= tracked.currLockLevel:
# if in lock section:
if tracked.currLockLevel > 0.TLockLevel:
localError tracked.config, n.info, errGenerated,
"expected lock level < " & $tracked.currLockLevel &
" but got lock level " & $lockLevel
tracked.maxLockLevel = max(tracked.maxLockLevel, lockLevel)

proc propagateEffects(tracked: PEffects, n: PNode, s: PSym) =
let pragma = s.ast[pragmasPos]
let spec = effectSpec(pragma, wRaises)
Expand All @@ -583,7 +535,6 @@ proc propagateEffects(tracked: PEffects, n: PNode, s: PSym) =
markGcUnsafe(tracked, s)
if tfNoSideEffect notin s.typ.flags:
markSideEffect(tracked, s, n.info)
mergeLockLevels(tracked, n, s.getLockLevel)

proc procVarCheck(n: PNode; conf: ConfigRef) =
if n.kind in nkSymChoices:
Expand Down Expand Up @@ -623,11 +574,6 @@ proc notNilCheck(tracked: PEffects, n: PNode, paramType: PType) =
proc assumeTheWorst(tracked: PEffects; n: PNode; op: PType) =
addRaiseEffect(tracked, createRaise(tracked.graph, n), nil)
addTag(tracked, createTag(tracked.graph, n), nil)
let lockLevel = if op.lockLevel == UnspecifiedLockLevel: UnknownLockLevel
else: op.lockLevel
#if lockLevel == UnknownLockLevel:
# message(??.config, n.info, warnUser, "had to assume the worst here")
mergeLockLevels(tracked, n, lockLevel)

proc isOwnedProcVar(tracked: PEffects; n: PNode): bool =
# XXX prove the soundness of this effect system rule
Expand Down Expand Up @@ -885,10 +831,9 @@ proc trackCall(tracked: PEffects; n: PNode) =
if a.kind == nkSym:
if a.sym == tracked.owner: tracked.isRecursive = true
# even for recursive calls we need to check the lock levels (!):
mergeLockLevels(tracked, n, a.sym.getLockLevel)
if sfSideEffect in a.sym.flags: markSideEffect(tracked, a, n.info)
else:
mergeLockLevels(tracked, n, op.lockLevel)
discard
var effectList = op.n[0]
if a.kind == nkSym and a.sym.kind == skMethod:
propagateEffects(tracked, n, a.sym)
Expand Down Expand Up @@ -967,7 +912,6 @@ proc trackCall(tracked: PEffects; n: PNode) =
type
PragmaBlockContext = object
oldLocked: int
oldLockLevel: TLockLevel
enforcedGcSafety, enforceNoSideEffects: bool
oldExc, oldTags, oldForbids: int
exc, tags, forbids: PNode
Expand All @@ -976,7 +920,6 @@ proc createBlockContext(tracked: PEffects): PragmaBlockContext =
var oldForbidsLen = 0
if tracked.forbids != nil: oldForbidsLen = tracked.forbids.len
result = PragmaBlockContext(oldLocked: tracked.locked.len,
oldLockLevel: tracked.currLockLevel,
enforcedGcSafety: false, enforceNoSideEffects: false,
oldExc: tracked.exc.len, oldTags: tracked.tags.len,
oldForbids: oldForbidsLen)
Expand All @@ -989,7 +932,6 @@ proc unapplyBlockContext(tracked: PEffects; bc: PragmaBlockContext) =
if bc.enforcedGcSafety: tracked.inEnforcedGcSafe = false
if bc.enforceNoSideEffects: tracked.inEnforcedNoSideEffects = false
setLen(tracked.locked, bc.oldLocked)
tracked.currLockLevel = bc.oldLockLevel
if bc.exc != nil:
# beware that 'raises: []' is very different from not saying
# anything about 'raises' in the 'cast' at all. Same applies for 'tags'.
Expand Down Expand Up @@ -1396,17 +1338,6 @@ proc checkMethodEffects*(g: ModuleGraph; disp, branch: PSym) =
localError(g.config, branch.info, "for method '" & branch.name.s &
"' the `.requires` or `.ensures` properties are incompatible.")

if branch.typ.lockLevel > disp.typ.lockLevel:
when true:
message(g.config, branch.info, warnLockLevel,
"base method has lock level $1, but dispatcher has $2" %
[$branch.typ.lockLevel, $disp.typ.lockLevel])
else:
# XXX make this an error after bigbreak has been released:
localError(g.config, branch.info,
"base method has lock level $1, but dispatcher has $2" %
[$branch.typ.lockLevel, $disp.typ.lockLevel])

proc setEffectsForProcType*(g: ModuleGraph; t: PType, n: PNode; s: PSym = nil) =
var effects = t.n[0]
if t.kind != tyProc or effects.kind != nkEffectList: return
Expand Down Expand Up @@ -1592,13 +1523,6 @@ proc trackProc*(c: PContext; s: PSym, body: PNode) =
s.typ.flags.incl tfGcSafe
if not t.hasSideEffect and sfSideEffect notin s.flags:
s.typ.flags.incl tfNoSideEffect
if s.typ.lockLevel == UnspecifiedLockLevel:
s.typ.lockLevel = t.maxLockLevel
elif t.maxLockLevel > s.typ.lockLevel:
#localError(s.info,
message(g.config, s.info, warnLockLevel,
"declared lock level is $1, but real lock level is $2" %
[$s.typ.lockLevel, $t.maxLockLevel])
when defined(drnim):
if c.graph.strongSemCheck != nil: c.graph.strongSemCheck(c.graph, s, body)
when defined(useDfa):
Expand Down
Loading