From def1fea43a79b28dd669942de6c2193e297a48e8 Mon Sep 17 00:00:00 2001 From: metagn Date: Sat, 12 Oct 2024 23:39:59 +0300 Subject: [PATCH 01/28] don't evaluate "cannot eval" errors with nim check (#24289) fixes #24288, refs #23625 Since #23625 "cannot evaluate" errors during VM code generation are "soft" errors with `nim check`, i.e. the code generation isn't halted (except for the current proc which `return`s which can cause wrong codegen) and the expression is still attempted to be evaluated. Now, these errors signal to the VM that the current generated VM code cannot be evaluated, and so instead of evaluating, an error node is returned. This keeps the benefit of the "soft" errors without potentially crashing the compiler on improperly generated VM code. Although maybe the compiler might not be able to handle the generated error node in some cases. This fixes the chame example in #24288 but this is not tested in CI. Presumably it or the compiler was doing something like `compiles()` on code that can't run in the VM. I would accept nicer ways of tracking non-evaluability than `c.cannotEval = true` but I tried to keep it as harmless as possible. --- compiler/vm.nim | 22 +++++++++++++++++----- compiler/vmdef.nim | 1 + compiler/vmgen.nim | 1 + 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/compiler/vm.nim b/compiler/vm.nim index 161b025a65..93d2f9daed 100644 --- a/compiler/vm.nim +++ b/compiler/vm.nim @@ -2332,9 +2332,17 @@ proc execProc*(c: PCtx; sym: PSym; args: openArray[PNode]): PNode = localError(c.config, sym.info, "NimScript: attempt to call non-routine: " & sym.name.s) +proc errorNode(idgen: IdGenerator; owner: PSym, n: PNode): PNode = + result = newNodeI(nkEmpty, n.info) + result.typ = newType(tyError, idgen, owner) + result.typ.flags.incl tfCheckedForDestructor + proc evalStmt*(c: PCtx, n: PNode) = let n = transformExpr(c.graph, c.idgen, c.module, n) let start = genStmt(c, n) + if c.cannotEval: + c.cannotEval = false + return # execute new instructions; this redundant opcEof check saves us lots # of allocations in 'execute': if c.code[start].opcode != opcEof: @@ -2345,7 +2353,10 @@ proc evalExpr*(c: PCtx, n: PNode): PNode = # `nim --eval:"expr"` might've used it at some point for idetools; could # be revived for nimsuggest let n = transformExpr(c.graph, c.idgen, c.module, n) + c.cannotEval = false let start = genExpr(c, n) + if c.cannotEval: + return errorNode(c.idgen, c.module, n) assert c.code[start].opcode != opcEof result = execute(c, start) @@ -2398,7 +2409,10 @@ proc evalConstExprAux(module: PSym; idgen: IdGenerator; var c = PCtx g.vm let oldMode = c.mode c.mode = mode + c.cannotEval = false let start = genExpr(c, n, requiresValue = mode!=emStaticStmt) + if c.cannotEval: + return errorNode(idgen, prc, n) if c.code[start].opcode == opcEof: return newNodeI(nkEmpty, n.info) assert c.code[start].opcode != opcEof when debugEchoCode: c.echoCode start @@ -2469,11 +2483,6 @@ iterator genericParamsInMacroCall*(macroSym: PSym, call: PNode): (PSym, PNode) = # to prevent endless recursion in macro instantiation const evalMacroLimit = 1000 -#proc errorNode(idgen: IdGenerator; owner: PSym, n: PNode): PNode = -# result = newNodeI(nkEmpty, n.info) -# result.typ = newType(tyError, idgen, owner) -# result.typ.flags.incl tfCheckedForDestructor - proc evalMacroCall*(module: PSym; idgen: IdGenerator; g: ModuleGraph; templInstCounter: ref int; n, nOrig: PNode, sym: PSym): PNode = #if g.config.errorCounter > 0: return errorNode(idgen, module, n) @@ -2497,7 +2506,10 @@ proc evalMacroCall*(module: PSym; idgen: IdGenerator; g: ModuleGraph; templInstC c.comesFromHeuristic.line = 0'u16 c.callsite = nOrig c.templInstCounter = templInstCounter + c.cannotEval = false let start = genProc(c, sym) + if c.cannotEval: + return errorNode(idgen, module, n) var tos = PStackFrame(prc: sym, comesFrom: 0, next: nil) let maxSlots = sym.offset diff --git a/compiler/vmdef.nim b/compiler/vmdef.nim index bdb0aeed15..42e58c63b8 100644 --- a/compiler/vmdef.nim +++ b/compiler/vmdef.nim @@ -270,6 +270,7 @@ type templInstCounter*: ref int # gives every template instantiation a unique ID, needed here for getAst vmstateDiff*: seq[(PSym, PNode)] # we remember the "diff" to global state here (feature for IC) procToCodePos*: Table[int, int] + cannotEval*: bool PStackFrame* = ref TStackFrame TStackFrame* {.acyclic.} = object diff --git a/compiler/vmgen.nim b/compiler/vmgen.nim index 0c7a49984f..ddb877d93a 100644 --- a/compiler/vmgen.nim +++ b/compiler/vmgen.nim @@ -1545,6 +1545,7 @@ template cannotEval(c: PCtx; n: PNode) = if c.config.cmd == cmdCheck: localError(c.config, n.info, "cannot evaluate at compile time: " & n.renderTree) + c.cannotEval = true return globalError(c.config, n.info, "cannot evaluate at compile time: " & n.renderTree) From 720d0aee5c2d56260587f9ab09220d18ab9b53f7 Mon Sep 17 00:00:00 2001 From: metagn Date: Sat, 12 Oct 2024 23:48:44 +0300 Subject: [PATCH 02/28] add retries to testament, use it for GC tests (#24279) Testament now retries a test by a specified amount if it fails in any way other than an invalid spec. This is to deal with the flaky GC tests on Windows CI that fail in many different ways, from the linker randomly erroring, segfaults, etc. Unfortunately I couldn't do this cleanly in testament's current code. The proc `addResult`, which is the "final" proc called in a test run's lifetime, is now wrapped in a proc `finishTest` that returns a bool `true` if the test failed and has to be retried. This result is propagated up from `cmpMsgs` and `compilerOutputTests` until it reaches `testSpecHelper`, which handles these results by recursing if the test has to be retried. Since calling `testSpecHelper` means "run this test with one given configuration", this means every single matrix option/target etc. receive an equal amount of retries each. The result of `finishTest` is ignored in cases where it's known that it won't be retried due to passing, being skipped, having an invalid spec etc. It's also ignored in `testNimblePackages` because it's not necessary for those specific tests yet and similar retry behavior is already implemented for part of it. This was a last resort for the flaky GC tests but they've been a problem for years at this point, they give us more work to do and turn off contributors. Ideally GC tests failing should mark as "needs review" in the CI rather than "failed" but I don't know if Github supports something like this. --- testament/categories.nim | 13 ++-- testament/specs.nim | 4 ++ testament/testament.nim | 68 ++++++++++++------- .../tests/shouldfail/tnotenoughretries.nim | 20 ++++++ tests/gc/bintrees.nim | 4 ++ tests/gc/closureleak.nim | 1 + tests/gc/cyclecollector.nim | 3 + tests/gc/cycleleak.nim | 1 + tests/gc/foreign_thr.nim | 1 + tests/gc/gcbench.nim | 1 + tests/gc/gcemscripten.nim | 1 + tests/gc/gcleak.nim | 1 + tests/gc/gcleak2.nim | 1 + tests/gc/gcleak3.nim | 1 + tests/gc/gcleak4.nim | 1 + tests/gc/gcleak5.nim | 1 + tests/gc/gctest.nim | 1 + tests/gc/growobjcrash.nim | 4 ++ tests/gc/refarrayleak.nim | 1 + tests/gc/stackrefleak.nim | 1 + tests/gc/tdisable_orc.nim | 1 + tests/gc/thavlak.nim | 1 + tests/gc/tlists.nim | 1 + tests/gc/trace_globals.nim | 1 + tests/gc/tregionleak.nim | 1 + tests/gc/weakrefs.nim | 1 + tests/testament/tretries.nim | 20 ++++++ tests/testament/tshould_not_work.nim | 4 +- 28 files changed, 126 insertions(+), 33 deletions(-) create mode 100644 testament/tests/shouldfail/tnotenoughretries.nim create mode 100644 tests/testament/tretries.nim diff --git a/testament/categories.nim b/testament/categories.nim index 6a05fcf0ce..8f497d0da3 100644 --- a/testament/categories.nim +++ b/testament/categories.nim @@ -449,7 +449,7 @@ proc testNimblePackages(r: var TResults; cat: Category; packageFilter: string) = if pkg.allowFailure: inc r.passed inc r.failedButAllowed - addResult(r, test, targetC, "", "", cmd & "\n" & outp, reFailed, allowFailure = pkg.allowFailure) + discard r.finishTest(test, targetC, "", "", cmd & "\n" & outp, reFailed, allowFailure = pkg.allowFailure) continue outp @@ -465,21 +465,21 @@ proc testNimblePackages(r: var TResults; cat: Category; packageFilter: string) = discard tryCommand(cmds[i], maxRetries = 3) discard tryCommand(cmds[^1], reFailed = reBuildFailed) inc r.passed - r.addResult(test, targetC, "", "", "", reSuccess, allowFailure = pkg.allowFailure) + discard r.finishTest(test, targetC, "", "", "", reSuccess, allowFailure = pkg.allowFailure) errors = r.total - r.passed if errors == 0: - r.addResult(packageFileTest, targetC, "", "", "", reSuccess) + discard r.finishTest(packageFileTest, targetC, "", "", "", reSuccess) else: - r.addResult(packageFileTest, targetC, "", "", "", reBuildFailed) + discard r.finishTest(packageFileTest, targetC, "", "", "", reBuildFailed) except JsonParsingError: errors = 1 - r.addResult(packageFileTest, targetC, "", "", "Invalid package file", reBuildFailed) + discard r.finishTest(packageFileTest, targetC, "", "", "Invalid package file", reBuildFailed) raise except ValueError: errors = 1 - r.addResult(packageFileTest, targetC, "", "", "Unknown package", reBuildFailed) + discard r.finishTest(packageFileTest, targetC, "", "", "Unknown package", reBuildFailed) raise # bug #18805 finally: if errors == 0: removeDir(packagesDir) @@ -568,6 +568,7 @@ proc isJoinableSpec(spec: TSpec): bool = spec.err != reDisabled and not spec.unjoinable and spec.exitCode == 0 and + spec.retries == 0 and spec.input.len == 0 and spec.nimout.len == 0 and spec.nimoutFull == false and diff --git a/testament/specs.nim b/testament/specs.nim index c3040c1d8f..1a50f5eb17 100644 --- a/testament/specs.nim +++ b/testament/specs.nim @@ -56,6 +56,7 @@ type reJoined, # test is disabled because it was joined into the megatest reSuccess # test was successful reInvalidSpec # test had problems to parse the spec + reRetry # test is being retried TTarget* = enum targetC = "c" @@ -102,6 +103,7 @@ type # but don't rely on much precision inlineErrors*: seq[InlineError] # line information to error message debugInfo*: string # debug info to give more context + retries*: int # number of retry attempts after the test fails proc getCmd*(s: TSpec): string = if s.cmd.len == 0: @@ -477,6 +479,8 @@ proc parseSpec*(filename: string): TSpec = result.timeout = parseFloat(e.value) except ValueError: result.parseErrors.addLine "cannot interpret as a float: ", e.value + of "retries": + discard parseInt(e.value, result.retries) of "targets", "target": try: result.targets.incl parseTargets(e.value) diff --git a/testament/testament.nim b/testament/testament.nim index 1e892e6367..41d3f50376 100644 --- a/testament/testament.nim +++ b/testament/testament.nim @@ -275,16 +275,13 @@ proc testName(test: TTest, target: TTarget, extraOptions: string, allowFailure: name.strip() proc addResult(r: var TResults, test: TTest, target: TTarget, - extraOptions, expected, given: string, successOrig: TResultEnum, + extraOptions, expected, given: string, success: TResultEnum, duration: float, allowFailure = false, givenSpec: ptr TSpec = nil) = # instead of `ptr TSpec` we could also use `Option[TSpec]`; passing `givenSpec` makes it easier to get what we need # instead of having to pass individual fields, or abusing existing ones like expected vs given. # test.name is easier to find than test.name.extractFilename # A bit hacky but simple and works with tests/testament/tshould_not_work.nim let name = testName(test, target, extraOptions, allowFailure) - let duration = epochTime() - test.startTime - let success = if test.spec.timeout > 0.0 and duration > test.spec.timeout: reTimeout - else: successOrig let durationStr = duration.formatFloat(ffDecimal, precision = 2).align(5) if backendLogging: @@ -345,6 +342,18 @@ proc addResult(r: var TResults, test: TTest, target: TTarget, discard waitForExit(p) close(p) +proc finishTest(r: var TResults, test: TTest, target: TTarget, + extraOptions, expected, given: string, successOrig: TResultEnum, + allowFailure = false, givenSpec: ptr TSpec = nil): bool = + result = false + let duration = epochTime() - test.startTime + let success = if test.spec.timeout > 0.0 and duration > test.spec.timeout: reTimeout + else: successOrig + if test.spec.retries > 0 and success notin {reSuccess, reDisabled, reJoined, reInvalidSpec}: + return true + else: + addResult(r, test, target, extraOptions, expected, given, success, duration, allowFailure, givenSpec) + proc toString(inlineError: InlineError, filename: string): string = result.add "$file($line, $col) $kind: $msg" % [ "file", filename, @@ -373,23 +382,23 @@ proc nimoutCheck(expected, given: TSpec): bool = result = false proc cmpMsgs(r: var TResults, expected, given: TSpec, test: TTest, - target: TTarget, extraOptions: string) = + target: TTarget, extraOptions: string): bool = if not checkForInlineErrors(expected, given) or (not expected.nimoutFull and not nimoutCheck(expected, given)): - r.addResult(test, target, extraOptions, expected.nimout & inlineErrorsMsgs(expected), given.nimout, reMsgsDiffer) + result = r.finishTest(test, target, extraOptions, expected.nimout & inlineErrorsMsgs(expected), given.nimout, reMsgsDiffer) elif strip(expected.msg) notin strip(given.msg): - r.addResult(test, target, extraOptions, expected.msg, given.msg, reMsgsDiffer) + result = r.finishTest(test, target, extraOptions, expected.msg, given.msg, reMsgsDiffer) elif not nimoutCheck(expected, given): - r.addResult(test, target, extraOptions, expected.nimout, given.nimout, reMsgsDiffer) + result = r.finishTest(test, target, extraOptions, expected.nimout, given.nimout, reMsgsDiffer) elif extractFilename(expected.file) != extractFilename(given.file) and "internal error:" notin expected.msg: - r.addResult(test, target, extraOptions, expected.file, given.file, reFilesDiffer) + result = r.finishTest(test, target, extraOptions, expected.file, given.file, reFilesDiffer) elif expected.line != given.line and expected.line != 0 or expected.column != given.column and expected.column != 0: - r.addResult(test, target, extraOptions, $expected.line & ':' & $expected.column, + result = r.finishTest(test, target, extraOptions, $expected.line & ':' & $expected.column, $given.line & ':' & $given.column, reLinesDiffer) else: - r.addResult(test, target, extraOptions, expected.msg, given.msg, reSuccess) + result = r.finishTest(test, target, extraOptions, expected.msg, given.msg, reSuccess) inc(r.passed) proc generatedFile(test: TTest, target: TTarget): string = @@ -428,7 +437,7 @@ proc codegenCheck(test: TTest, target: TTarget, spec: TSpec, expectedMsg: var st echo getCurrentExceptionMsg() proc compilerOutputTests(test: TTest, target: TTarget, extraOptions: string, - given: var TSpec, expected: TSpec; r: var TResults) = + given: var TSpec, expected: TSpec; r: var TResults): bool = var expectedmsg: string = "" var givenmsg: string = "" if given.err == reSuccess: @@ -443,7 +452,7 @@ proc compilerOutputTests(test: TTest, target: TTarget, extraOptions: string, else: givenmsg = "$ " & given.cmd & '\n' & given.nimout if given.err == reSuccess: inc(r.passed) - r.addResult(test, target, extraOptions, expectedmsg, givenmsg, given.err) + result = r.finishTest(test, target, extraOptions, expectedmsg, givenmsg, given.err) proc getTestSpecTarget(): TTarget = if getEnv("NIM_COMPILE_TO_CPP", "false") == "true": @@ -459,31 +468,38 @@ proc equalModuloLastNewline(a, b: string): bool = proc testSpecHelper(r: var TResults, test: var TTest, expected: TSpec, target: TTarget, extraOptions: string, nimcache: string) = - test.startTime = epochTime() + template maybeRetry(x: bool) = + if x: + test.spec.err = reRetry + dec test.spec.retries + testSpecHelper(r, test, expected, target, extraOptions, nimcache) + return + if test.spec.err != reRetry: + test.startTime = epochTime() if testName(test, target, extraOptions, false) in skips: test.spec.err = reDisabled if test.spec.err in {reDisabled, reJoined}: - r.addResult(test, target, extraOptions, "", "", test.spec.err) + discard r.finishTest(test, target, extraOptions, "", "", test.spec.err) inc(r.skipped) return var given = callNimCompiler(expected.getCmd, test.name, test.options, nimcache, target, extraOptions) case expected.action of actionCompile: - compilerOutputTests(test, target, extraOptions, given, expected, r) + maybeRetry compilerOutputTests(test, target, extraOptions, given, expected, r) of actionRun: if given.err != reSuccess: - r.addResult(test, target, extraOptions, "", "$ " & given.cmd & '\n' & given.nimout, given.err, givenSpec = given.addr) + maybeRetry r.finishTest(test, target, extraOptions, "", "$ " & given.cmd & '\n' & given.nimout, given.err, givenSpec = given.addr) else: let isJsTarget = target == targetJS var exeFile = changeFileExt(test.name, if isJsTarget: "js" else: ExeExt) if not fileExists(exeFile): - r.addResult(test, target, extraOptions, expected.output, + maybeRetry r.finishTest(test, target, extraOptions, expected.output, "executable not found: " & exeFile, reExeNotFound) else: let nodejs = if isJsTarget: findNodeJs() else: "" if isJsTarget and nodejs == "": - r.addResult(test, target, extraOptions, expected.output, "nodejs binary not in PATH", + maybeRetry r.finishTest(test, target, extraOptions, expected.output, "nodejs binary not in PATH", reExeNotFound) else: var exeCmd: string @@ -515,19 +531,19 @@ proc testSpecHelper(r: var TResults, test: var TTest, expected: TSpec, buf if exitCode != expected.exitCode: given.err = reExitcodesDiffer - r.addResult(test, target, extraOptions, "exitcode: " & $expected.exitCode, + maybeRetry r.finishTest(test, target, extraOptions, "exitcode: " & $expected.exitCode, "exitcode: " & $exitCode & "\n\nOutput:\n" & bufB, reExitcodesDiffer) elif (expected.outputCheck == ocEqual and not expected.output.equalModuloLastNewline(bufB)) or (expected.outputCheck == ocSubstr and expected.output notin bufB): given.err = reOutputsDiffer - r.addResult(test, target, extraOptions, expected.output, bufB, reOutputsDiffer) - compilerOutputTests(test, target, extraOptions, given, expected, r) + maybeRetry r.finishTest(test, target, extraOptions, expected.output, bufB, reOutputsDiffer) + maybeRetry compilerOutputTests(test, target, extraOptions, given, expected, r) of actionReject: # Make sure its the compiler rejecting and not the system (e.g. segfault) - cmpMsgs(r, expected, given, test, target, extraOptions) + maybeRetry cmpMsgs(r, expected, given, test, target, extraOptions) if given.exitCode != QuitFailure: - r.addResult(test, target, extraOptions, "exitcode: " & $QuitFailure, + maybeRetry r.finishTest(test, target, extraOptions, "exitcode: " & $QuitFailure, "exitcode: " & $given.exitCode & "\n\nOutput:\n" & given.nimout, reExitcodesDiffer) @@ -552,7 +568,7 @@ proc targetHelper(r: var TResults, test: TTest, expected: TSpec, extraOptions: s for target in expected.targets: inc(r.total) if target notin gTargets: - r.addResult(test, target, extraOptions, "", "", reDisabled) + discard r.finishTest(test, target, extraOptions, "", "", reDisabled) inc(r.skipped) elif simulate: inc count @@ -567,7 +583,7 @@ proc testSpec(r: var TResults, test: TTest, targets: set[TTarget] = {}) = var expected = test.spec if expected.parseErrors.len > 0: # targetC is a lie, but a parameter is required - r.addResult(test, targetC, "", "", expected.parseErrors, reInvalidSpec) + discard r.finishTest(test, targetC, "", "", expected.parseErrors, reInvalidSpec) inc(r.total) return diff --git a/testament/tests/shouldfail/tnotenoughretries.nim b/testament/tests/shouldfail/tnotenoughretries.nim new file mode 100644 index 0000000000..20eda20754 --- /dev/null +++ b/testament/tests/shouldfail/tnotenoughretries.nim @@ -0,0 +1,20 @@ +discard """ + retries: 1 +""" + +import os + +const tempFile = "tnotenoughretries_temp" + +if not fileExists(tempFile): + writeFile(tempFile, "abc") + quit(1) +else: + let content = readFile(tempFile) + if content == "abc": + writeFile(tempFile, "def") + quit(1) + else: + # success + removeFile(tempFile) + discard diff --git a/tests/gc/bintrees.nim b/tests/gc/bintrees.nim index 5b65bb4376..8c9324dad7 100644 --- a/tests/gc/bintrees.nim +++ b/tests/gc/bintrees.nim @@ -1,3 +1,7 @@ +discard """ + retries: 2 +""" + # -*- nim -*- import os, strutils diff --git a/tests/gc/closureleak.nim b/tests/gc/closureleak.nim index e67beb513d..d20452f956 100644 --- a/tests/gc/closureleak.nim +++ b/tests/gc/closureleak.nim @@ -1,6 +1,7 @@ discard """ outputsub: "true" disabled: "32bit" + retries: 2 """ type diff --git a/tests/gc/cyclecollector.nim b/tests/gc/cyclecollector.nim index 2d02a7a3c4..9cc1bbcee1 100644 --- a/tests/gc/cyclecollector.nim +++ b/tests/gc/cyclecollector.nim @@ -1,3 +1,6 @@ +discard """ + retries: 2 +""" # Program to detect bug #1796 reliably diff --git a/tests/gc/cycleleak.nim b/tests/gc/cycleleak.nim index e355abc968..d774661e86 100644 --- a/tests/gc/cycleleak.nim +++ b/tests/gc/cycleleak.nim @@ -1,5 +1,6 @@ discard """ outputsub: "no leak: " + retries: 2 """ type diff --git a/tests/gc/foreign_thr.nim b/tests/gc/foreign_thr.nim index 88ab951133..2709261c11 100644 --- a/tests/gc/foreign_thr.nim +++ b/tests/gc/foreign_thr.nim @@ -6,6 +6,7 @@ Hello from thread Hello from thread ''' cmd: "nim $target --hints:on --threads:on --tlsEmulation:off $options $file" + retries: 2 """ # Copied from stdlib import strutils diff --git a/tests/gc/gcbench.nim b/tests/gc/gcbench.nim index e29ea762d0..da339f6d4b 100644 --- a/tests/gc/gcbench.nim +++ b/tests/gc/gcbench.nim @@ -1,5 +1,6 @@ discard """ outputsub: "Success!" + retries: 2 """ # This is adapted from a benchmark written by John Ellis and Pete Kovac diff --git a/tests/gc/gcemscripten.nim b/tests/gc/gcemscripten.nim index cc12b230f1..4905027bc5 100644 --- a/tests/gc/gcemscripten.nim +++ b/tests/gc/gcemscripten.nim @@ -1,5 +1,6 @@ discard """ outputsub: "77\n77" + retries: 2 """ ## Check how GC/Alloc works in Emscripten diff --git a/tests/gc/gcleak.nim b/tests/gc/gcleak.nim index 0bf993968e..326f421429 100644 --- a/tests/gc/gcleak.nim +++ b/tests/gc/gcleak.nim @@ -1,5 +1,6 @@ discard """ outputsub: "no leak: " + retries: 2 """ when defined(GC_setMaxPause): diff --git a/tests/gc/gcleak2.nim b/tests/gc/gcleak2.nim index bc943dbe71..4bd5d57c4a 100644 --- a/tests/gc/gcleak2.nim +++ b/tests/gc/gcleak2.nim @@ -1,5 +1,6 @@ discard """ outputsub: "no leak: " + retries: 2 """ when defined(GC_setMaxPause): diff --git a/tests/gc/gcleak3.nim b/tests/gc/gcleak3.nim index 5e146d69f0..897d87382a 100644 --- a/tests/gc/gcleak3.nim +++ b/tests/gc/gcleak3.nim @@ -1,5 +1,6 @@ discard """ outputsub: "no leak: " + retries: 2 """ when defined(GC_setMaxPause): diff --git a/tests/gc/gcleak4.nim b/tests/gc/gcleak4.nim index a72db67b74..84a1ee943b 100644 --- a/tests/gc/gcleak4.nim +++ b/tests/gc/gcleak4.nim @@ -1,5 +1,6 @@ discard """ outputsub: "no leak: " + retries: 2 """ type diff --git a/tests/gc/gcleak5.nim b/tests/gc/gcleak5.nim index f1913831b4..f926b8ae67 100644 --- a/tests/gc/gcleak5.nim +++ b/tests/gc/gcleak5.nim @@ -1,5 +1,6 @@ discard """ output: "success" + retries: 2 """ import os, times diff --git a/tests/gc/gctest.nim b/tests/gc/gctest.nim index 78b78934c0..e970e557cd 100644 --- a/tests/gc/gctest.nim +++ b/tests/gc/gctest.nim @@ -1,5 +1,6 @@ discard """ outputsub: "finished" + retries: 2 """ # Test the garbage collector. diff --git a/tests/gc/growobjcrash.nim b/tests/gc/growobjcrash.nim index ff1aa7e98d..7101ae40b4 100644 --- a/tests/gc/growobjcrash.nim +++ b/tests/gc/growobjcrash.nim @@ -1,3 +1,7 @@ +discard """ + retries: 2 +""" + import std/[cgi, strtabs] proc handleRequest(query: string): StringTableRef = diff --git a/tests/gc/refarrayleak.nim b/tests/gc/refarrayleak.nim index 57b489721a..8c94e83287 100644 --- a/tests/gc/refarrayleak.nim +++ b/tests/gc/refarrayleak.nim @@ -1,5 +1,6 @@ discard """ outputsub: "no leak: " + retries: 2 """ type diff --git a/tests/gc/stackrefleak.nim b/tests/gc/stackrefleak.nim index 7f3fbff43d..b273bb8753 100644 --- a/tests/gc/stackrefleak.nim +++ b/tests/gc/stackrefleak.nim @@ -1,5 +1,6 @@ discard """ outputsub: "no leak: " + retries: 2 """ type diff --git a/tests/gc/tdisable_orc.nim b/tests/gc/tdisable_orc.nim index b5f161c791..d43da55664 100644 --- a/tests/gc/tdisable_orc.nim +++ b/tests/gc/tdisable_orc.nim @@ -1,5 +1,6 @@ discard """ joinable: false + retries: 2 """ import std/asyncdispatch diff --git a/tests/gc/thavlak.nim b/tests/gc/thavlak.nim index cfd860e258..f697a9eba9 100644 --- a/tests/gc/thavlak.nim +++ b/tests/gc/thavlak.nim @@ -8,6 +8,7 @@ Performing Loop Recognition Another 3 iterations... ... Found 1 loops (including artificial root node) (3)''' + retries: 2 """ # bug #3184 diff --git a/tests/gc/tlists.nim b/tests/gc/tlists.nim index 959cc5f7c4..b9d5c07c33 100644 --- a/tests/gc/tlists.nim +++ b/tests/gc/tlists.nim @@ -1,5 +1,6 @@ discard """ output: '''Success''' + retries: 2 """ # bug #3793 diff --git a/tests/gc/trace_globals.nim b/tests/gc/trace_globals.nim index f62a15692c..2d8eca2f4e 100644 --- a/tests/gc/trace_globals.nim +++ b/tests/gc/trace_globals.nim @@ -3,6 +3,7 @@ discard """ 10000000 10000000 10000000''' + retries: 2 """ # bug #17085 diff --git a/tests/gc/tregionleak.nim b/tests/gc/tregionleak.nim index 277cfc9875..578ede01e4 100644 --- a/tests/gc/tregionleak.nim +++ b/tests/gc/tregionleak.nim @@ -4,6 +4,7 @@ discard """ finalized finalized ''' + retries: 2 """ proc finish(o: RootRef) = diff --git a/tests/gc/weakrefs.nim b/tests/gc/weakrefs.nim index 81c048d746..b20b364c5d 100644 --- a/tests/gc/weakrefs.nim +++ b/tests/gc/weakrefs.nim @@ -1,5 +1,6 @@ discard """ output: "true" + retries: 2 """ import intsets diff --git a/tests/testament/tretries.nim b/tests/testament/tretries.nim new file mode 100644 index 0000000000..eaa5b1e916 --- /dev/null +++ b/tests/testament/tretries.nim @@ -0,0 +1,20 @@ +discard """ + retries: 2 +""" + +import os + +const tempFile = "tretries_temp" + +if not fileExists(tempFile): + writeFile(tempFile, "abc") + quit(1) +else: + let content = readFile(tempFile) + if content == "abc": + writeFile(tempFile, "def") + quit(1) + else: + # success + removeFile(tempFile) + discard diff --git a/tests/testament/tshould_not_work.nim b/tests/testament/tshould_not_work.nim index 8c99510a0a..088d0d2bdf 100644 --- a/tests/testament/tshould_not_work.nim +++ b/tests/testament/tshould_not_work.nim @@ -24,6 +24,8 @@ FAIL: tests/shouldfail/tnimout.nim Failure: reMsgsDiffer FAIL: tests/shouldfail/tnimoutfull.nim Failure: reMsgsDiffer +FAIL: tests/shouldfail/tnotenoughretries.nim +Failure: reExitcodesDiffer FAIL: tests/shouldfail/toutput.nim Failure: reOutputsDiffer FAIL: tests/shouldfail/toutputsub.nim @@ -43,7 +45,7 @@ import stdtest/testutils proc main = const nim = getCurrentCompilerExe() - let testamentExe = "bin/testament" + let testamentExe = "testament/testament" let cmd = fmt"{testamentExe} --directory:testament --colors:off --backendLogging:off --nim:{nim} category shouldfail" let (outp, status) = execCmdEx(cmd) doAssert status == 1, $status From 2f7586c0668490efb26fceb91b840d121c571a40 Mon Sep 17 00:00:00 2001 From: metagn Date: Sun, 13 Oct 2024 07:59:20 +0300 Subject: [PATCH 03/28] clean up testament retries, add some comments (#24294) follows up #24279 `discard finishTest` was wrong if the test still had a `retries` option: it would just ignore the result of the test. This is an unlikely mistake but we safeguard against it by splitting `finishTest` into two, one that completely ignores the retries option and `finishTestRetryable` which has to be checked for a retry. This also makes the code look slightly better. --- testament/categories.nim | 12 +++++----- testament/testament.nim | 52 +++++++++++++++++++++++++++------------- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/testament/categories.nim b/testament/categories.nim index 8f497d0da3..4704301b5f 100644 --- a/testament/categories.nim +++ b/testament/categories.nim @@ -449,7 +449,7 @@ proc testNimblePackages(r: var TResults; cat: Category; packageFilter: string) = if pkg.allowFailure: inc r.passed inc r.failedButAllowed - discard r.finishTest(test, targetC, "", "", cmd & "\n" & outp, reFailed, allowFailure = pkg.allowFailure) + r.finishTest(test, targetC, "", "", cmd & "\n" & outp, reFailed, allowFailure = pkg.allowFailure) continue outp @@ -465,21 +465,21 @@ proc testNimblePackages(r: var TResults; cat: Category; packageFilter: string) = discard tryCommand(cmds[i], maxRetries = 3) discard tryCommand(cmds[^1], reFailed = reBuildFailed) inc r.passed - discard r.finishTest(test, targetC, "", "", "", reSuccess, allowFailure = pkg.allowFailure) + r.finishTest(test, targetC, "", "", "", reSuccess, allowFailure = pkg.allowFailure) errors = r.total - r.passed if errors == 0: - discard r.finishTest(packageFileTest, targetC, "", "", "", reSuccess) + r.finishTest(packageFileTest, targetC, "", "", "", reSuccess) else: - discard r.finishTest(packageFileTest, targetC, "", "", "", reBuildFailed) + r.finishTest(packageFileTest, targetC, "", "", "", reBuildFailed) except JsonParsingError: errors = 1 - discard r.finishTest(packageFileTest, targetC, "", "", "Invalid package file", reBuildFailed) + r.finishTest(packageFileTest, targetC, "", "", "Invalid package file", reBuildFailed) raise except ValueError: errors = 1 - discard r.finishTest(packageFileTest, targetC, "", "", "Unknown package", reBuildFailed) + r.finishTest(packageFileTest, targetC, "", "", "Unknown package", reBuildFailed) raise # bug #18805 finally: if errors == 0: removeDir(packagesDir) diff --git a/testament/testament.nim b/testament/testament.nim index 41d3f50376..9bea815eab 100644 --- a/testament/testament.nim +++ b/testament/testament.nim @@ -344,7 +344,22 @@ proc addResult(r: var TResults, test: TTest, target: TTarget, proc finishTest(r: var TResults, test: TTest, target: TTarget, extraOptions, expected, given: string, successOrig: TResultEnum, - allowFailure = false, givenSpec: ptr TSpec = nil): bool = + allowFailure = false, givenSpec: ptr TSpec = nil) = + ## calculates duration of test, reports result + ## `retries` option in the test is ignored + let duration = epochTime() - test.startTime + let success = if test.spec.timeout > 0.0 and duration > test.spec.timeout: reTimeout + else: successOrig + addResult(r, test, target, extraOptions, expected, given, success, duration, allowFailure, givenSpec) + +proc finishTestRetryable(r: var TResults, test: TTest, target: TTarget, + extraOptions, expected, given: string, successOrig: TResultEnum, + allowFailure = false, givenSpec: ptr TSpec = nil): bool = + ## if test failed and has remaining retries, return `true`, + ## otherwise calculate duration and report result + ## + ## warning: if `true` is returned, then the result is not reported, + ## it has to be retried or `finishTest` should be called instead result = false let duration = epochTime() - test.startTime let success = if test.spec.timeout > 0.0 and duration > test.spec.timeout: reTimeout @@ -383,22 +398,23 @@ proc nimoutCheck(expected, given: TSpec): bool = proc cmpMsgs(r: var TResults, expected, given: TSpec, test: TTest, target: TTarget, extraOptions: string): bool = + # result has to be checked for retry if not checkForInlineErrors(expected, given) or (not expected.nimoutFull and not nimoutCheck(expected, given)): - result = r.finishTest(test, target, extraOptions, expected.nimout & inlineErrorsMsgs(expected), given.nimout, reMsgsDiffer) + result = r.finishTestRetryable(test, target, extraOptions, expected.nimout & inlineErrorsMsgs(expected), given.nimout, reMsgsDiffer) elif strip(expected.msg) notin strip(given.msg): - result = r.finishTest(test, target, extraOptions, expected.msg, given.msg, reMsgsDiffer) + result = r.finishTestRetryable(test, target, extraOptions, expected.msg, given.msg, reMsgsDiffer) elif not nimoutCheck(expected, given): - result = r.finishTest(test, target, extraOptions, expected.nimout, given.nimout, reMsgsDiffer) + result = r.finishTestRetryable(test, target, extraOptions, expected.nimout, given.nimout, reMsgsDiffer) elif extractFilename(expected.file) != extractFilename(given.file) and "internal error:" notin expected.msg: - result = r.finishTest(test, target, extraOptions, expected.file, given.file, reFilesDiffer) + result = r.finishTestRetryable(test, target, extraOptions, expected.file, given.file, reFilesDiffer) elif expected.line != given.line and expected.line != 0 or expected.column != given.column and expected.column != 0: - result = r.finishTest(test, target, extraOptions, $expected.line & ':' & $expected.column, + result = r.finishTestRetryable(test, target, extraOptions, $expected.line & ':' & $expected.column, $given.line & ':' & $given.column, reLinesDiffer) else: - result = r.finishTest(test, target, extraOptions, expected.msg, given.msg, reSuccess) + result = r.finishTestRetryable(test, target, extraOptions, expected.msg, given.msg, reSuccess) inc(r.passed) proc generatedFile(test: TTest, target: TTarget): string = @@ -438,6 +454,7 @@ proc codegenCheck(test: TTest, target: TTarget, spec: TSpec, expectedMsg: var st proc compilerOutputTests(test: TTest, target: TTarget, extraOptions: string, given: var TSpec, expected: TSpec; r: var TResults): bool = + # result has to be checked for retry var expectedmsg: string = "" var givenmsg: string = "" if given.err == reSuccess: @@ -452,7 +469,7 @@ proc compilerOutputTests(test: TTest, target: TTarget, extraOptions: string, else: givenmsg = "$ " & given.cmd & '\n' & given.nimout if given.err == reSuccess: inc(r.passed) - result = r.finishTest(test, target, extraOptions, expectedmsg, givenmsg, given.err) + result = r.finishTestRetryable(test, target, extraOptions, expectedmsg, givenmsg, given.err) proc getTestSpecTarget(): TTarget = if getEnv("NIM_COMPILE_TO_CPP", "false") == "true": @@ -469,6 +486,7 @@ proc equalModuloLastNewline(a, b: string): bool = proc testSpecHelper(r: var TResults, test: var TTest, expected: TSpec, target: TTarget, extraOptions: string, nimcache: string) = template maybeRetry(x: bool) = + # if `x` is true, retries the test if x: test.spec.err = reRetry dec test.spec.retries @@ -480,7 +498,7 @@ proc testSpecHelper(r: var TResults, test: var TTest, expected: TSpec, test.spec.err = reDisabled if test.spec.err in {reDisabled, reJoined}: - discard r.finishTest(test, target, extraOptions, "", "", test.spec.err) + r.finishTest(test, target, extraOptions, "", "", test.spec.err) inc(r.skipped) return var given = callNimCompiler(expected.getCmd, test.name, test.options, nimcache, target, extraOptions) @@ -489,17 +507,17 @@ proc testSpecHelper(r: var TResults, test: var TTest, expected: TSpec, maybeRetry compilerOutputTests(test, target, extraOptions, given, expected, r) of actionRun: if given.err != reSuccess: - maybeRetry r.finishTest(test, target, extraOptions, "", "$ " & given.cmd & '\n' & given.nimout, given.err, givenSpec = given.addr) + maybeRetry r.finishTestRetryable(test, target, extraOptions, "", "$ " & given.cmd & '\n' & given.nimout, given.err, givenSpec = given.addr) else: let isJsTarget = target == targetJS var exeFile = changeFileExt(test.name, if isJsTarget: "js" else: ExeExt) if not fileExists(exeFile): - maybeRetry r.finishTest(test, target, extraOptions, expected.output, + maybeRetry r.finishTestRetryable(test, target, extraOptions, expected.output, "executable not found: " & exeFile, reExeNotFound) else: let nodejs = if isJsTarget: findNodeJs() else: "" if isJsTarget and nodejs == "": - maybeRetry r.finishTest(test, target, extraOptions, expected.output, "nodejs binary not in PATH", + maybeRetry r.finishTestRetryable(test, target, extraOptions, expected.output, "nodejs binary not in PATH", reExeNotFound) else: var exeCmd: string @@ -531,19 +549,19 @@ proc testSpecHelper(r: var TResults, test: var TTest, expected: TSpec, buf if exitCode != expected.exitCode: given.err = reExitcodesDiffer - maybeRetry r.finishTest(test, target, extraOptions, "exitcode: " & $expected.exitCode, + maybeRetry r.finishTestRetryable(test, target, extraOptions, "exitcode: " & $expected.exitCode, "exitcode: " & $exitCode & "\n\nOutput:\n" & bufB, reExitcodesDiffer) elif (expected.outputCheck == ocEqual and not expected.output.equalModuloLastNewline(bufB)) or (expected.outputCheck == ocSubstr and expected.output notin bufB): given.err = reOutputsDiffer - maybeRetry r.finishTest(test, target, extraOptions, expected.output, bufB, reOutputsDiffer) + maybeRetry r.finishTestRetryable(test, target, extraOptions, expected.output, bufB, reOutputsDiffer) maybeRetry compilerOutputTests(test, target, extraOptions, given, expected, r) of actionReject: # Make sure its the compiler rejecting and not the system (e.g. segfault) maybeRetry cmpMsgs(r, expected, given, test, target, extraOptions) if given.exitCode != QuitFailure: - maybeRetry r.finishTest(test, target, extraOptions, "exitcode: " & $QuitFailure, + maybeRetry r.finishTestRetryable(test, target, extraOptions, "exitcode: " & $QuitFailure, "exitcode: " & $given.exitCode & "\n\nOutput:\n" & given.nimout, reExitcodesDiffer) @@ -568,7 +586,7 @@ proc targetHelper(r: var TResults, test: TTest, expected: TSpec, extraOptions: s for target in expected.targets: inc(r.total) if target notin gTargets: - discard r.finishTest(test, target, extraOptions, "", "", reDisabled) + r.finishTest(test, target, extraOptions, "", "", reDisabled) inc(r.skipped) elif simulate: inc count @@ -583,7 +601,7 @@ proc testSpec(r: var TResults, test: TTest, targets: set[TTarget] = {}) = var expected = test.spec if expected.parseErrors.len > 0: # targetC is a lie, but a parameter is required - discard r.finishTest(test, targetC, "", "", expected.parseErrors, reInvalidSpec) + r.finishTest(test, targetC, "", "", expected.parseErrors, reInvalidSpec) inc(r.total) return From 1dbf61485811ac28d38bf16102de4e4e82c7f062 Mon Sep 17 00:00:00 2001 From: Aryo Date: Sun, 13 Oct 2024 07:00:23 +0200 Subject: [PATCH 04/28] Expand enum example tut1.md (#24268) I couldn't understand why there is "x" declaration. Comparison make it easier to understand to people not familiar to enums. --- doc/tut1.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/tut1.md b/doc/tut1.md index 2e83effa3b..3eaa1b1610 100644 --- a/doc/tut1.md +++ b/doc/tut1.md @@ -1182,6 +1182,9 @@ at runtime by 0, the second by 1, and so on. For example: var x = south # `x` is of type `Direction`; its value is `south` echo x # prints "south" + + if x == Direction.south: + echo "It's south!" ``` All the comparison operators can be used with enumeration types. From a0d78d259bdc603bee9aa2d65e3507526cadd8bf Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Sun, 13 Oct 2024 19:07:49 +0800 Subject: [PATCH 05/28] update to setup-nim-action@v2 (#24297) --- .github/workflows/bisects.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/bisects.yml b/.github/workflows/bisects.yml index 4a6a92f018..db1c1f5bf9 100644 --- a/.github/workflows/bisects.yml +++ b/.github/workflows/bisects.yml @@ -17,9 +17,10 @@ jobs: steps: - uses: actions/checkout@v4 - - uses: jiro4989/setup-nim-action@v1 + - uses: jiro4989/setup-nim-action@v2 with: - nim-version: 'devel' + nim-version: devel + repo-token: ${{ secrets.GITHUB_TOKEN }} - uses: juancarlospaco/nimrun-action@nim if: | From 71515bf278505c625245a2ea2fd5f85354e5b284 Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Sun, 13 Oct 2024 19:38:23 +0800 Subject: [PATCH 06/28] Revert "update to setup-nim-action@v2" (#24299) Reverts nim-lang/Nim#24297 We need to set up a choosenim action --- .github/workflows/bisects.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/bisects.yml b/.github/workflows/bisects.yml index db1c1f5bf9..4a6a92f018 100644 --- a/.github/workflows/bisects.yml +++ b/.github/workflows/bisects.yml @@ -17,10 +17,9 @@ jobs: steps: - uses: actions/checkout@v4 - - uses: jiro4989/setup-nim-action@v2 + - uses: jiro4989/setup-nim-action@v1 with: - nim-version: devel - repo-token: ${{ secrets.GITHUB_TOKEN }} + nim-version: 'devel' - uses: juancarlospaco/nimrun-action@nim if: | From 80e6b357216cdc3f320e57cf6e11c4a738bb9159 Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Mon, 14 Oct 2024 01:54:30 +0800 Subject: [PATCH 07/28] templates/macros use no expected types when return types are specified (#24298) fixes #24296 fixes #24295 Templates use `expectedType` for type inference. It's justified that when templates don't have an actual return type, i.e., `untyped` etc. When the return type of templates is specified, we should not infer the type ```nim template g(): string = "" let c: cstring = g() ``` In this example, it is not reasonable to annotate the templates expression with the `cstring` type before the `fitNode` check with its specified return type. --- compiler/sem.nim | 2 +- tests/types/ttopdowninference.nim | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/compiler/sem.nim b/compiler/sem.nim index 699bcbb948..1866d85fcb 100644 --- a/compiler/sem.nim +++ b/compiler/sem.nim @@ -494,7 +494,7 @@ proc semAfterMacroCall(c: PContext, call, macroResult: PNode, if retType.kind == tyVoid: result = semStmt(c, result, flags) else: - result = semExpr(c, result, flags, expectedType) + result = semExpr(c, result, flags) result = fitNode(c, retType, result, result.info) #globalError(s.info, errInvalidParamKindX, typeToString(s.typ.returnType)) dec(c.config.evalTemplateCounter) diff --git a/tests/types/ttopdowninference.nim b/tests/types/ttopdowninference.nim index 765761e993..303effdf7b 100644 --- a/tests/types/ttopdowninference.nim +++ b/tests/types/ttopdowninference.nim @@ -331,3 +331,25 @@ block: # issue #24164, related regression template bar(x: untyped = nil) = foo(x) bar() + +block: # bug #24296 + # Either changing the template to `proc`/`func` or using `$""`, not a string + # literal alone, allows any version of Nim 2.x to compile this. + template g(): string = "" + + # Alternatively: don't retrieve the string through g(), but directly, also + # allows compilation across Nim 2.x versions. + const d: cstring = "" + const f: cstring = $"" + const b = cstring g() + const m = cstring "" + const p = cstring $"" + + # But this does not compile across Nim 2.x/devel. + const c: cstring = g() + let e: cstring = g() + +block: # bug #24295 + template g(_: int): string = "" + const c: cstring = 0.g() + From 07628b0dec43dd417b6b536eff1c0f0a5c539b6d Mon Sep 17 00:00:00 2001 From: metagn Date: Sun, 13 Oct 2024 20:56:17 +0300 Subject: [PATCH 08/28] use cbuilder for most braced initializers (#24259) `StructInitializer` is now used for most braced initializers in the C generation, mostly in `genBracedInit`, `getNullValueAux`, `getDefaultValue`. The exceptions are: * the default case branch initializer for objects uses C99 designated initializers with field names, which are not implemented for `StructInitializer` yet (`siNamedStruct`) * the uses in `ccgliterals` are untouched so all of ccgliterals can be done separately and in 1 go There is one case where `genBracedInit` does not use cbuilder, which is the global literal variable for openarrays. The reason for this is simply that variables with C array type are not implemented, which I thought would be best to leave out of this PR. For the simplicity of the implementation, code in `getNullValueAuxT` that reset the initializer back to its initial state if the `Sup` field did not have any fields itself, is now disabled. This was so the compiler does not generate `{}` for the Sup field, i.e. `{{}}`, but every call to `getNullValueAuxT` still generates `{}` if the struct doesn't have any fields, so I don't know if it really breaks anything. The case where the Sup field doesn't have any fields but the struct does also would have generated `{{}, field}`. Worst case, we can implement either the "resetting" or just disable the generation of the `Sup` field if there are no fields total. But a better fix might be to always generate `{0}` if the struct has no fields, in line with the `char dummy` field that gets added for all objects with no fields. This doesn't seem necessary for now but might be for the NIFC output, in which case we can probably keep the logic contained inside cbuilder (if no fields generated for `siOrderedStruct`/`siNamedStruct`, we add a `0` for the `dummy` field). This would stipulate that all uses of struct initializers are exhaustive for every field in structs. --- compiler/cbuilderdecls.nim | 46 ++++-- compiler/cbuilderexprs.nim | 6 + compiler/ccgexprs.nim | 327 ++++++++++++++++++++++--------------- compiler/ccgliterals.nim | 2 +- compiler/ccgstmts.nim | 2 +- compiler/ccgtypes.nim | 1 + 6 files changed, 234 insertions(+), 150 deletions(-) diff --git a/compiler/cbuilderdecls.nim b/compiler/cbuilderdecls.nim index 4185c71826..e3c1c31075 100644 --- a/compiler/cbuilderdecls.nim +++ b/compiler/cbuilderdecls.nim @@ -63,41 +63,55 @@ template addTypedef(builder: var Builder, name: string, typeBody: typed) = builder.add(name) builder.add(";\n") -type StructInitializer = object - ## context for building struct initializers, i.e. `{ field1, field2 }` - # XXX use in genBracedInit - orderCompliant: bool - ## if true, fields will not be named, instead values are placed in order - needsComma: bool +type + StructInitializerKind = enum + siOrderedStruct ## struct constructor, but without named fields on C + siNamedStruct ## struct constructor, with named fields i.e. C99 designated initializer + siArray ## array constructor + siWrapper ## wrapper for a single field, generates it verbatim -proc initStructInitializer(builder: var Builder, orderCompliant: bool): StructInitializer = + StructInitializer = object + ## context for building struct initializers, i.e. `{ field1, field2 }` + kind: StructInitializerKind + ## if true, fields will not be named, instead values are placed in order + needsComma: bool + +proc initStructInitializer(builder: var Builder, kind: StructInitializerKind): StructInitializer = ## starts building a struct initializer, `orderCompliant = true` means ## built fields must be ordered correctly - doAssert orderCompliant, "named struct constructors unimplemented" - result = StructInitializer(orderCompliant: true, needsComma: false) - builder.add("{ ") + assert kind != siNamedStruct, "named struct constructors unimplemented" + result = StructInitializer(kind: kind, needsComma: false) + if kind != siWrapper: + builder.add("{") template addField(builder: var Builder, constr: var StructInitializer, name: string, valueBody: typed) = ## adds a field to a struct initializer, with the value built in `valueBody` if constr.needsComma: + assert constr.kind != siWrapper, "wrapper constructor cannot have multiple fields" builder.add(", ") else: constr.needsComma = true - if constr.orderCompliant: + case constr.kind + of siArray, siWrapper: # no name, can just add value valueBody - else: - doAssert false, "named struct constructors unimplemented" + of siOrderedStruct: + # no name, can just add value on C + assert name.len != 0, "name has to be given for struct initializer field" + valueBody + of siNamedStruct: + assert false, "named struct constructors unimplemented" proc finishStructInitializer(builder: var Builder, constr: StructInitializer) = ## finishes building a struct initializer - builder.add(" }") + if constr.kind != siWrapper: + builder.add("}") -template addStructInitializer(builder: var Builder, constr: out StructInitializer, orderCompliant: bool, body: typed) = +template addStructInitializer(builder: var Builder, constr: out StructInitializer, kind: StructInitializerKind, body: typed) = ## builds a struct initializer, i.e. `{ field1, field2 }` ## a `var StructInitializer` must be declared and passed as a parameter so ## that it can be used with `addField` - constr = builder.initStructInitializer(orderCompliant) + constr = builder.initStructInitializer(kind) body builder.finishStructInitializer(constr) diff --git a/compiler/cbuilderexprs.nim b/compiler/cbuilderexprs.nim index 16caaa0215..f70405be7a 100644 --- a/compiler/cbuilderexprs.nim +++ b/compiler/cbuilderexprs.nim @@ -15,5 +15,11 @@ const proc procPtrType(conv: TCallingConvention, rettype: Snippet, name: string): Snippet = CallingConvToStr[conv] & "_PTR(" & rettype & ", " & name & ")" +proc cCast(typ, value: Snippet): Snippet = + "((" & typ & ") " & value & ")" + +proc cAddr(value: Snippet): Snippet = + "&" & value + proc bitOr(a, b: Snippet): Snippet = a & " | " & b diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim index a344816ba8..21ecc9688b 100644 --- a/compiler/ccgexprs.nim +++ b/compiler/ccgexprs.nim @@ -13,7 +13,7 @@ when defined(nimCompilerStacktraceHints): import std/stackframes proc getNullValueAuxT(p: BProc; orig, t: PType; obj, constOrNil: PNode, - result: var Rope; count: var int; + result: var Builder; init: var StructInitializer; isConst: bool, info: TLineInfo) # -------------------------- constant expressions ------------------------ @@ -3193,7 +3193,7 @@ proc expr(p: BProc, n: PNode, d: var TLoc) = of nkMixinStmt, nkBindStmt: discard else: internalError(p.config, n.info, "expr(" & $n.kind & "); unknown node kind") -proc getDefaultValue(p: BProc; typ: PType; info: TLineInfo; result: var Rope) = +proc getDefaultValue(p: BProc; typ: PType; info: TLineInfo; result: var Builder) = var t = skipTypes(typ, abstractRange+{tyOwned}-{tyTypeDesc}) case t.kind of tyBool: result.add rope"NIM_FALSE" @@ -3204,38 +3204,56 @@ proc getDefaultValue(p: BProc; typ: PType; info: TLineInfo; result: var Rope) = result.add rope"NIM_NIL" of tyString, tySequence: if optSeqDestructors in p.config.globalOptions: - result.add "{0, NIM_NIL}" + var seqInit: StructInitializer + result.addStructInitializer(seqInit, kind = siOrderedStruct): + result.addField(seqInit, name = "len"): + result.add("0") + result.addField(seqInit, name = "p"): + result.add("NIM_NIL") else: result.add "NIM_NIL" of tyProc: if t.callConv != ccClosure: result.add "NIM_NIL" else: - result.add "{NIM_NIL, NIM_NIL}" + var closureInit: StructInitializer + result.addStructInitializer(closureInit, kind = siOrderedStruct): + result.addField(closureInit, name = "ClP_0"): + result.add("NIM_NIL") + result.addField(closureInit, name = "ClE_0"): + result.add("NIM_NIL") of tyObject: - var count = 0 - result.add "{" - getNullValueAuxT(p, t, t, t.n, nil, result, count, true, info) - result.add "}" + var objInit: StructInitializer + result.addStructInitializer(objInit, kind = siOrderedStruct): + getNullValueAuxT(p, t, t, t.n, nil, result, objInit, true, info) of tyTuple: - result.add "{" - if p.vccAndC and t.isEmptyTupleType: - result.add "0" - for i, a in t.ikids: - if i > 0: result.add ", " - getDefaultValue(p, a, info, result) - result.add "}" + var tupleInit: StructInitializer + result.addStructInitializer(tupleInit, kind = siOrderedStruct): + if p.vccAndC and t.isEmptyTupleType: + result.addField(tupleInit, name = "dummy"): + result.add "0" + for i, a in t.ikids: + result.addField(tupleInit, name = "Field" & $i): + getDefaultValue(p, a, info, result) of tyArray: - result.add "{" - for i in 0.. 0: result.add ", " - getDefaultValue(p, t.elementType, info, result) - result.add "}" + var arrInit: StructInitializer + result.addStructInitializer(arrInit, kind = siArray): + for i in 0.. 0: res.add ", " + getNullValueAux(p, t, obj[0], constOrNil, result, init, isConst, info) var branch = Zero if constOrNil != nil: ## find kind value, default is zero if not specified @@ -3269,140 +3285,177 @@ proc getNullValueAux(p: BProc; t: PType; obj, constOrNil: PNode, break let selectedBranch = caseObjDefaultBranch(obj, branch) - res.add "{" - var countB = 0 + # XXX siNamedStruct needs to be implemented to replace `res` here + var res = "{" + var branchInit: StructInitializer let b = lastSon(obj[selectedBranch]) # designated initilization is the only way to init non first element of unions # branches are allowed to have no members (b.len == 0), in this case they don't need initializer if b.kind == nkRecList and not isEmptyCaseObjectBranch(b): - res.add "._" & mangleRecFieldName(p.module, obj[0].sym) & "_" & $selectedBranch & " = {" - getNullValueAux(p, t, b, constOrNil, res, countB, isConst, info) - res.add "}" + res.add "._" & mangleRecFieldName(p.module, obj[0].sym) & "_" & $selectedBranch & " = " + res.addStructInitializer(branchInit, kind = siOrderedStruct): + getNullValueAux(p, t, b, constOrNil, res, branchInit, isConst, info) elif b.kind == nkSym: res.add "." & mangleRecFieldName(p.module, b.sym) & " = " - getNullValueAux(p, t, b, constOrNil, res, countB, isConst, info) + res.addStructInitializer(branchInit, kind = siWrapper): + getNullValueAux(p, t, b, constOrNil, res, branchInit, isConst, info) else: return - result.add res - result.add "}" + result.addField(init, name = ""): + # XXX figure out name for the union, see use of `addAnonUnion` + result.add res + result.add "}" of nkSym: - if count > 0: result.add ", " - inc count let field = obj.sym - if constOrNil != nil: - for i in 1.. 0: result.add ",\n" - if it.kind == nkExprColonExpr: genBracedInit(p, it[1], isConst, it[0].typ, result) - else: genBracedInit(p, it, isConst, it.typ, result) - result.add("}\n") - -proc genConstTuple(p: BProc, n: PNode; isConst: bool; tup: PType; result: var Rope) = - result.add "{" - if p.vccAndC and n.len == 0: - result.add "0" - for i in 0.. 0: result.add ",\n" - if it.kind == nkExprColonExpr: genBracedInit(p, it[1], isConst, tup[i], result) - else: genBracedInit(p, it, isConst, tup[i], result) - result.add("}\n") - -proc genConstSeq(p: BProc, n: PNode, t: PType; isConst: bool; result: var Rope) = - var data = "{{$1, $1 | NIM_STRLIT_FLAG}" % [n.len.rope] - let base = t.skipTypes(abstractInst)[0] - if n.len > 0: - # array part needs extra curlies: - data.add(", {") +proc genConstSimpleList(p: BProc, n: PNode; isConst: bool; result: var Builder) = + var arrInit: StructInitializer + result.addStructInitializer(arrInit, kind = siArray): + if p.vccAndC and n.len == 0 and n.typ.kind == tyArray: + result.addField(arrInit, name = ""): + getDefaultValue(p, n.typ.elementType, n.info, result) for i in 0.. 0: data.addf(",$n", []) - genBracedInit(p, n[i], isConst, base, data) - data.add("}") - data.add("}") + let it = n[i] + var ind, val: PNode + if it.kind == nkExprColonExpr: + ind = it[0] + val = it[1] + else: + ind = it + val = it + result.addField(arrInit, name = ""): + genBracedInit(p, val, isConst, ind.typ, result) +proc genConstTuple(p: BProc, n: PNode; isConst: bool; tup: PType; result: var Builder) = + var tupleInit: StructInitializer + result.addStructInitializer(tupleInit, kind = siOrderedStruct): + if p.vccAndC and n.len == 0: + result.addField(tupleInit, name = "dummy"): + result.add("0") + for i in 0.. 0: + def.addField(structInit, name = "data"): + var arrInit: StructInitializer + def.addStructInitializer(arrInit, kind = siArray): + for i in 0.. 0: - data.add(", {") - for i in 0.. 0: data.addf(",$n", []) - genBracedInit(p, n[i], isConst, base, data) - data.add("}") - let payload = getTempName(p.module) - appcg(p.module, cfsStrData, - "static $5 struct {$n" & - " NI cap; $1 data[$2];$n" & - "} $3 = {$2 | NIM_STRLIT_FLAG$4};$n", [ - getTypeDesc(p.module, base), n.len, payload, data, - if isConst: "const" else: ""]) - result.add "{$1, ($2*)&$3}" % [rope(n.len), getSeqPayloadType(p.module, t), payload] -proc genBracedInit(p: BProc, n: PNode; isConst: bool; optionalType: PType; result: var Rope) = + var def = newBuilder("") + def.addVarWithTypeAndInitializer( + if isConst: AlwaysConst else: Global, + name = payload): + def.addSimpleStruct(p.module, name = "", baseType = ""): + def.addField(name = "cap", typ = "NI") + def.addArrayField(name = "data", elementType = getTypeDesc(p.module, base), len = n.len) + do: + var structInit: StructInitializer + def.addStructInitializer(structInit, kind = siOrderedStruct): + def.addField(structInit, name = "cap"): + def.add(bitOr(rope(n.len), "NIM_STRLIT_FLAG")) + if n.len > 0: + def.addField(structInit, name = "data"): + var arrInit: StructInitializer + def.addStructInitializer(arrInit, kind = siArray): + for i in 0.. Date: Mon, 14 Oct 2024 10:26:44 +0800 Subject: [PATCH 09/28] define `-d:nimHasDefaultFloatRoundtrip` and enable datamancer (#24300) ref https://github.com/SciNim/Datamancer/pull/73 ref https://github.com/SciNim/Datamancer/issues/72 --- compiler/condsyms.nim | 1 + testament/important_packages.nim | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/condsyms.nim b/compiler/condsyms.nim index 5043fc5d4c..4523081a63 100644 --- a/compiler/condsyms.nim +++ b/compiler/condsyms.nim @@ -169,3 +169,4 @@ proc initDefines*(symbols: StringTableRef) = defineSymbol("nimHasGenericsOpenSym2") defineSymbol("nimHasGenericsOpenSym3") defineSymbol("nimHasJsNoLambdaLifting") + defineSymbol("nimHasDefaultFloatRoundtrip") diff --git a/testament/important_packages.nim b/testament/important_packages.nim index 1a9837ee0f..9bab3fe434 100644 --- a/testament/important_packages.nim +++ b/testament/important_packages.nim @@ -61,7 +61,7 @@ pkg "constantine", "nimble make_lib" pkg "cowstrings", "nim c -r tests/tcowstrings.nim" pkg "criterion" pkg "dashing", "nim c tests/functional.nim" -pkg "datamancer", url = "https://github.com/nim-lang/Datamancer" +pkg "datamancer" pkg "delaunay" pkg "docopt" pkg "dotenv" From 34c87de9841c0f2d01272f1c34513ce9f6f591ce Mon Sep 17 00:00:00 2001 From: metagn Date: Mon, 14 Oct 2024 09:46:50 +0300 Subject: [PATCH 10/28] use cbuilder for ccgliterals (#24302) follows up #24259 This was the only use of the `STRING_LITERAL` macro in `nimbase.h`, so this macro is now removed. We don't have to remove it though, maybe people use it. --- compiler/cbuilderexprs.nim | 2 +- compiler/ccgliterals.nim | 70 ++++++++++++++++++++++++++------------ lib/nimbase.h | 6 ---- 3 files changed, 50 insertions(+), 28 deletions(-) diff --git a/compiler/cbuilderexprs.nim b/compiler/cbuilderexprs.nim index f70405be7a..f381944ecc 100644 --- a/compiler/cbuilderexprs.nim +++ b/compiler/cbuilderexprs.nim @@ -22,4 +22,4 @@ proc cAddr(value: Snippet): Snippet = "&" & value proc bitOr(a, b: Snippet): Snippet = - a & " | " & b + "(" & a & " | " & b & ")" diff --git a/compiler/ccgliterals.nim b/compiler/ccgliterals.nim index 8e53be6447..477876c9fe 100644 --- a/compiler/ccgliterals.nim +++ b/compiler/ccgliterals.nim @@ -36,22 +36,37 @@ proc genStringLiteralDataOnlyV1(m: BModule, s: string; result: var Rope) = cgsym(m, "TGenericSeq") let tmp = getTempName(m) result.add tmp - m.s[cfsStrData].addf("STRING_LITERAL($1, $2, $3);$n", - [tmp, makeCString(s), rope(s.len)]) + var res = newBuilder("") + res.addVarWithTypeAndInitializer(AlwaysConst, name = tmp): + res.addSimpleStruct(m, name = "", baseType = ""): + res.addField(name = "Sup", typ = "TGenericSeq") + res.addArrayField(name = "data", elementType = "NIM_CHAR", len = s.len + 1) + do: + var strInit: StructInitializer + res.addStructInitializer(strInit, kind = siOrderedStruct): + res.addField(strInit, name = "Sup"): + var seqInit: StructInitializer + res.addStructInitializer(seqInit, kind = siOrderedStruct): + res.addField(seqInit, name = "len"): + res.add(rope(s.len)) + res.addField(seqInit, name = "reserved"): + res.add(cCast("NI", bitOr(cCast("NU", rope(s.len)), "NIM_STRLIT_FLAG"))) + res.addField(strInit, name = "data"): + res.add(makeCString(s)) + m.s[cfsStrData].add(res) proc genStringLiteralV1(m: BModule; n: PNode; result: var Rope) = if s.isNil: - appcg(m, result, "((#NimStringDesc*) NIM_NIL)", []) + result.add(cCast(ptrType(cgsymValue(m, "NimStringDesc")), "NIM_NIL")) else: let id = nodeTableTestOrSet(m.dataCache, n, m.labels) + var name: string = "" if id == m.labels: # string literal not found in the cache: - appcg(m, result, "((#NimStringDesc*) &", []) - genStringLiteralDataOnlyV1(m, n.strVal, result) - result.add ")" + genStringLiteralDataOnlyV1(m, n.strVal, name) else: - appcg(m, result, "((#NimStringDesc*) &$1$2)", - [m.tmpBase, id]) + name = m.tmpBase & $id + result.add(cCast(ptrType(cgsymValue(m, "NimStringDesc")), cAddr(name))) # ------ Version 2: destructor based strings and seqs ----------------------- @@ -74,22 +89,30 @@ proc genStringLiteralDataOnlyV2(m: BModule, s: string; result: Rope; isConst: bo proc genStringLiteralV2(m: BModule; n: PNode; isConst: bool; result: var Rope) = let id = nodeTableTestOrSet(m.dataCache, n, m.labels) + var litName: string if id == m.labels: - let pureLit = getTempName(m) - genStringLiteralDataOnlyV2(m, n.strVal, pureLit, isConst) - let tmp = getTempName(m) - result.add tmp cgsym(m, "NimStrPayload") cgsym(m, "NimStringV2") # string literal not found in the cache: - m.s[cfsStrData].addf("static $4 NimStringV2 $1 = {$2, (NimStrPayload*)&$3};$n", - [tmp, rope(n.strVal.len), pureLit, rope(if isConst: "const" else: "")]) + litName = getTempName(m) + genStringLiteralDataOnlyV2(m, n.strVal, litName, isConst) else: - let tmp = getTempName(m) - result.add tmp - m.s[cfsStrData].addf("static $4 NimStringV2 $1 = {$2, (NimStrPayload*)&$3};$n", - [tmp, rope(n.strVal.len), m.tmpBase & rope(id), - rope(if isConst: "const" else: "")]) + litName = m.tmpBase & $id + let tmp = getTempName(m) + result.add tmp + var res = newBuilder("") + res.addVarWithTypeAndInitializer( + if isConst: AlwaysConst else: Global, + name = tmp): + res.add("NimStringV2") + do: + var strInit: StructInitializer + res.addStructInitializer(strInit, kind = siOrderedStruct): + res.addField(strInit, name = "len"): + res.add(rope(n.strVal.len)) + res.addField(strInit, name = "p"): + res.add(cCast(ptrType("NimStrPayload"), cAddr(litName))) + m.s[cfsStrData].add(res) proc genStringLiteralV2Const(m: BModule; n: PNode; isConst: bool; result: var Rope) = let id = nodeTableTestOrSet(m.dataCache, n, m.labels) @@ -102,7 +125,12 @@ proc genStringLiteralV2Const(m: BModule; n: PNode; isConst: bool; result: var Ro genStringLiteralDataOnlyV2(m, n.strVal, pureLit, isConst) else: pureLit = m.tmpBase & rope(id) - result.addf "{$1, (NimStrPayload*)&$2}", [rope(n.strVal.len), pureLit] + var strInit: StructInitializer + result.addStructInitializer(strInit, kind = siOrderedStruct): + result.addField(strInit, name = "len"): + result.add(rope(n.strVal.len)) + result.addField(strInit, name = "p"): + result.add(cCast(ptrType("NimStrPayload"), cAddr(pureLit))) # ------ Version selector --------------------------------------------------- @@ -118,7 +146,7 @@ proc genStringLiteralDataOnly(m: BModule; s: string; info: TLineInfo; localError(m.config, info, "cannot determine how to produce code for string literal") proc genNilStringLiteral(m: BModule; info: TLineInfo; result: var Rope) = - appcg(m, result, "((#NimStringDesc*) NIM_NIL)", []) + result.add(cCast(ptrType(cgsymValue(m, "NimStringDesc")), "NIM_NIL")) proc genStringLiteral(m: BModule; n: PNode; result: var Rope) = case detectStrVersion(m) diff --git a/lib/nimbase.h b/lib/nimbase.h index 258bf8862f..f64bdd8951 100644 --- a/lib/nimbase.h +++ b/lib/nimbase.h @@ -469,12 +469,6 @@ typedef char* NCSTRING; #define NIM_STRLIT_FLAG ((NU)(1) << ((NIM_INTBITS) - 2)) /* This has to be the same as system.strlitFlag! */ -#define STRING_LITERAL(name, str, length) \ - static const struct { \ - TGenericSeq Sup; \ - NIM_CHAR data[(length) + 1]; \ - } name = {{length, (NI) ((NU)length | NIM_STRLIT_FLAG)}, str} - /* declared size of a sequence/variable length array: */ #if defined(__cplusplus) && defined(__clang__) # define SEQ_DECL_SIZE 1 From 6df050d6d2e6e0b62aff3bba093149e15c6e1060 Mon Sep 17 00:00:00 2001 From: metagn Date: Mon, 14 Oct 2024 18:07:57 +0300 Subject: [PATCH 11/28] only generate first field for default value of union (#24303) fixes #20653 --- compiler/ccgexprs.nim | 4 ++++ tests/ccgbugs/tunioninit.nim | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 tests/ccgbugs/tunioninit.nim diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim index 21ecc9688b..d59dd7aec9 100644 --- a/compiler/ccgexprs.nim +++ b/compiler/ccgexprs.nim @@ -3268,8 +3268,12 @@ proc getNullValueAux(p: BProc; t: PType; obj, constOrNil: PNode, isConst: bool, info: TLineInfo) = case obj.kind of nkRecList: + let isUnion = tfUnion in t.flags for it in obj.sons: getNullValueAux(p, t, it, constOrNil, result, init, isConst, info) + if isUnion: + # generate only 1 field for default value of union + return of nkRecCase: getNullValueAux(p, t, obj[0], constOrNil, result, init, isConst, info) var branch = Zero diff --git a/tests/ccgbugs/tunioninit.nim b/tests/ccgbugs/tunioninit.nim new file mode 100644 index 0000000000..a58c8d27f6 --- /dev/null +++ b/tests/ccgbugs/tunioninit.nim @@ -0,0 +1,32 @@ +# issue #20653 + +type + EmptySeq* {.bycopy.} = object + + ChoiceWithEmptySeq_d* {.bycopy.} = object + a*: bool + + INNER_C_UNION* {.bycopy, union.} = object + a*: char + b*: EmptySeq + c*: byte + d*: ChoiceWithEmptySeq_d + + ChoiceWithEmptySeq_selection* = enum + ChoiceWithEmptySeq_NONE, + ChoiceWithEmptySeq_a_PRESENT, + ChoiceWithEmptySeq_b_PRESENT, + ChoiceWithEmptySeq_c_PRESENT, + ChoiceWithEmptySeq_d_PRESENT + + ChoiceWithEmptySeq* {.bycopy.} = object + kind*: ChoiceWithEmptySeq_selection + u*: INNER_C_UNION + + Og_Context* {.bycopy.} = object + state*: int + init_done*: bool + ch*: ChoiceWithEmptySeq + em*: EmptySeq + +var context* : Og_Context = Og_Context(init_done: false) From 8b39b2df7d638344a8df918486b94f6fde572494 Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Mon, 14 Oct 2024 23:43:12 +0800 Subject: [PATCH 12/28] fixes #24258; compiler crash on `len` of `varargs[untyped]` (#24307) fixes #24258 It uses conditionals to guard against ill formed AST to produce better error messages, rather than crashing --- compiler/renderer.nim | 7 ++++--- tests/errmsgs/t24258.nim | 10 ++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) create mode 100644 tests/errmsgs/t24258.nim diff --git a/compiler/renderer.nim b/compiler/renderer.nim index cc07c0c2d5..a598a0ae5e 100644 --- a/compiler/renderer.nim +++ b/compiler/renderer.nim @@ -1311,10 +1311,11 @@ proc gsub(g: var TSrcGen, n: PNode, c: TContext, fromStmtList = false) = put(g, tkCustomLit, n[0].strVal) gsub(g, n, 1) else: - gsub(g, n, 0) + for i in 0.. 1: + accentedName(g, n[^1]) of nkBind: putWithSpace(g, tkBind, "bind") gsub(g, n, 0) diff --git a/tests/errmsgs/t24258.nim b/tests/errmsgs/t24258.nim new file mode 100644 index 0000000000..17466d4b2d --- /dev/null +++ b/tests/errmsgs/t24258.nim @@ -0,0 +1,10 @@ +discard """ + cmd: "nim check $file" + errormsg: "illformed AST: [22]43.len" + joinable: false +""" + +template encodeList*(args: varargs[untyped]): seq[byte] = + @[byte args.len] + +let x = encodeList([22], 43) \ No newline at end of file From 3e8f44b232bf6a5108e8abd6f00d208d7e1eb2d8 Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Mon, 14 Oct 2024 23:43:41 +0800 Subject: [PATCH 13/28] fixes ci_generate produces unnecessary spaces on Windows (#24309) follow up https://github.com/nim-lang/Nim/pull/17899 --- build_all.bat | 6 +++--- tools/ci_generate.nim | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/build_all.bat b/build_all.bat index ef4a5f6a4e..bef7956f28 100644 --- a/build_all.bat +++ b/build_all.bat @@ -24,6 +24,6 @@ if not exist %nim_csources% ( cd .. copy /y bin\nim.exe %nim_csources% ) - bin\nim.exe c --noNimblePath --skipUserCfg --skipParentCfg --hints:off koch - koch boot -d:release --skipUserCfg --skipParentCfg --hints:off - koch tools --skipUserCfg --skipParentCfg --hints:off +bin\nim.exe c --noNimblePath --skipUserCfg --skipParentCfg --hints:off koch +koch boot -d:release --skipUserCfg --skipParentCfg --hints:off +koch tools --skipUserCfg --skipParentCfg --hints:off diff --git a/tools/ci_generate.nim b/tools/ci_generate.nim index 46bc39470f..a461be5915 100644 --- a/tools/ci_generate.nim +++ b/tools/ci_generate.nim @@ -42,9 +42,9 @@ triggers: proc genBuildExtras(echoRun, koch, nim: string): string = result = fmt""" -{echoRun} {nim} c --noNimblePath --skipUserCfg --skipParentCfg --hints:off koch -{echoRun} {koch} boot -d:release --skipUserCfg --skipParentCfg --hints:off -{echoRun} {koch} tools --skipUserCfg --skipParentCfg --hints:off +{echoRun}{nim} c --noNimblePath --skipUserCfg --skipParentCfg --hints:off koch +{echoRun}{koch} boot -d:release --skipUserCfg --skipParentCfg --hints:off +{echoRun}{koch} tools --skipUserCfg --skipParentCfg --hints:off """ proc genWindowsScript(buildAll: bool): string = @@ -95,7 +95,7 @@ set -e # exit on first error . ci/funs.sh nimBuildCsourcesIfNeeded "$@" -{genBuildExtras("echo_run", "./koch", "bin/nim")} +{genBuildExtras("echo_run ", "./koch", "bin/nim")} """ proc main()= From 922f7dfd712130bb68a16336085c0320afceb273 Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Tue, 15 Oct 2024 09:19:46 +0800 Subject: [PATCH 14/28] closes #19585; adds a test case for #21648 (#24310) closes #19585 follow up #21648 --- tests/ccgbugs/t19585.nim | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 tests/ccgbugs/t19585.nim diff --git a/tests/ccgbugs/t19585.nim b/tests/ccgbugs/t19585.nim new file mode 100644 index 0000000000..2a23738ec6 --- /dev/null +++ b/tests/ccgbugs/t19585.nim @@ -0,0 +1,13 @@ +discard """ + targets: "c cpp" +""" + +# bug #19585 + +type + X* {.exportc.} = object + v: int + +{.emit:""" +X x = { 1234 }; +""".} \ No newline at end of file From 53460f312cb5411a2291bb6ad76314a712f36434 Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Tue, 15 Oct 2024 15:45:06 +0800 Subject: [PATCH 15/28] make owner a private field of `PSym` (#24311) --- compiler/ast.nim | 9 ++++++--- compiler/evaltempl.nim | 2 +- compiler/ic/ic.nim | 4 ++-- compiler/modules.nim | 2 +- compiler/sem.nim | 2 +- compiler/semexprs.nim | 2 +- compiler/seminst.nim | 8 ++++---- compiler/semmagic.nim | 6 +++--- compiler/semstmts.nim | 22 +++++++++++----------- compiler/semtypes.nim | 2 +- compiler/semtypinst.nim | 2 +- compiler/transf.nim | 2 +- 12 files changed, 33 insertions(+), 30 deletions(-) diff --git a/compiler/ast.nim b/compiler/ast.nim index a342e1ea71..f665444b17 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -706,7 +706,7 @@ type when defined(nimsuggest): endInfo*: TLineInfo hasUserSpecifiedType*: bool # used for determining whether to display inlay type hints - owner*: PSym + ownerField: PSym flags*: TSymFlags ast*: PNode # syntax tree of proc, iterator, etc.: # the whole proc including header; this is used @@ -814,6 +814,9 @@ type template nodeId(n: PNode): int = cast[int](n) +template owner*(s: PSym): PSym = + s.ownerField + type Gconfig = object # we put comments in a side channel to avoid increasing `sizeof(TNode)`, which # reduces memory usage given that `PNode` is the most allocated type by far. @@ -1197,7 +1200,7 @@ proc newSym*(symKind: TSymKind, name: PIdent, idgen: IdGenerator; owner: PSym, assert not name.isNil let id = nextSymId idgen result = PSym(name: name, kind: symKind, flags: {}, info: info, itemId: id, - options: options, owner: owner, offset: defaultOffset, + options: options, ownerField: owner, offset: defaultOffset, disamb: getOrDefault(idgen.disambTable, name).int32) idgen.disambTable.inc name when false: @@ -1707,7 +1710,7 @@ proc transitionNoneToSym*(n: PNode) = template transitionSymKindCommon*(k: TSymKind) = let obj {.inject.} = s[] s[] = TSym(kind: k, itemId: obj.itemId, magic: obj.magic, typ: obj.typ, name: obj.name, - info: obj.info, owner: obj.owner, flags: obj.flags, ast: obj.ast, + info: obj.info, ownerField: obj.ownerField, flags: obj.flags, ast: obj.ast, options: obj.options, position: obj.position, offset: obj.offset, loc: obj.loc, annex: obj.annex, constraint: obj.constraint) when hasFFI: diff --git a/compiler/evaltempl.nim b/compiler/evaltempl.nim index 77c136d632..199a047d4a 100644 --- a/compiler/evaltempl.nim +++ b/compiler/evaltempl.nim @@ -64,7 +64,7 @@ proc evalTemplateAux(templ, actual: PNode, c: var TemplCtx, result: PNode) = if x == nil: x = copySym(s, c.idgen) # sem'check needs to set the owner properly later, see bug #9476 - x.owner = nil # c.genSymOwner + x.owner() = nil # c.genSymOwner #if x.kind == skParam and x.owner.kind == skModule: # internalAssert c.config, false idTablePut(c.mapping, s, x) diff --git a/compiler/ic/ic.nim b/compiler/ic/ic.nim index 8e81633efe..23cff281b7 100644 --- a/compiler/ic/ic.nim +++ b/compiler/ic/ic.nim @@ -942,7 +942,7 @@ proc symBodyFromPacked(c: var PackedDecoder; g: var PackedModuleGraph; result.guard = loadSym(c, g, si, s.guard) result.bitsize = s.bitsize result.alignment = s.alignment - result.owner = loadSym(c, g, si, s.owner) + result.owner() = loadSym(c, g, si, s.owner) let externalName = g[si].fromDisk.strings[s.externalName] if externalName != "": result.loc.snippet = externalName @@ -1062,7 +1062,7 @@ proc setupLookupTables(g: var PackedModuleGraph; conf: ConfigRef; cache: IdentCa name: getIdent(cache, splitFile(filename).name), info: newLineInfo(fileIdx, 1, 1), position: int(fileIdx)) - m.module.owner = getPackage(conf, cache, fileIdx) + m.module.owner() = getPackage(conf, cache, fileIdx) m.module.flags = m.fromDisk.moduleFlags proc loadToReplayNodes(g: var PackedModuleGraph; conf: ConfigRef; cache: IdentCache; diff --git a/compiler/modules.nim b/compiler/modules.nim index 6e2af8bcc7..6b893f11ee 100644 --- a/compiler/modules.nim +++ b/compiler/modules.nim @@ -25,7 +25,7 @@ template getModuleIdent(graph: ModuleGraph, filename: AbsoluteFile): PIdent = proc partialInitModule*(result: PSym; graph: ModuleGraph; fileIdx: FileIndex; filename: AbsoluteFile) = let packSym = getPackage(graph, fileIdx) - result.owner = packSym + result.owner() = packSym result.position = int fileIdx proc newModule*(graph: ModuleGraph; fileIdx: FileIndex): PSym = diff --git a/compiler/sem.nim b/compiler/sem.nim index 1866d85fcb..43da922854 100644 --- a/compiler/sem.nim +++ b/compiler/sem.nim @@ -256,7 +256,7 @@ proc newSymG*(kind: TSymKind, n: PNode, c: PContext): PSym = # when there is a nested proc inside a template, semtmpl # will assign a wrong owner during the first pass over the # template; we must fix it here: see #909 - result.owner = getCurrOwner(c) + result.owner() = getCurrOwner(c) else: result = newSym(kind, considerQuotedIdent(c, n), c.idgen, getCurrOwner(c), n.info) if find(result.name.s, '`') >= 0: diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index b87c854aa5..34f0f8ccae 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -2965,7 +2965,7 @@ proc semBlock(c: PContext, n: PNode; flags: TExprFlags; expectedType: PType = ni if sfGenSym notin labl.flags: addDecl(c, labl) elif labl.owner == nil: - labl.owner = c.p.owner + labl.owner() = c.p.owner n[0] = newSymNode(labl, n[0].info) suggestSym(c.graph, n[0].info, labl, c.graph.usageSym) styleCheckDef(c, labl) diff --git a/compiler/seminst.nim b/compiler/seminst.nim index b9b1a97be1..6d37b5354d 100644 --- a/compiler/seminst.nim +++ b/compiler/seminst.nim @@ -111,7 +111,7 @@ proc freshGenSyms(c: PContext; n: PNode, owner, orig: PSym, symMap: var SymMappi elif s.owner == nil or s.owner.kind == skPackage: #echo "copied this ", s.name.s x = copySym(s, c.idgen) - x.owner = owner + x.owner() = owner idTablePut(symMap, s, x) n.sym = x else: @@ -273,7 +273,7 @@ proc instantiateProcType(c: PContext, pt: LayeredIdTable, internalAssert c.config, originalParams[i].kind == nkSym let oldParam = originalParams[i].sym let param = copySym(oldParam, c.idgen) - param.owner = prc + param.owner() = prc param.typ = result[i] # The default value is instantiated and fitted against the final @@ -395,9 +395,9 @@ proc generateInstance(c: PContext, fn: PSym, pt: LayeredIdTable, let passc = getLocalPassC(c, producer) if passc != "": #pass the local compiler options to the consumer module too extccomp.addLocalCompileOption(c.config, passc, toFullPathConsiderDirty(c.config, c.module.info.fileIndex)) - result.owner = c.module + result.owner() = c.module else: - result.owner = fn + result.owner() = fn result.ast = n pushOwner(c, result) diff --git a/compiler/semmagic.nim b/compiler/semmagic.nim index a12e933e79..e65f1445ad 100644 --- a/compiler/semmagic.nim +++ b/compiler/semmagic.nim @@ -453,7 +453,7 @@ proc turnFinalizerIntoDestructor(c: PContext; orig: PSym; info: TLineInfo): PSym result = copySym(orig, c.idgen) result.info = info result.flags.incl sfFromGeneric - result.owner = orig + result.owner() = orig let origParamType = orig.typ.firstParamType let newParamType = makeVarType(result, origParamType.skipTypes(abstractPtrs), c.idgen) let oldParam = orig.typ.n[1].sym @@ -524,7 +524,7 @@ proc semNewFinalize(c: PContext; n: PNode): PNode = discard "already turned this one into a finalizer" else: if fin.instantiatedFrom != nil and fin.instantiatedFrom != fin.owner: #undo move - fin.owner = fin.instantiatedFrom + fin.owner() = fin.instantiatedFrom let wrapperSym = newSym(skProc, getIdent(c.graph.cache, fin.name.s & "FinalizerWrapper"), c.idgen, fin.owner, fin.info) let selfSymNode = newSymNode(copySym(fin.ast[paramsPos][1][0].sym, c.idgen)) selfSymNode.typ = fin.typ.firstParamType @@ -539,7 +539,7 @@ proc semNewFinalize(c: PContext; n: PNode): PNode = genericParams = fin.ast[genericParamsPos], pragmas = fin.ast[pragmasPos], exceptions = fin.ast[miscPos]), {}) var transFormedSym = turnFinalizerIntoDestructor(c, wrapperSym, wrapper.info) - transFormedSym.owner = fin + transFormedSym.owner() = fin if c.config.backend == backendCpp or sfCompileToCpp in c.module.flags: let origParamType = transFormedSym.ast[bodyPos][1].typ let selfSymbolType = makePtrType(c, origParamType.skipTypes(abstractPtrs)) diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim index 3dad09edfd..506e0694c6 100644 --- a/compiler/semstmts.nim +++ b/compiler/semstmts.nim @@ -908,7 +908,7 @@ proc semVarOrLet(c: PContext, n: PNode, symkind: TSymKind): PNode = if sfGenSym notin v.flags: if not isDiscardUnderscore(v): addInterfaceDecl(c, v) else: - if v.owner == nil: v.owner = c.p.owner + if v.owner == nil: v.owner() = c.p.owner when oKeepVariableNames: if c.inUnrolledContext > 0: v.flags.incl(sfShadowed) else: @@ -1026,7 +1026,7 @@ proc semConst(c: PContext, n: PNode): PNode = when defined(nimsuggest): v.hasUserSpecifiedType = hasUserSpecifiedType if sfGenSym notin v.flags: addInterfaceDecl(c, v) - elif v.owner == nil: v.owner = getCurrOwner(c) + elif v.owner == nil: v.owner() = getCurrOwner(c) styleCheckDef(c, v) onDef(a[j].info, v) @@ -1097,7 +1097,7 @@ proc semForVars(c: PContext, n: PNode; flags: TExprFlags): PNode = v.typ = iter[i] n[0][i] = newSymNode(v) if sfGenSym notin v.flags and not isDiscardUnderscore(v): addDecl(c, v) - elif v.owner == nil: v.owner = getCurrOwner(c) + elif v.owner == nil: v.owner() = getCurrOwner(c) else: var v = symForVar(c, n[0]) if getCurrOwner(c).kind == skModule: incl(v.flags, sfGlobal) @@ -1107,7 +1107,7 @@ proc semForVars(c: PContext, n: PNode; flags: TExprFlags): PNode = v.typ = iterBase n[0] = newSymNode(v) if sfGenSym notin v.flags and not isDiscardUnderscore(v): addDecl(c, v) - elif v.owner == nil: v.owner = getCurrOwner(c) + elif v.owner == nil: v.owner() = getCurrOwner(c) else: localError(c.config, n.info, errWrongNumberOfVariables) elif n.len-2 != iterAfterVarLent.len: @@ -1141,7 +1141,7 @@ proc semForVars(c: PContext, n: PNode; flags: TExprFlags): PNode = v.typ = iter[i][j] n[i][j] = newSymNode(v) if not isDiscardUnderscore(v): addDecl(c, v) - elif v.owner == nil: v.owner = getCurrOwner(c) + elif v.owner == nil: v.owner() = getCurrOwner(c) else: var v = symForVar(c, n[i]) if getCurrOwner(c).kind == skModule: incl(v.flags, sfGlobal) @@ -1156,7 +1156,7 @@ proc semForVars(c: PContext, n: PNode; flags: TExprFlags): PNode = n[i] = newSymNode(v) if sfGenSym notin v.flags: if not isDiscardUnderscore(v): addDecl(c, v) - elif v.owner == nil: v.owner = getCurrOwner(c) + elif v.owner == nil: v.owner() = getCurrOwner(c) inc(c.p.nestedLoopCounter) let oldBreakInLoop = c.p.breakInLoop c.p.breakInLoop = true @@ -1475,7 +1475,7 @@ proc typeDefLeftSidePass(c: PContext, typeSection: PNode, i: int) = s = typsym # add it here, so that recursive types are possible: if sfGenSym notin s.flags: addInterfaceDecl(c, s) - elif s.owner == nil: s.owner = getCurrOwner(c) + elif s.owner == nil: s.owner() = getCurrOwner(c) if name.kind == nkPragmaExpr: if name[0].kind == nkPostfix: @@ -1975,7 +1975,7 @@ proc semInferredLambda(c: PContext, pt: LayeredIdTable, n: PNode): PNode = let original = n[namePos].sym let s = original #copySym(original, false) #incl(s.flags, sfFromGeneric) - #s.owner = original + #s.owner() = original n = replaceTypesInBody(c, pt, n, original) result = n @@ -1990,7 +1990,7 @@ proc semInferredLambda(c: PContext, pt: LayeredIdTable, n: PNode): PNode = tyFromExpr}+tyTypeClasses: localError(c.config, params[i].info, "cannot infer type of parameter: " & params[i].sym.name.s) - #params[i].sym.owner = s + #params[i].sym.owner() = s openScope(c) pushOwner(c, s) addParams(c, params, skProc) @@ -2362,7 +2362,7 @@ proc semProcAux(c: PContext, n: PNode, kind: TSymKind, n[namePos] = newSymNode(s) of nkSym: s = n[namePos].sym - s.owner = c.getCurrOwner + s.owner() = c.getCurrOwner else: # Highlighting needs to be done early so the position for # name isn't changed (see taccent_highlight). We don't want to check if this is the @@ -2629,7 +2629,7 @@ proc semIterator(c: PContext, n: PNode): PNode = # gensym'ed iterator? if n[namePos].kind == nkSym: # gensym'ed iterators might need to become closure iterators: - n[namePos].sym.owner = getCurrOwner(c) + n[namePos].sym.owner() = getCurrOwner(c) n[namePos].sym.transitionRoutineSymKind(skIterator) result = semProcAux(c, n, skIterator, iteratorPragmas) # bug #7093: if after a macro transformation we don't have an diff --git a/compiler/semtypes.nim b/compiler/semtypes.nim index 113946fef2..eb03bffb3f 100644 --- a/compiler/semtypes.nim +++ b/compiler/semtypes.nim @@ -1096,7 +1096,7 @@ proc addParamOrResult(c: PContext, param: PSym, kind: TSymKind) = if sfGenSym in param.flags: # bug #XXX, fix the gensym'ed parameters owner: if param.owner == nil: - param.owner = getCurrOwner(c) + param.owner() = getCurrOwner(c) else: addDecl(c, param) template shouldHaveMeta(t) = diff --git a/compiler/semtypinst.nim b/compiler/semtypinst.nim index ce5e3e9a65..f2964f86bc 100644 --- a/compiler/semtypinst.nim +++ b/compiler/semtypinst.nim @@ -358,7 +358,7 @@ proc replaceTypeVarsS(cl: var TReplTypeVars, s: PSym, t: PType): PSym = result = copySym(s, cl.c.idgen) incl(result.flags, sfFromGeneric) #idTablePut(cl.symMap, s, result) - result.owner = s.owner + result.owner() = s.owner result.typ = t if result.kind != skType: result.ast = replaceTypeVarsN(cl, s.ast) diff --git a/compiler/transf.nim b/compiler/transf.nim index 8dd24e090c..baf87fb98e 100644 --- a/compiler/transf.nim +++ b/compiler/transf.nim @@ -182,7 +182,7 @@ proc freshVar(c: PTransf; v: PSym): PNode = else: var newVar = copySym(v, c.idgen) incl(newVar.flags, sfFromGeneric) - newVar.owner = owner + newVar.owner() = owner result = newSymNode(newVar) proc transformVarSection(c: PTransf, v: PNode): PNode = From a3aea224c9190367fdb98daeeba36fadf73f8d98 Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Tue, 15 Oct 2024 23:32:51 +0800 Subject: [PATCH 16/28] make owner a private field of `PType` (#24314) follow up https://github.com/nim-lang/Nim/pull/24311 --- compiler/ast.nim | 8 ++++---- compiler/ic/ic.nim | 2 +- compiler/semdata.nim | 9 +++++---- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/compiler/ast.nim b/compiler/ast.nim index f665444b17..2b428cec09 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -775,7 +775,7 @@ type # formal param list # for concepts, the concept body # else: unused - owner*: PSym # the 'owner' of the type + ownerField: PSym # the 'owner' of the type sym*: PSym # types have the sym associated with them # it is used for converting types to strings size*: BiggestInt # the size of the type in bytes @@ -814,7 +814,7 @@ type template nodeId(n: PNode): int = cast[int](n) -template owner*(s: PSym): PSym = +template owner*(s: PSym|PType): PSym = s.ownerField type Gconfig = object @@ -1506,7 +1506,7 @@ iterator signature*(t: PType): PType = proc newType*(kind: TTypeKind; idgen: IdGenerator; owner: PSym; son: sink PType = nil): PType = let id = nextTypeId idgen - result = PType(kind: kind, owner: owner, size: defaultSize, + result = PType(kind: kind, ownerField: owner, size: defaultSize, align: defaultAlignment, itemId: id, uniqueId: id, sons: @[]) if son != nil: result.sons.add son @@ -1561,7 +1561,7 @@ proc copyType*(t: PType, idgen: IdGenerator, owner: PSym): PType = result.sym = t.sym # backend-info should not be copied proc exactReplica*(t: PType): PType = - result = PType(kind: t.kind, owner: t.owner, size: defaultSize, + result = PType(kind: t.kind, ownerField: t.owner, size: defaultSize, align: defaultAlignment, itemId: t.itemId, uniqueId: t.uniqueId) assignType(result, t) diff --git a/compiler/ic/ic.nim b/compiler/ic/ic.nim index 23cff281b7..238825ac56 100644 --- a/compiler/ic/ic.nim +++ b/compiler/ic/ic.nim @@ -998,7 +998,7 @@ proc typeHeaderFromPacked(c: var PackedDecoder; g: var PackedModuleGraph; proc typeBodyFromPacked(c: var PackedDecoder; g: var PackedModuleGraph; t: PackedType; si, item: int32; result: PType) = result.sym = loadSym(c, g, si, t.sym) - result.owner = loadSym(c, g, si, t.owner) + result.owner() = loadSym(c, g, si, t.owner) when false: for op, item in pairs t.attachedOps: result.attachedOps[op] = loadSym(c, g, si, item) diff --git a/compiler/semdata.nim b/compiler/semdata.nim index 16d3d6f5b8..1888d8f6e1 100644 --- a/compiler/semdata.nim +++ b/compiler/semdata.nim @@ -529,10 +529,11 @@ template localErrorNode*(c: PContext, n: PNode, arg: string): PNode = liMessage(c.config, n2.info, errGenerated, arg, doNothing, instLoc()) errorNode(c, n2) -proc fillTypeS*(dest: PType, kind: TTypeKind, c: PContext) = - dest.kind = kind - dest.owner = getCurrOwner(c) - dest.size = - 1 +when false: + proc fillTypeS*(dest: PType, kind: TTypeKind, c: PContext) = + dest.kind = kind + dest.owner = getCurrOwner(c) + dest.size = - 1 proc makeRangeType*(c: PContext; first, last: BiggestInt; info: TLineInfo; intType: PType = nil): PType = From 4a056b184921eb9d6f6a0508da0190f7443d89bf Mon Sep 17 00:00:00 2001 From: metagn Date: Wed, 16 Oct 2024 21:48:53 +0300 Subject: [PATCH 17/28] cbuilder: implement designated initializers, finish default value braces (#24312) follows up #24259 This is the remaining missing use of `StructInitializer` in `getDefaultValue` after #24259 and #24302. The only remaining direct C code in getDefaultValue is [this line](https://github.com/nim-lang/Nim/blob/922f7dfd712130bb68a16336085c0320afceb273/compiler/ccgexprs.nim#L3525) which creates a global array variable, which isn't implemented yet. Next steps would be all remaining variable and `typedef` declarations, then hopefully we can move on to general statements and expressions. --- compiler/cbuilderdecls.nim | 10 ++++++---- compiler/ccgexprs.nim | 35 ++++++++++++++++++++++------------- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/compiler/cbuilderdecls.nim b/compiler/cbuilderdecls.nim index e3c1c31075..e5032d7431 100644 --- a/compiler/cbuilderdecls.nim +++ b/compiler/cbuilderdecls.nim @@ -77,9 +77,7 @@ type needsComma: bool proc initStructInitializer(builder: var Builder, kind: StructInitializerKind): StructInitializer = - ## starts building a struct initializer, `orderCompliant = true` means - ## built fields must be ordered correctly - assert kind != siNamedStruct, "named struct constructors unimplemented" + ## starts building a struct initializer, i.e. braced initializer list result = StructInitializer(kind: kind, needsComma: false) if kind != siWrapper: builder.add("{") @@ -100,7 +98,11 @@ template addField(builder: var Builder, constr: var StructInitializer, name: str assert name.len != 0, "name has to be given for struct initializer field" valueBody of siNamedStruct: - assert false, "named struct constructors unimplemented" + assert name.len != 0, "name has to be given for struct initializer field" + builder.add(".") + builder.add(name) + builder.add(" = ") + valueBody proc finishStructInitializer(builder: var Builder, constr: StructInitializer) = ## finishes building a struct initializer diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim index d59dd7aec9..88d177800b 100644 --- a/compiler/ccgexprs.nim +++ b/compiler/ccgexprs.nim @@ -3289,26 +3289,35 @@ proc getNullValueAux(p: BProc; t: PType; obj, constOrNil: PNode, break let selectedBranch = caseObjDefaultBranch(obj, branch) - # XXX siNamedStruct needs to be implemented to replace `res` here - var res = "{" - var branchInit: StructInitializer let b = lastSon(obj[selectedBranch]) # designated initilization is the only way to init non first element of unions # branches are allowed to have no members (b.len == 0), in this case they don't need initializer + var fieldName: string = "" if b.kind == nkRecList and not isEmptyCaseObjectBranch(b): - res.add "._" & mangleRecFieldName(p.module, obj[0].sym) & "_" & $selectedBranch & " = " - res.addStructInitializer(branchInit, kind = siOrderedStruct): - getNullValueAux(p, t, b, constOrNil, res, branchInit, isConst, info) + fieldName = "_" & mangleRecFieldName(p.module, obj[0].sym) & "_" & $selectedBranch + result.addField(init, name = ""): + # XXX figure out name for the union, see use of `addAnonUnion` + var branchInit: StructInitializer + result.addStructInitializer(branchInit, kind = siNamedStruct): + result.addField(branchInit, name = fieldName): + var branchObjInit: StructInitializer + result.addStructInitializer(branchObjInit, kind = siOrderedStruct): + getNullValueAux(p, t, b, constOrNil, result, branchObjInit, isConst, info) elif b.kind == nkSym: - res.add "." & mangleRecFieldName(p.module, b.sym) & " = " - res.addStructInitializer(branchInit, kind = siWrapper): - getNullValueAux(p, t, b, constOrNil, res, branchInit, isConst, info) + fieldName = mangleRecFieldName(p.module, b.sym) + result.addField(init, name = ""): + # XXX figure out name for the union, see use of `addAnonUnion` + var branchInit: StructInitializer + result.addStructInitializer(branchInit, kind = siNamedStruct): + result.addField(branchInit, name = fieldName): + # we need to generate the default value of the single sym, + # to do this create a dummy wrapper initializer and recurse + var branchFieldInit: StructInitializer + result.addStructInitializer(branchFieldInit, kind = siWrapper): + getNullValueAux(p, t, b, constOrNil, result, branchFieldInit, isConst, info) else: + # no fields, don't initialize return - result.addField(init, name = ""): - # XXX figure out name for the union, see use of `addAnonUnion` - result.add res - result.add "}" of nkSym: let field = obj.sym From 8be82c36c97d51a183633249d284afd0a621b0fd Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Thu, 17 Oct 2024 02:49:31 +0800 Subject: [PATCH 18/28] fixes #18896; fixes #20886; importc types alias doesn't work with distinct (#24313) fixes #18896 fixes #20886 ```nim type PFile {.importc: "FILE*", header: "".} = distinct pointer # import C's FILE* type; Nim will treat it as a new pointer type ``` This is an excerpt from the Nim manual. In the old Nim versions, it produces a void pointer type instead of the `FILE*` type that should have been generated. Because these C types tend to be opaque and adapt to changes on different platforms. It might affect the portability of Nim on various OS, i.e. `csource_v2` cannot build on the apline platform because of `Time` relies on Nim types instead of the `time_t` type. ref https://github.com/nim-lang/Nim/pull/18851 --- compiler/ccgtypes.nim | 10 ++++++++-- tests/ccgbugs/timportc_distinct.nim | 13 +++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 tests/ccgbugs/timportc_distinct.nim diff --git a/compiler/ccgtypes.nim b/compiler/ccgtypes.nim index 154ab7475d..2337c70e63 100644 --- a/compiler/ccgtypes.nim +++ b/compiler/ccgtypes.nim @@ -340,7 +340,12 @@ proc getSimpleTypeDesc(m: BModule; typ: PType): Rope = of tyNil: result = typeNameOrLiteral(m, typ, "void*") of tyInt..tyUInt64: result = typeNameOrLiteral(m, typ, NumericalTypeToStr[typ.kind]) - of tyDistinct, tyRange, tyOrdinal: result = getSimpleTypeDesc(m, typ.skipModifier) + of tyRange, tyOrdinal: result = getSimpleTypeDesc(m, typ.skipModifier) + of tyDistinct: + result = getSimpleTypeDesc(m, typ.skipModifier) + if isImportedType(typ) and result != "": + useHeader(m, typ.sym) + result = typ.sym.loc.snippet of tyStatic: if typ.n != nil: result = getSimpleTypeDesc(m, skipModifier typ) else: @@ -861,7 +866,8 @@ proc getTypeDescAux(m: BModule; origTyp: PType, check: var IntSet; kind: TypeDes if t != origTyp and origTyp.sym != nil: useHeader(m, origTyp.sym) let sig = hashType(origTyp, m.config) - result = getTypePre(m, t, sig) + # tyDistinct matters if it is an importc type + result = getTypePre(m, origTyp.skipTypes(irrelevantForBackend-{tyOwned, tyDistinct}), sig) defer: # defer is the simplest in this case if isImportedType(t) and not m.typeABICache.containsOrIncl(sig): addAbiCheck(m, t, result) diff --git a/tests/ccgbugs/timportc_distinct.nim b/tests/ccgbugs/timportc_distinct.nim new file mode 100644 index 0000000000..afeadf33f9 --- /dev/null +++ b/tests/ccgbugs/timportc_distinct.nim @@ -0,0 +1,13 @@ +discard """ + ccodecheck: "time_t" + joinable: false +""" + +type + Time* {.importc: "time_t", header: "".} = distinct clong + +proc foo = + var s: Time = default(Time) + discard s + +foo() From d0b6b9346ec89baf67bb8d4a5dcf1b5c1b20a194 Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Thu, 17 Oct 2024 21:16:57 +0800 Subject: [PATCH 19/28] adds a getter/setter for `owner` (#24318) --- compiler/ast.nim | 7 +++++-- compiler/evaltempl.nim | 2 +- compiler/ic/ic.nim | 6 +++--- compiler/modules.nim | 2 +- compiler/sem.nim | 2 +- compiler/semexprs.nim | 2 +- compiler/seminst.nim | 8 ++++---- compiler/semmagic.nim | 6 +++--- compiler/semstmts.nim | 18 +++++++++--------- compiler/semtypes.nim | 2 +- compiler/semtypinst.nim | 2 +- compiler/transf.nim | 2 +- 12 files changed, 31 insertions(+), 28 deletions(-) diff --git a/compiler/ast.nim b/compiler/ast.nim index 2b428cec09..6754f11409 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -814,8 +814,11 @@ type template nodeId(n: PNode): int = cast[int](n) -template owner*(s: PSym|PType): PSym = - s.ownerField +proc owner*(s: PSym|PType): PSym {.inline.} = + result = s.ownerField + +proc setOwner*(s: PSym|PType, owner: PSym) {.inline.} = + s.ownerField = owner type Gconfig = object # we put comments in a side channel to avoid increasing `sizeof(TNode)`, which diff --git a/compiler/evaltempl.nim b/compiler/evaltempl.nim index 199a047d4a..d2e6046094 100644 --- a/compiler/evaltempl.nim +++ b/compiler/evaltempl.nim @@ -64,7 +64,7 @@ proc evalTemplateAux(templ, actual: PNode, c: var TemplCtx, result: PNode) = if x == nil: x = copySym(s, c.idgen) # sem'check needs to set the owner properly later, see bug #9476 - x.owner() = nil # c.genSymOwner + setOwner(x, nil) # c.genSymOwner #if x.kind == skParam and x.owner.kind == skModule: # internalAssert c.config, false idTablePut(c.mapping, s, x) diff --git a/compiler/ic/ic.nim b/compiler/ic/ic.nim index 238825ac56..3a1acd4b16 100644 --- a/compiler/ic/ic.nim +++ b/compiler/ic/ic.nim @@ -942,7 +942,7 @@ proc symBodyFromPacked(c: var PackedDecoder; g: var PackedModuleGraph; result.guard = loadSym(c, g, si, s.guard) result.bitsize = s.bitsize result.alignment = s.alignment - result.owner() = loadSym(c, g, si, s.owner) + setOwner(result, loadSym(c, g, si, s.owner)) let externalName = g[si].fromDisk.strings[s.externalName] if externalName != "": result.loc.snippet = externalName @@ -998,7 +998,7 @@ proc typeHeaderFromPacked(c: var PackedDecoder; g: var PackedModuleGraph; proc typeBodyFromPacked(c: var PackedDecoder; g: var PackedModuleGraph; t: PackedType; si, item: int32; result: PType) = result.sym = loadSym(c, g, si, t.sym) - result.owner() = loadSym(c, g, si, t.owner) + setOwner(result, loadSym(c, g, si, t.owner)) when false: for op, item in pairs t.attachedOps: result.attachedOps[op] = loadSym(c, g, si, item) @@ -1062,7 +1062,7 @@ proc setupLookupTables(g: var PackedModuleGraph; conf: ConfigRef; cache: IdentCa name: getIdent(cache, splitFile(filename).name), info: newLineInfo(fileIdx, 1, 1), position: int(fileIdx)) - m.module.owner() = getPackage(conf, cache, fileIdx) + setOwner(m.module, getPackage(conf, cache, fileIdx)) m.module.flags = m.fromDisk.moduleFlags proc loadToReplayNodes(g: var PackedModuleGraph; conf: ConfigRef; cache: IdentCache; diff --git a/compiler/modules.nim b/compiler/modules.nim index 6b893f11ee..7f56119ccc 100644 --- a/compiler/modules.nim +++ b/compiler/modules.nim @@ -25,7 +25,7 @@ template getModuleIdent(graph: ModuleGraph, filename: AbsoluteFile): PIdent = proc partialInitModule*(result: PSym; graph: ModuleGraph; fileIdx: FileIndex; filename: AbsoluteFile) = let packSym = getPackage(graph, fileIdx) - result.owner() = packSym + setOwner(result, packSym) result.position = int fileIdx proc newModule*(graph: ModuleGraph; fileIdx: FileIndex): PSym = diff --git a/compiler/sem.nim b/compiler/sem.nim index 43da922854..86f92f851e 100644 --- a/compiler/sem.nim +++ b/compiler/sem.nim @@ -256,7 +256,7 @@ proc newSymG*(kind: TSymKind, n: PNode, c: PContext): PSym = # when there is a nested proc inside a template, semtmpl # will assign a wrong owner during the first pass over the # template; we must fix it here: see #909 - result.owner() = getCurrOwner(c) + setOwner(result, getCurrOwner(c)) else: result = newSym(kind, considerQuotedIdent(c, n), c.idgen, getCurrOwner(c), n.info) if find(result.name.s, '`') >= 0: diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 34f0f8ccae..ef389c7ca4 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -2965,7 +2965,7 @@ proc semBlock(c: PContext, n: PNode; flags: TExprFlags; expectedType: PType = ni if sfGenSym notin labl.flags: addDecl(c, labl) elif labl.owner == nil: - labl.owner() = c.p.owner + setOwner(labl, c.p.owner) n[0] = newSymNode(labl, n[0].info) suggestSym(c.graph, n[0].info, labl, c.graph.usageSym) styleCheckDef(c, labl) diff --git a/compiler/seminst.nim b/compiler/seminst.nim index 6d37b5354d..00f81a2f27 100644 --- a/compiler/seminst.nim +++ b/compiler/seminst.nim @@ -111,7 +111,7 @@ proc freshGenSyms(c: PContext; n: PNode, owner, orig: PSym, symMap: var SymMappi elif s.owner == nil or s.owner.kind == skPackage: #echo "copied this ", s.name.s x = copySym(s, c.idgen) - x.owner() = owner + setOwner(x, owner) idTablePut(symMap, s, x) n.sym = x else: @@ -273,7 +273,7 @@ proc instantiateProcType(c: PContext, pt: LayeredIdTable, internalAssert c.config, originalParams[i].kind == nkSym let oldParam = originalParams[i].sym let param = copySym(oldParam, c.idgen) - param.owner() = prc + setOwner(param, prc) param.typ = result[i] # The default value is instantiated and fitted against the final @@ -395,9 +395,9 @@ proc generateInstance(c: PContext, fn: PSym, pt: LayeredIdTable, let passc = getLocalPassC(c, producer) if passc != "": #pass the local compiler options to the consumer module too extccomp.addLocalCompileOption(c.config, passc, toFullPathConsiderDirty(c.config, c.module.info.fileIndex)) - result.owner() = c.module + setOwner(result, c.module) else: - result.owner() = fn + setOwner(result, fn) result.ast = n pushOwner(c, result) diff --git a/compiler/semmagic.nim b/compiler/semmagic.nim index e65f1445ad..cf5cfad6d8 100644 --- a/compiler/semmagic.nim +++ b/compiler/semmagic.nim @@ -453,7 +453,7 @@ proc turnFinalizerIntoDestructor(c: PContext; orig: PSym; info: TLineInfo): PSym result = copySym(orig, c.idgen) result.info = info result.flags.incl sfFromGeneric - result.owner() = orig + setOwner(result, orig) let origParamType = orig.typ.firstParamType let newParamType = makeVarType(result, origParamType.skipTypes(abstractPtrs), c.idgen) let oldParam = orig.typ.n[1].sym @@ -524,7 +524,7 @@ proc semNewFinalize(c: PContext; n: PNode): PNode = discard "already turned this one into a finalizer" else: if fin.instantiatedFrom != nil and fin.instantiatedFrom != fin.owner: #undo move - fin.owner() = fin.instantiatedFrom + setOwner(fin, fin.instantiatedFrom) let wrapperSym = newSym(skProc, getIdent(c.graph.cache, fin.name.s & "FinalizerWrapper"), c.idgen, fin.owner, fin.info) let selfSymNode = newSymNode(copySym(fin.ast[paramsPos][1][0].sym, c.idgen)) selfSymNode.typ = fin.typ.firstParamType @@ -539,7 +539,7 @@ proc semNewFinalize(c: PContext; n: PNode): PNode = genericParams = fin.ast[genericParamsPos], pragmas = fin.ast[pragmasPos], exceptions = fin.ast[miscPos]), {}) var transFormedSym = turnFinalizerIntoDestructor(c, wrapperSym, wrapper.info) - transFormedSym.owner() = fin + setOwner(transFormedSym, fin) if c.config.backend == backendCpp or sfCompileToCpp in c.module.flags: let origParamType = transFormedSym.ast[bodyPos][1].typ let selfSymbolType = makePtrType(c, origParamType.skipTypes(abstractPtrs)) diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim index 506e0694c6..2bf5877402 100644 --- a/compiler/semstmts.nim +++ b/compiler/semstmts.nim @@ -908,7 +908,7 @@ proc semVarOrLet(c: PContext, n: PNode, symkind: TSymKind): PNode = if sfGenSym notin v.flags: if not isDiscardUnderscore(v): addInterfaceDecl(c, v) else: - if v.owner == nil: v.owner() = c.p.owner + if v.owner == nil: setOwner(v, c.p.owner) when oKeepVariableNames: if c.inUnrolledContext > 0: v.flags.incl(sfShadowed) else: @@ -1026,7 +1026,7 @@ proc semConst(c: PContext, n: PNode): PNode = when defined(nimsuggest): v.hasUserSpecifiedType = hasUserSpecifiedType if sfGenSym notin v.flags: addInterfaceDecl(c, v) - elif v.owner == nil: v.owner() = getCurrOwner(c) + elif v.owner == nil: setOwner(v, getCurrOwner(c)) styleCheckDef(c, v) onDef(a[j].info, v) @@ -1097,7 +1097,7 @@ proc semForVars(c: PContext, n: PNode; flags: TExprFlags): PNode = v.typ = iter[i] n[0][i] = newSymNode(v) if sfGenSym notin v.flags and not isDiscardUnderscore(v): addDecl(c, v) - elif v.owner == nil: v.owner() = getCurrOwner(c) + elif v.owner == nil: setOwner(v, getCurrOwner(c)) else: var v = symForVar(c, n[0]) if getCurrOwner(c).kind == skModule: incl(v.flags, sfGlobal) @@ -1107,7 +1107,7 @@ proc semForVars(c: PContext, n: PNode; flags: TExprFlags): PNode = v.typ = iterBase n[0] = newSymNode(v) if sfGenSym notin v.flags and not isDiscardUnderscore(v): addDecl(c, v) - elif v.owner == nil: v.owner() = getCurrOwner(c) + elif v.owner == nil: setOwner(v, getCurrOwner(c)) else: localError(c.config, n.info, errWrongNumberOfVariables) elif n.len-2 != iterAfterVarLent.len: @@ -1141,7 +1141,7 @@ proc semForVars(c: PContext, n: PNode; flags: TExprFlags): PNode = v.typ = iter[i][j] n[i][j] = newSymNode(v) if not isDiscardUnderscore(v): addDecl(c, v) - elif v.owner == nil: v.owner() = getCurrOwner(c) + elif v.owner == nil: setOwner(v, getCurrOwner(c)) else: var v = symForVar(c, n[i]) if getCurrOwner(c).kind == skModule: incl(v.flags, sfGlobal) @@ -1156,7 +1156,7 @@ proc semForVars(c: PContext, n: PNode; flags: TExprFlags): PNode = n[i] = newSymNode(v) if sfGenSym notin v.flags: if not isDiscardUnderscore(v): addDecl(c, v) - elif v.owner == nil: v.owner() = getCurrOwner(c) + elif v.owner == nil: setOwner(v, getCurrOwner(c)) inc(c.p.nestedLoopCounter) let oldBreakInLoop = c.p.breakInLoop c.p.breakInLoop = true @@ -1475,7 +1475,7 @@ proc typeDefLeftSidePass(c: PContext, typeSection: PNode, i: int) = s = typsym # add it here, so that recursive types are possible: if sfGenSym notin s.flags: addInterfaceDecl(c, s) - elif s.owner == nil: s.owner() = getCurrOwner(c) + elif s.owner == nil: setOwner(s, getCurrOwner(c)) if name.kind == nkPragmaExpr: if name[0].kind == nkPostfix: @@ -2362,7 +2362,7 @@ proc semProcAux(c: PContext, n: PNode, kind: TSymKind, n[namePos] = newSymNode(s) of nkSym: s = n[namePos].sym - s.owner() = c.getCurrOwner + setOwner(s, c.getCurrOwner) else: # Highlighting needs to be done early so the position for # name isn't changed (see taccent_highlight). We don't want to check if this is the @@ -2629,7 +2629,7 @@ proc semIterator(c: PContext, n: PNode): PNode = # gensym'ed iterator? if n[namePos].kind == nkSym: # gensym'ed iterators might need to become closure iterators: - n[namePos].sym.owner() = getCurrOwner(c) + setOwner(n[namePos].sym, getCurrOwner(c)) n[namePos].sym.transitionRoutineSymKind(skIterator) result = semProcAux(c, n, skIterator, iteratorPragmas) # bug #7093: if after a macro transformation we don't have an diff --git a/compiler/semtypes.nim b/compiler/semtypes.nim index eb03bffb3f..6e509ddc1f 100644 --- a/compiler/semtypes.nim +++ b/compiler/semtypes.nim @@ -1096,7 +1096,7 @@ proc addParamOrResult(c: PContext, param: PSym, kind: TSymKind) = if sfGenSym in param.flags: # bug #XXX, fix the gensym'ed parameters owner: if param.owner == nil: - param.owner() = getCurrOwner(c) + setOwner(param, getCurrOwner(c)) else: addDecl(c, param) template shouldHaveMeta(t) = diff --git a/compiler/semtypinst.nim b/compiler/semtypinst.nim index f2964f86bc..09cf422b49 100644 --- a/compiler/semtypinst.nim +++ b/compiler/semtypinst.nim @@ -358,7 +358,7 @@ proc replaceTypeVarsS(cl: var TReplTypeVars, s: PSym, t: PType): PSym = result = copySym(s, cl.c.idgen) incl(result.flags, sfFromGeneric) #idTablePut(cl.symMap, s, result) - result.owner() = s.owner + setOwner(result, s.owner) result.typ = t if result.kind != skType: result.ast = replaceTypeVarsN(cl, s.ast) diff --git a/compiler/transf.nim b/compiler/transf.nim index baf87fb98e..cfeff18cac 100644 --- a/compiler/transf.nim +++ b/compiler/transf.nim @@ -182,7 +182,7 @@ proc freshVar(c: PTransf; v: PSym): PNode = else: var newVar = copySym(v, c.idgen) incl(newVar.flags, sfFromGeneric) - newVar.owner() = owner + setOwner(newVar, owner) result = newSymNode(newVar) proc transformVarSection(c: PTransf, v: PNode): PNode = From 0347536ff2649e889cb11606a80293bf61043604 Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Fri, 18 Oct 2024 10:56:37 +0800 Subject: [PATCH 20/28] fixes #24319; `move` doesn't work well with (deref (var array)) (#24321) fixes #24319 `byRefLoc` (`mapType`) requires the Loc `a` to have the right type. Without `lfEnforceDeref`, it produces the wrong type for `deref (var array)`, which may come from `mitems`. --- compiler/ccgexprs.nim | 2 +- tests/destructor/tmove.nim | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim index 88d177800b..dcf0c4c931 100644 --- a/compiler/ccgexprs.nim +++ b/compiler/ccgexprs.nim @@ -2321,7 +2321,7 @@ proc genWasMoved(p: BProc; n: PNode) = # [addrLoc(p.config, a), getTypeDesc(p.module, a.t)]) proc genMove(p: BProc; n: PNode; d: var TLoc) = - var a: TLoc = initLocExpr(p, n[1].skipAddr) + var a: TLoc = initLocExpr(p, n[1].skipAddr, {lfEnforceDeref}) if n.len == 4: # generated by liftdestructors: var src: TLoc = initLocExpr(p, n[2]) diff --git a/tests/destructor/tmove.nim b/tests/destructor/tmove.nim index 2762aff900..da91442def 100644 --- a/tests/destructor/tmove.nim +++ b/tests/destructor/tmove.nim @@ -16,3 +16,16 @@ block: doAssert s == 2 foo() + +import std/deques + +block: # bug #24319 + var queue = initDeque[array[32, byte]]() + for i in 0 ..< 5: + let element: array[32, byte] = [ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 1, + ] + queue.addLast(element) + + doAssert queue.popLast[^1] == byte(1) From 52cf7dfde01ca70e867607c2df7de66263c51025 Mon Sep 17 00:00:00 2001 From: metagn Date: Fri, 18 Oct 2024 08:37:05 +0300 Subject: [PATCH 21/28] shallow fold prevention for `addr`, `nkHiddenAddr` (#24322) fixes #24305, refs #23807 Since #23014 `nkHiddenAddr` is produced to fast assign array elements in iterators. However the array access inside this `nkHiddenAddr` can get folded at compile time, generating invalid code. In #23807, compile time folding of regular `addr` expressions was changed to be prevented in `transf` but `nkHiddenAddr` was not updated alongside it. The method for preventing folding in `addr` in #23807 was also faulty, it should only trigger on the immediate child node of the address rather than all nodes nested inside it. This caused a regression as outlined in [this comment](https://github.com/nim-lang/Nim/pull/24322#issuecomment-2419560182). To fix both issues, `addr` and `nkHiddenAddr` now both shallowly prevent constant folding for their immediate children. --- compiler/transf.nim | 22 +++++++++------------- tests/iter/tfoldedaddr.nim | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 13 deletions(-) create mode 100644 tests/iter/tfoldedaddr.nim diff --git a/compiler/transf.nim b/compiler/transf.nim index cfeff18cac..7ee9683743 100644 --- a/compiler/transf.nim +++ b/compiler/transf.nim @@ -56,7 +56,6 @@ type contSyms, breakSyms: seq[PSym] # to transform 'continue' and 'break' deferDetected, tooEarly: bool isIntroducingNewLocalVars: bool # true if we are in `introducingNewLocalVars` (don't transform yields) - inAddr: bool flags: TransformFlags graph: ModuleGraph idgen: IdGenerator @@ -103,12 +102,12 @@ proc newTemp(c: PTransf, typ: PType, info: TLineInfo): PNode = else: result = newSymNode(r) -proc transform(c: PTransf, n: PNode): PNode +proc transform(c: PTransf, n: PNode, noConstFold = false): PNode -proc transformSons(c: PTransf, n: PNode): PNode = +proc transformSons(c: PTransf, n: PNode, noConstFold = false): PNode = result = newTransNode(n) for i in 0.. Date: Fri, 18 Oct 2024 14:39:20 +0900 Subject: [PATCH 22/28] Document about noinline calling convention and exportcpp pragma in Nim manual (#24323) It seems exportcpp was implemented in v1.0 but there is no documentation about it excepts changelog. `noinline` is used in many procedures in Nim code but there is also no documentation about it. --------- Co-authored-by: Andreas Rumpf --- doc/manual.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/doc/manual.md b/doc/manual.md index 236c19b0e7..d3c81aa78f 100644 --- a/doc/manual.md +++ b/doc/manual.md @@ -2241,6 +2241,10 @@ Nim supports these `calling conventions`:idx:\: only a hint for the compiler: it may completely ignore it, and it may inline procedures that are not marked as `inline`. +`noinline`:idx: +: The backend compiler may inline procedures that are not marked as `inline`. + The noinline convention prevents it. + `fastcall`:idx: : Fastcall means different things to different C compilers. One gets whatever the C `__fastcall` means. @@ -8646,6 +8650,14 @@ pragma should be used in addition to the `exportc` pragma. See [Dynlib pragma for export]. +Exportcpp pragma +---------------- +The `exportcpp` pragma works like the `exportc` pragma but it requires the `cpp` backend. +When compiled with the `cpp` backend, the `exportc` pragma adds `export "C"` to +the declaration in the generated code so that it can be called from both C and +C++ code. `exportcpp` pragma doesn't add `export "C"`. + + Extern pragma ------------- Like `exportc` or `importc`, the `extern` pragma affects name From 5fa96ef270a5137510d9a21e91013cea70f4c63f Mon Sep 17 00:00:00 2001 From: Yuriy Glukhov Date: Fri, 18 Oct 2024 10:36:41 +0200 Subject: [PATCH 23/28] Fixes #3824, fixes #19154, and hopefully #24094. Re-applies #23787. (#24316) The first commit reverts the revert of #23787. The second fixes lambdalifting in convolutedly nested closures/closureiters. This is considered to be the reason of #24094, though I can't tell for sure, as I was not able to reproduce #24094 for complicated but irrelevant reasons. Therefore I ask @jmgomez, @metagn or anyone who could reproduce it to try it again with this PR. I would suggest this PR to not be squashed if possible, as the history is already messy enough. Some theory behind the lambdalifting fix: - A closureiter that captures anything outside its body will always have `:up` in its env. This property is now used as a trigger to lift any proc that captures such a closureiter. - Instantiating a closureiter involves filling out its `:up`, which was previously done incorrectly. The fixed algorithm is to use "current" env if it is the owner of the iter declaration, or traverse through `:up`s of env param until the common ancestor is found. --------- Co-authored-by: Andreas Rumpf --- compiler/closureiters.nim | 126 +++------------- compiler/lambdalifting.nim | 76 +++++----- compiler/transf.nim | 16 +- tests/destructor/tuse_ownedref_after_move.nim | 2 +- tests/iter/tnestedclosures.nim | 139 ++++++++++++++++++ tests/iter/tyieldintry.nim | 4 +- 6 files changed, 204 insertions(+), 159 deletions(-) create mode 100644 tests/iter/tnestedclosures.nim diff --git a/compiler/closureiters.nim b/compiler/closureiters.nim index 8bdd04ca78..9ee394111b 100644 --- a/compiler/closureiters.nim +++ b/compiler/closureiters.nim @@ -161,10 +161,6 @@ type # is their finally. For finally it is parent finally. Otherwise -1 idgen: IdGenerator varStates: Table[ItemId, int] # Used to detect if local variable belongs to multiple states - stateVarSym: PSym # :state variable. nil if env already introduced by lambdalifting - # remove if -d:nimOptIters is default, treating it as always nil - nimOptItersEnabled: bool # tracks if -d:nimOptIters is enabled - # should be default when issues are fixed, see #24094 const nkSkip = {nkEmpty..nkNilLit, nkTemplateDef, nkTypeSection, nkStaticStmt, @@ -174,11 +170,8 @@ const localRequiresLifting = -2 proc newStateAccess(ctx: var Ctx): PNode = - if ctx.stateVarSym.isNil: - result = rawIndirectAccess(newSymNode(getEnvParam(ctx.fn)), + result = rawIndirectAccess(newSymNode(getEnvParam(ctx.fn)), getStateField(ctx.g, ctx.fn), ctx.fn.info) - else: - result = newSymNode(ctx.stateVarSym) proc newStateAssgn(ctx: var Ctx, toValue: PNode): PNode = # Creates state assignment: @@ -196,22 +189,12 @@ proc newEnvVar(ctx: var Ctx, name: string, typ: PType): PSym = result.flags.incl sfNoInit assert(not typ.isNil, "Env var needs a type") - if not ctx.stateVarSym.isNil: - # We haven't gone through labmda lifting yet, so just create a local var, - # it will be lifted later - if ctx.tempVars.isNil: - ctx.tempVars = newNodeI(nkVarSection, ctx.fn.info) - addVar(ctx.tempVars, newSymNode(result)) - else: - let envParam = getEnvParam(ctx.fn) - # let obj = envParam.typ.lastSon - result = addUniqueField(envParam.typ.elementType, result, ctx.g.cache, ctx.idgen) + let envParam = getEnvParam(ctx.fn) + # let obj = envParam.typ.lastSon + result = addUniqueField(envParam.typ.elementType, result, ctx.g.cache, ctx.idgen) proc newEnvVarAccess(ctx: Ctx, s: PSym): PNode = - if ctx.stateVarSym.isNil: - result = rawIndirectAccess(newSymNode(getEnvParam(ctx.fn)), s, ctx.fn.info) - else: - result = newSymNode(s) + result = rawIndirectAccess(newSymNode(getEnvParam(ctx.fn)), s, ctx.fn.info) proc newTempVarAccess(ctx: Ctx, s: PSym): PNode = result = newSymNode(s, ctx.fn.info) @@ -263,20 +246,12 @@ proc newTempVarDef(ctx: Ctx, s: PSym, initialValue: PNode): PNode = v = ctx.g.emptyNode newTree(nkVarSection, newTree(nkIdentDefs, newSymNode(s), ctx.g.emptyNode, v)) -proc newEnvVarAsgn(ctx: Ctx, s: PSym, v: PNode): PNode - proc newTempVar(ctx: var Ctx, typ: PType, parent: PNode, initialValue: PNode = nil): PSym = - if ctx.nimOptItersEnabled: - result = newSym(skVar, getIdent(ctx.g.cache, ":tmpSlLower" & $ctx.tempVarId), ctx.idgen, ctx.fn, ctx.fn.info) - else: - result = ctx.newEnvVar(":tmpSlLower" & $ctx.tempVarId, typ) + result = newSym(skVar, getIdent(ctx.g.cache, ":tmpSlLower" & $ctx.tempVarId), ctx.idgen, ctx.fn, ctx.fn.info) inc ctx.tempVarId result.typ = typ assert(not typ.isNil, "Temp var needs a type") - if ctx.nimOptItersEnabled: - parent.add(ctx.newTempVarDef(result, initialValue)) - elif initialValue != nil: - parent.add(ctx.newEnvVarAsgn(result, initialValue)) + parent.add(ctx.newTempVarDef(result, initialValue)) proc hasYields(n: PNode): bool = # TODO: This is very inefficient. It traverses the node, looking for nkYieldStmt. @@ -455,24 +430,13 @@ proc newTempVarAsgn(ctx: Ctx, s: PSym, v: PNode): PNode = result = newTree(nkFastAsgn, ctx.newTempVarAccess(s), v) result.info = v.info -proc newEnvVarAsgn(ctx: Ctx, s: PSym, v: PNode): PNode = - # unused with -d:nimOptIters - if isEmptyType(v.typ): - result = v - else: - result = newTree(nkFastAsgn, ctx.newEnvVarAccess(s), v) - result.info = v.info - proc addExprAssgn(ctx: Ctx, output, input: PNode, sym: PSym) = - var input = input if input.kind == nkStmtListExpr: let (st, res) = exprToStmtList(input) output.add(st) - input = res - if ctx.nimOptItersEnabled: - output.add(ctx.newTempVarAsgn(sym, input)) + output.add(ctx.newTempVarAsgn(sym, res)) else: - output.add(ctx.newEnvVarAsgn(sym, input)) + output.add(ctx.newTempVarAsgn(sym, input)) proc convertExprBodyToAsgn(ctx: Ctx, exprBody: PNode, res: PSym): PNode = result = newNodeI(nkStmtList, exprBody.info) @@ -601,11 +565,7 @@ proc lowerStmtListExprs(ctx: var Ctx, n: PNode, needsSplit: var bool): PNode = else: internalError(ctx.g.config, "lowerStmtListExpr(nkIf): " & $branch.kind) - if isExpr: - if ctx.nimOptItersEnabled: - result.add(ctx.newTempVarAccess(tmp)) - else: - result.add(ctx.newEnvVarAccess(tmp)) + if isExpr: result.add(ctx.newTempVarAccess(tmp)) of nkTryStmt, nkHiddenTryStmt: var ns = false @@ -635,10 +595,7 @@ proc lowerStmtListExprs(ctx: var Ctx, n: PNode, needsSplit: var bool): PNode = else: internalError(ctx.g.config, "lowerStmtListExpr(nkTryStmt): " & $branch.kind) result.add(n) - if ctx.nimOptItersEnabled: - result.add(ctx.newTempVarAccess(tmp)) - else: - result.add(ctx.newEnvVarAccess(tmp)) + result.add(ctx.newTempVarAccess(tmp)) of nkCaseStmt: var ns = false @@ -670,10 +627,7 @@ proc lowerStmtListExprs(ctx: var Ctx, n: PNode, needsSplit: var bool): PNode = else: internalError(ctx.g.config, "lowerStmtListExpr(nkCaseStmt): " & $branch.kind) result.add(n) - if ctx.nimOptItersEnabled: - result.add(ctx.newTempVarAccess(tmp)) - else: - result.add(ctx.newEnvVarAccess(tmp)) + result.add(ctx.newTempVarAccess(tmp)) elif n[0].kind == nkStmtListExpr: result = newNodeI(nkStmtList, n.info) let (st, ex) = exprToStmtList(n[0]) @@ -706,11 +660,7 @@ proc lowerStmtListExprs(ctx: var Ctx, n: PNode, needsSplit: var bool): PNode = let tmp = ctx.newTempVar(cond.typ, result, cond) # result.add(ctx.newTempVarAsgn(tmp, cond)) - var check: PNode - if ctx.nimOptItersEnabled: - check = ctx.newTempVarAccess(tmp) - else: - check = ctx.newEnvVarAccess(tmp) + var check = ctx.newTempVarAccess(tmp) if n[0].sym.magic == mOr: check = ctx.g.newNotCall(check) @@ -720,18 +670,12 @@ proc lowerStmtListExprs(ctx: var Ctx, n: PNode, needsSplit: var bool): PNode = let (st, ex) = exprToStmtList(cond) ifBody.add(st) cond = ex - if ctx.nimOptItersEnabled: - ifBody.add(ctx.newTempVarAsgn(tmp, cond)) - else: - ifBody.add(ctx.newEnvVarAsgn(tmp, cond)) + ifBody.add(ctx.newTempVarAsgn(tmp, cond)) let ifBranch = newTree(nkElifBranch, check, ifBody) let ifNode = newTree(nkIfStmt, ifBranch) result.add(ifNode) - if ctx.nimOptItersEnabled: - result.add(ctx.newTempVarAccess(tmp)) - else: - result.add(ctx.newEnvVarAccess(tmp)) + result.add(ctx.newTempVarAccess(tmp)) else: for i in 0..