From f32ffb6ed821cc01e52c48181a4caa15e73c0362 Mon Sep 17 00:00:00 2001 From: Andreas Rumpf Date: Wed, 17 Feb 2021 11:00:03 +0100 Subject: [PATCH] fixes #17033 [backport:1.4] (#17061) * fixes #17033 [backport:1.4] * make test robust against stdlib gensym things * cleanup assertions.nim to make topt_no_cursor easier to get right --- compiler/varpartitions.nim | 22 +++++++-- lib/system/assertions.nim | 6 +-- tests/arc/topt_no_cursor.nim | 86 +++++++++++++++++++++++++++++++++++- 3 files changed, 105 insertions(+), 9 deletions(-) diff --git a/compiler/varpartitions.nim b/compiler/varpartitions.nim index 812ab262c8..3093fb38f1 100644 --- a/compiler/varpartitions.nim +++ b/compiler/varpartitions.nim @@ -57,6 +57,7 @@ type ownsData, preventCursor, isReassigned, + isConditionallyReassigned, viewDoesMutate, viewBorrowsFromConst @@ -98,6 +99,7 @@ type goals: set[Goal] unanalysableMutation: bool inAsgnSource, inConstructor, inNoSideEffectSection: int + inConditional, inLoop: int owner: PSym g: ModuleGraph @@ -748,6 +750,13 @@ proc traverse(c: var Partitions; n: PNode) = else: for child in n: traverse(c, child) +proc markAsReassigned(c: var Partitions; vid: int) {.inline.} = + c.s[vid].flags.incl isReassigned + if c.inConditional > 0 and c.inLoop > 0: + # bug #17033: live ranges with loops and conditionals are too + # complex for our current analysis, so we prevent the cursorfication. + c.s[vid].flags.incl isConditionallyReassigned + proc computeLiveRanges(c: var Partitions; n: PNode) = # first pass: Compute live ranges for locals. # **Watch out!** We must traverse the tree like 'traverse' does @@ -777,7 +786,7 @@ proc computeLiveRanges(c: var Partitions; n: PNode) = if n[1].kind == nkSym and (c.s[vid].reassignedTo == 0 or c.s[vid].reassignedTo == n[1].sym.id): c.s[vid].reassignedTo = n[1].sym.id else: - c.s[vid].flags.incl isReassigned + markAsReassigned(c, vid) of nkSym: dec c.abstractTime @@ -804,7 +813,7 @@ proc computeLiveRanges(c: var Partitions; n: PNode) = if not paramType.isCompileTimeOnly and paramType.kind == tyVar: let vid = variableId(c, it.sym) if vid >= 0: - c.s[vid].flags.incl isReassigned + markAsReassigned(c, vid) of nkAddr, nkHiddenAddr: computeLiveRanges(c, n[0]) @@ -822,7 +831,13 @@ proc computeLiveRanges(c: var Partitions; n: PNode) = # while cond: # mutate(graph) # connect(graph, cursorVar) + inc c.inLoop for child in n: computeLiveRanges(c, child) + inc c.inLoop + of nkElifBranch, nkElifExpr, nkElse, nkOfBranch: + inc c.inConditional + for child in n: computeLiveRanges(c, child) + dec c.inConditional else: for child in n: computeLiveRanges(c, child) @@ -896,7 +911,8 @@ proc computeCursors*(s: PSym; n: PNode; g: ModuleGraph) = var par = computeGraphPartitions(s, n, g, {cursorInference}) for i in 0 ..< par.s.len: let v = addr(par.s[i]) - if v.flags * {ownsData, preventCursor} == {} and v.sym.kind notin {skParam, skResult} and + if v.flags * {ownsData, preventCursor, isConditionallyReassigned} == {} and + v.sym.kind notin {skParam, skResult} and v.sym.flags * {sfThread, sfGlobal} == {} and hasDestructor(v.sym.typ) and v.sym.typ.skipTypes({tyGenericInst, tyAlias}).kind != tyOwned: let rid = root(par, i) diff --git a/lib/system/assertions.nim b/lib/system/assertions.nim index b30b2e26d3..cf705dd6ed 100644 --- a/lib/system/assertions.nim +++ b/lib/system/assertions.nim @@ -86,13 +86,11 @@ template assert*(cond: untyped, msg = "") = ## ## No code will be generated for `assert` when passing `-d:danger` (implied by `--assertions:off`). ## See `command line switches `_. - const expr = astToStr(cond) - assertImpl(cond, msg, expr, compileOption("assertions")) + assertImpl(cond, msg, astToStr(cond), compileOption("assertions")) template doAssert*(cond: untyped, msg = "") = ## Similar to `assert <#assert.t,untyped,string>`_ but is always turned on regardless of `--assertions`. - const expr = astToStr(cond) - assertImpl(cond, msg, expr, true) + assertImpl(cond, msg, astToStr(cond), true) template onFailedAssert*(msg, code: untyped): untyped {.dirty.} = ## Sets an assertion failure handler that will intercept any assert diff --git a/tests/arc/topt_no_cursor.nim b/tests/arc/topt_no_cursor.nim index 3d37e6269d..a8cb3e848c 100644 --- a/tests/arc/topt_no_cursor.nim +++ b/tests/arc/topt_no_cursor.nim @@ -3,8 +3,12 @@ discard """ doing shady stuff... 3 6 -(@[1], @[2])''' - cmd: '''nim c --gc:arc --expandArc:newTarget --expandArc:delete --expandArc:p1 --expandArc:tt --hint:Performance:off $file''' +(@[1], @[2]) +192.168.0.1 +192.168.0.1 +192.168.0.1 +192.168.0.1''' + cmd: '''nim c --gc:arc --expandArc:newTarget --expandArc:delete --expandArc:p1 --expandArc:tt --hint:Performance:off --assertions:off --expandArc:extractConfig $file''' nimout: '''--expandArc: newTarget var @@ -78,6 +82,31 @@ try: finally: `=destroy`(:tmpD_2) `=destroy_1`(a) +-- end of expandArc ------------------------ +--expandArc: extractConfig + +var lan_ip +try: + lan_ip = "" + block :tmp: + var line + var i = 0 + let L = len(txt) + block :tmp_1: + while i < L: + var splitted + try: + line = txt[i] + splitted = split(line, " ", -1) + if splitted[0] == "opt": + `=copy`(lan_ip, splitted[1]) + echo [lan_ip] + echo [splitted[1]] + inc(i, 1) + finally: + `=destroy`(splitted) +finally: + `=destroy_1`(lan_ip) -- end of expandArc ------------------------''' """ @@ -194,3 +223,56 @@ proc plus(input: string) = (rvalue, rnext) = rresult plus("123;") + +func substrEq(s: string, pos: int, substr: string): bool = + var i = 0 + var length = substr.len + while i < length and pos+i < s.len and s[pos+i] == substr[i]: + inc i + return i == length + +template stringHasSep(s: string, index: int, sep: string): bool = + s.substrEq(index, sep) + +template splitCommon(s, sep, maxsplit, sepLen) = + var last = 0 + var splits = maxsplit + + while last <= len(s): + var first = last + while last < len(s) and not stringHasSep(s, last, sep): + inc(last) + if splits == 0: last = len(s) + yield substr(s, first, last-1) + if splits == 0: break + dec(splits) + inc(last, sepLen) + +iterator split(s: string, sep: string, maxsplit = -1): string = + splitCommon(s, sep, maxsplit, sep.len) + +template accResult(iter: untyped) = + result = @[] + for x in iter: add(result, x) + +func split*(s: string, sep: string, maxsplit = -1): seq[string] = + accResult(split(s, sep, maxsplit)) + + +let txt = @["opt 192.168.0.1", "static_lease 192.168.0.1"] + +# bug #17033 + +proc extractConfig() = + var lan_ip = "" + + for line in txt: + let splitted = line.split(" ") + if splitted[0] == "opt": + lan_ip = splitted[1] # "borrow" is conditional and inside a loop. + # Not good enough... + # we need a flag that live-ranges are disjoint + echo lan_ip + echo splitted[1] # Without this line everything works + +extractConfig()