Commit Graph

263 Commits

Author SHA1 Message Date
ringabout
2de409cd0c fixes #24806; don't elide wasMoved when syms are used in blocks (#24831)
fixes #24806

Blocks don't merge symbols that are used before destruction to the
parent scope, which causes `wasMoved; destroy` to elide incorrectly

(cherry picked from commit 73aeac81d1)
2025-04-04 10:08:30 +02:00
ringabout
8f563f2cc9 fixes #24754; {.gcsafe.} block breaks move analysis (#24757)
fixes #24754

(cherry picked from commit a7711d452d)
2025-03-13 12:27:15 +01:00
narimiran
58f1e22db3 Revert "sink tuples by values (#24731)"
This reverts commit b9d3348dab.
2025-03-03 20:32:56 +01:00
ringabout
b9d3348dab sink tuples by values (#24731)
A reduced case
```nim
type AnObject = tuple
  a: string
  b: int
  c: int

proc mutate(a: sink AnObject) =
  `=wasMoved`(a)
  echo 1

# echo "Value is: ", obj.value
proc bar =
  mutate(("1.2", 0, 0))

bar()
```

(cherry picked from commit 7e8a650729)
2025-03-03 14:11:23 +01:00
ringabout
130e7182c4 implements quirky for functions (#24700)
ref https://github.com/nim-lang/Nim/pull/24686

With this PR

```nim
import std/streams

proc foo() =
  var name = newStringStream("2r2")
  raise newException(ValueError, "sh")

try:
  foo()
except:
 discard

echo 123
```
this example no longer leaks

---------

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
(cherry picked from commit 510ac84518)
2025-03-03 14:02:48 +01:00
metagn
12347eae74 generate destructor in nodestroy proc for explicit destructor call (#24627)
fixes #24626

`createTypeboundOps` in sempass2 is called when generating destructors
for types including for explicit destructor calls, however it blocks
destructors from getting generated in a `nodestroy` proc. This causes
issues when a destructor is explicitly called in a `nodestroy` proc. To
fix this, allow destructors to get generated only for explicit
destructor calls in nodestroy procs.

(cherry picked from commit 793baf34ff)
2025-01-20 18:46:41 +01:00
Esteban C Borsani
ede6540c55 fixes #23212; Asyncdispatch leaks under --mm:arc (#24556)
Fixes #23212

Inspired by [this chronos
PR](https://github.com/status-im/nim-chronos/pull/243)

(cherry picked from commit f29234b40f)
2025-01-15 10:19:26 +01:00
ringabout
5a71c36d25 fixes strictdefs warnings continue (#24520)
(cherry picked from commit d2d810585c)
2025-01-14 13:23:18 +01:00
ringabout
428b64251f adds a test case (#24532)
closes #18070

(cherry picked from commit f796c01e3c)
2025-01-14 13:20:04 +01:00
metagn
9c87f2cb4b always reinstantiate nominal values of generic instantiations (#24425)
fixes #22479, fixes #24374, depends on #24429 and #24430

When instantiating generic types which directly have nominal types
(object, distinct, ref/ptr object but not enums[^1]) as their values,
the nominal type is now copied (in the case of ref objects, its child as
well) so that it receives a fresh ID and `typeInst` field. Previously
this only happened if it contained any generic types in its structure,
as is the case for all other types.

This solves #22479 and #24374 by virtue of the IDs being unique, which
is what destructors check for. Technically types containing generic
param fields work for the same reason. There is also the benefit that
the `typeInst` field is correct. However issues like #22445 aren't
solved because the compiler still uses structural object equality checks
for inheritance etc. which could be removed in a later PR.

Also fixes a pre-existing issue where destructors bound to object types
with generic fields would not error when attempting to define a user
destructor after the fact, but the error message doesn't show where the
implicit destructor was created now since it was only created for
another instance. To do this, a type flag is used that marks the generic
type symbol when a generic instance has a destructor created. Reusing
`tfCheckedForDestructor` for this doesn't work.

Maybe there is a nicer design that isn't an overreliance on the ID
mechanism, but the shortcomings of `tyGenericInst` are too ingrained in
the compiler to use for this. I thought about maybe adding something
like `tyNominalGenericInst`, but it's really much easier if the nominal
type itself directly contains the information of its generic parameters,
or at least its "symbol", which the design is heading towards.

[^1]: See [this
test](21420d8b09/lib/std/enumutils.nim (L102))
in enumutils. The field symbols `b0`/`b1` always have the uninstantiated
type `B` because enum fields don't expect to be generic, so no generic
instance of `B` matches its own symbols. Wouldn't expect anyone to use
generic enums but maybe someone does.

(cherry picked from commit 05c74d6844)
2025-01-14 09:07:03 +01:00
ringabout
2d658e8de5 fixes #24402; Memory leak under Arc/Orc on inline iterators with nested seq (#24419)
fixes #24402

```nim
iterator myPairsInline*[T](twoDarray: seq[seq[T]]): (int, seq[T]) {.inline.} =
  for indexValuePair in twoDarray.pairs:
    yield indexValuePair

proc innerTestTotalMem() =
  var my2dArray: seq[seq[int32]] = @[]

  # fill with some data...
  for i in 0'i32..100:
    var z = @[i, i+1]
    my2dArray.add z

  for oneDindex, innerArray in myPairsInline(my2dArray):
    discard

innerTestTotalMem()
```

In `for oneDindex, innerArray in myPairsInline(my2dArray)`, `oneDindex`
and `innerArray` becomes `cursors` because they satisfy the criterion of
`isSimpleIteratorVar`. On the one hand, it is not correct to have them
point to the temporary generated by tuple unpacking, which left the
memory of the temporary uncleaned up. On the other hand, we don't need
to generate a temporary for a symbol node when unpacking the tuple.

(cherry picked from commit 21420d8b09)
2025-01-14 09:05:36 +01:00
ringabout
4bdeddcac5 deprecate NewFinalize with the ref T finalizer (#24354)
pre-existing issues:

```nim
block:
  type
    FooObj = object
      data: int
    Foo = ref ref FooObj

  proc delete(self: Foo) =
    echo self.data

  var s: Foo
  new(s, delete)
```
it crashed with arc/orc in 1.6.x and 2.x.x

```nim
block:
  type
    Foo = ref int

  proc delete(self: Foo) =
    echo self[]

  var s: Foo
  new(s, delete)
```

The simple fix is to add a type restriction for the type `T` for arc/orc
versions
```nim
  proc new*[T: object](a: var ref T, finalizer: proc (x: T) {.nimcall.})
```

(cherry picked from commit 2af602a5c8)
2025-01-14 07:50:33 +01:00
ringabout
5c843d3d60 fixes #24147; Copy hook causes an incompatible-pointer-types (#24149)
fixes #24147
2024-09-22 13:51:51 +02:00
ringabout
755307be61 fixes #24141; Calling algorithm reverse causes a SIGSEGV on ORC (#24142)
fixes #24141
2024-09-19 15:17:25 +02:00
metagn
cd22560af5 fix string literal assignment with different lengths on ARC (#24083)
fixes #24080
2024-09-08 20:17:26 +02:00
ringabout
43274bfb92 fixes #23982; codegen regression passing pointer expressions to inline iterators (#23986)
fixes #23982
2024-08-20 19:09:06 +08:00
ringabout
26107e931c fixes #23973; fixes #23974; Memory corruption with lent and ORC (#23981)
fixes #23973;
fixes #23974
2024-08-20 11:57:47 +02:00
ringabout
1d59e1cbb6 fixes #23907; Double destroy using proc type alias with a sink (#23909)
fixes #23907
2024-08-11 10:12:48 +02:00
ringabout
3a103669d1 fixes #23858; 2.2.0 rc1 regression with cdecl functions (#23859)
fixes #23858

We should not assign fields to fields for returns of function calls
because calls might have side effects.
2024-07-18 20:53:07 +02:00
ringabout
e53a2ed19b fixes #20865; fixes #20987; Missing bounds check in array slicing (#23814)
fixes #20865
fixes #20987
2024-07-10 17:25:34 +02:00
ringabout
262ff648aa [backport] fixes #23690; SIGSEGV with object variants and RTTI (#23703)
fixes #23690

```nim
dest.`:state` = src.`:state`
var :tmp_553651276 = dest.e1.a
`=wasMoved`(dest.e1.a)
dest.e1.a.kind = src.e1.a.kind
case dest.e1.a.kind
of 0:
  dest.e1.a.a = src.e1.a.a
of 1:
  `=copy`(dest.e1.a.c, src.e1.a.c)
case :tmp_553651276.kind
of 0:
of 1:
  `=destroy`(:tmp_553651276.c)
```
`dest.e1.a.kind = src.e1.a.kind` changes the discrimant but it fails to
clear the memory of `dest.e1.a`. Before using hooks for copying, we need
to clear the dest, e.g. `=wasMoved(dest.e1.a.c)`.

```nim
dest.`:state` = src.`:state`
var :tmp_553651276 = dest.e1.a
`=wasMoved`(dest.e1.a)
dest.e1.a.kind = src.e1.a.kind
case dest.e1.a.kind
of 0:
  `=wasMoved`(dest.e1.a.a)
  dest.e1.a.a = src.e1.a.a
  `=wasMoved`(dest.e1.a.b)
of 1:
  `=wasMoved`(dest.e1.a.c)
  `=copy`(dest.e1.a.c, src.e1.a.c)
case :tmp_553651276.kind
of 0:
of 1:
  `=destroy`(:tmp_553651276.c)
```
2024-06-11 05:55:08 +02:00
ringabout
309f97af4c fixes #23627; Simple destructor code gives invalid C (#23631)
fixes #23627

```nim
type
  TestObj = object of RootObj

  TestTestObj = object of RootObj
    testo: TestObj

proc `=destroy`(x: TestTestObj) =
  echo "Destructor for TestTestObj"

proc testCaseT() =
  echo "\nTest Case T"
  let tt1 {.used.} = TestTestObj(testo: TestObj())
```

When generating const object fields, it's likely that
we need to generate type infos for the object, which may be an object
with
custom hooks. We need to generate potential consts in the hooks first.

https://github.com/nim-lang/Nim/pull/20433 changed the semantics of
initialization. It should evaluate`BracedInit` first.
2024-05-21 14:53:08 +02:00
ringabout
2c8551556e fixes lifting subtype calling parent's hooks (#23612)
ref https://forum.nim-lang.org/t/11587

Tested with `gcc version 14.0.1 20240412` locally
2024-05-15 20:52:18 +02:00
ringabout
cd3cf3a20e fixes #23524; global variables cannot be analysed when injecting move (#23529)
fixes #23524

```nim
proc isAnalysableFieldAccess*(orig: PNode; owner: PSym): bool =
  ...
  result = n.kind == nkSym and n.sym.owner == owner and
    {sfGlobal, sfThread, sfCursor} * n.sym.flags == {} and
    (n.sym.kind != skParam or isSinkParam(n.sym))
```
In `isAnalysableFieldAccess`, globals, cursors are already rejected
2024-04-24 12:47:05 +02:00
ringabout
acd4c8a353 fixes #23505; fixes injectdestructors errors on transformed addr (deref) refs (#23507)
fixes #23505
2024-04-18 17:57:44 +08:00
ringabout
7d9721007c fixes regression #22909; don't optimize result init if statements can raise which corrupts the compiler (#23271)
fixes #22909
required by https://github.com/nim-lang/Nim/pull/23267

```nim
proc foo: string =
  assert false
  result = ""
```

In the function `foo`, `assert false` raises an exception, which can
cause `result` to be uninitialized if the default result initialization
is optimized out
2024-02-01 16:51:07 +01:00
ringabout
24a606902a fixes #23247; don't destroy openarray since it doesn't own the data (#23254)
fixes #23247
closes #23251 (which accounts for why the openarray type is lifted
because ops are lifted for openarray conversions)

related: https://github.com/nim-lang/Nim/pull/18713

It seems to me that openarray doesn't own the data, so it cannot destroy
itself. The same case should be applied to
https://github.com/nim-lang/Nim/issues/19435. It shouldn't be destroyed
even openarray can have a destructor. A cleanup will be followed for
https://github.com/nim-lang/Nim/pull/19723 if it makes sense.

According to https://github.com/nim-lang/Nim/pull/12073, it lifts
destructor for openarray when openarray is sunk into the function, when
means `sink openarray` owns the data and needs to destroy it. In other
cases, destructor shouldn't be lifted for `openarray` in the first place
and it shouldn't destroy the data if it doesn't own it.

---------

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
2024-01-26 08:04:16 +01:00
ringabout
b2f79df81c fixes #22218; avoids cursor when copy is disabled (#23209)
fixes #22218
2024-01-18 21:47:13 +01:00
ringabout
29ac3c9986 fixes #22923; fixes =dup issues (#23182)
fixes #22923
2024-01-11 11:23:42 +01:00
Andreas Rumpf
0d24f76546 fixes #22552 (#23014) 2023-12-02 05:28:24 +01:00
ringabout
379299a5ac fixes #22286; enforce Non-var T destructors by nimPreviewNonVarDestructor (#22975)
fixes #22286
ref https://forum.nim-lang.org/t/10642

For backwards compatibilities, we might need to keep the changes under a
preview compiler flag. Let's see how many packags it break.

**TODO** in the following PRs

- [ ] Turn the `var T` destructors warning into an error with
`nimPreviewNonVarDestructor`

---------

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
2023-11-25 18:27:27 +01:00
ringabout
b68e0aab4c fixes #22866; fixes #19998; ensure destruction for Object construction with custom destructors (#22901)
fixes #22866;
fixes #19998
2023-11-02 11:14:50 +01:00
ringabout
f5d70e7fa7 fixes #19250; fixes #22259; ORC AssertionDefect not containsManagedMemory(n.typ) (#22823)
fixes #19250
fixes #22259

The strings, seqs, refs types all have this flag, why should closures be
treated differently?

follow up https://github.com/nim-lang/Nim/pull/14336
2023-10-13 21:34:13 +02:00
ringabout
efa64aa49b fixes #22787; marks var section in the loop as reassign preventing cursor (#22800)
fixes #22787

---------

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
2023-10-07 07:43:39 +02:00
ringabout
d82bc0a29f items, pairs and friends now use unCheckedInc (#22729)
`{.push overflowChecks: off.}` works in backends. Though it could be
implemented as a magic function.

By inspecting the generated C code, the overflow check is eliminated in
the debug or release mode.


![image](https://github.com/nim-lang/Nim/assets/43030857/49c3dbf4-675e-414a-b972-b91cf218c9f8)

Likewise, the index checking is probably not needed.
2023-09-20 12:50:23 +02:00
ringabout
5f13e15e0a fixes #22664; guard against potential seqs self assignments (#22671)
fixes #22664
2023-09-08 17:05:57 +02:00
metagn
53d43e9671 round out tuple unpacking assignment, support underscores (#22537)
* round out tuple unpacking assignment, support underscores

fixes #18710

* fix test messages

* use discard instead of continue

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>

---------

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
2023-08-24 06:11:48 +02:00
ringabout
ee817557ec close #22748; cursorinference + -d:nimNoLentIterators results in err… (#22495)
closed #22748; cursorinference + -d:nimNoLentIterators results in erroneous recursion
2023-08-17 13:33:19 +02:00
ringabout
ecc6ab7ee0 fixes #22237; fixes #21160; wrong cursor on unowned parameters in the for loop in ORC (#22240)
fixes #22237; fixes #21160; wrong cursor on unowned parameters
2023-07-10 10:31:13 +02:00
Andreas Rumpf
a15db5d60b fixes #22175 (#22229) 2023-07-06 15:15:50 +02:00
ringabout
145e002c74 fixes #22132; hoisted openArray params result in erroneous code (#22224) 2023-07-05 11:21:57 +02:00
Jacek Sieka
4d2ebbb877 fix controlflow test (#22194)
the function actually returns
2023-06-30 14:54:46 +02:00
Andreas Rumpf
427ad17161 fixes #22001 (#22177)
* fixes #22001

* added test case
2023-06-27 22:42:48 +02:00
ringabout
e422b3c860 adds =destroy T support for strings and seqs (#22167)
* adds =destroy T support for strings and seqs

* fixes system

* fixes tests
2023-06-27 13:07:29 +02:00
ringabout
2e12d3e26b fixes #22058; invalid free with {.noSideEffect.} in template (#22088) 2023-06-13 12:03:20 +02:00
ringabout
929cb4d601 fixes #21987; don't create type bound ops for anything in a function with a nodestroy pragma (#21992)
* fixes #21987; don't create type bound ops for anything in a function with a `nodestroy` pragma

* add a comment
2023-06-04 08:37:58 +02:00
ringabout
1133f20fe2 lift the =dup hook (#21903)
* fixes tests again
* remove helper functions
* fixes closures, owned refs
* final cleanup
2023-06-02 16:03:32 +02:00
ringabout
8e35b3d577 fixes #21974; fixes sameConstant fieldDefect (#21981)
* fixes #21974; fixes sameConstant fieldDefect

* add a test case
2023-06-01 19:02:56 +02:00
ringabout
ce1ba91573 close #19990; adds a test case (#21853) 2023-05-15 21:17:06 +02:00
ringabout
3b9999b93c adds documentation for =wasMoved and =dup hooks and small fixes (#21827)
* adds documentation for `=wasMoved` and `=dup` hooks and small fixes

* Update doc/destructors.md

* Update doc/destructors.md

---------

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
2023-05-11 19:38:27 +08:00