From 2194d9ea7a8f79a81c57c55bf1d31f5771037384 Mon Sep 17 00:00:00 2001 From: zerbina <100542850+zerbina@users.noreply.github.com> Date: Tue, 10 Sep 2024 23:27:01 +0000 Subject: [PATCH 1/3] typesallowed: move view queries to `types` Querying whether and what kind of view a type represents has nothing to do with the "type allowed" checks. --- compiler/ast/types.nim | 71 +++++++++++++++++++++++++++++++++ compiler/sem/typeallowed.nim | 72 ---------------------------------- compiler/sem/varpartitions.nim | 3 -- 3 files changed, 71 insertions(+), 75 deletions(-) diff --git a/compiler/ast/types.nim b/compiler/ast/types.nim index 9bb6c072301..0c174e9887d 100644 --- a/compiler/ast/types.nim +++ b/compiler/ast/types.nim @@ -60,6 +60,9 @@ type bvcSingle ## single-location view bvcSequence ## view of contiguous locations + ViewTypeKind* = enum + noView, immutableView, mutableView + proc base*(t: PType): PType = result = t[0] @@ -1558,6 +1561,74 @@ proc classifyBackendView*(t: PType): BackendViewKind = tyAnd, tyOr, tyNot, tyAnything, tyFromExpr: unreachable() +proc combine(dest: var ViewTypeKind, b: ViewTypeKind) {.inline.} = + case dest + of noView, mutableView: + dest = b + of immutableView: + if b == mutableView: dest = b + +proc classifyViewTypeAux(marker: var IntSet, t: PType): ViewTypeKind + +proc classifyViewTypeNode(marker: var IntSet, n: PNode): ViewTypeKind = + case n.kind + of nkSym: + result = classifyViewTypeAux(marker, n.typ) + of nkOfBranch: + result = classifyViewTypeNode(marker, n.lastSon) + else: + result = noView + for child in n: + result.combine classifyViewTypeNode(marker, child) + if result == mutableView: break + +proc classifyViewTypeAux(marker: var IntSet, t: PType): ViewTypeKind = + if containsOrIncl(marker, t.id): return noView + case t.kind + of tyVar: + result = mutableView + of tyLent, tyOpenArray, tyVarargs: + result = immutableView + of tyGenericInst, tyDistinct, tyAlias, tyInferred, tySink, + tyUncheckedArray, tySequence, tyArray, tyRef, tyStatic: + result = classifyViewTypeAux(marker, lastSon(t)) + of tyFromExpr: + if t.len > 0: + result = classifyViewTypeAux(marker, lastSon(t)) + else: + result = noView + of tyTuple: + result = noView + for i in 0.. 0: - result = classifyViewTypeAux(marker, lastSon(t)) - else: - result = noView - of tyTuple: - result = noView - for i in 0.. Date: Wed, 11 Sep 2024 00:35:04 +0000 Subject: [PATCH 2/3] consider non-direct views in return type positions Both `mirgen` and `isPassByRef` didn't consider non-direct views when deciding whether the parameter uses pass-by-reference. Now they do. --- compiler/ast/types.nim | 7 ++++--- compiler/mir/mirgen.nim | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/compiler/ast/types.nim b/compiler/ast/types.nim index 0c174e9887d..ed0068390f1 100644 --- a/compiler/ast/types.nim +++ b/compiler/ast/types.nim @@ -1652,8 +1652,9 @@ proc isPassByRef*(conf: ConfigRef; s: PSym, retType: PType): bool = else: result = false - # first parameter and return type is 'lent T'? --> use pass by pointer if - # not already a pointer-like type - if s.position == 0 and retType != nil and retType.kind == tyLent: + # first parameter and return type is immutable view? --> use pass by pointer + # if not already a pointer-like type + if s.position == 0 and retType != nil and + classifyViewType(retType) == immutableView: result = pt.kind notin {tyVar, tyOpenArray, tyVarargs, tyRef, tyPtr, tyPointer} diff --git a/compiler/mir/mirgen.nim b/compiler/mir/mirgen.nim index 8bdff9a0fc2..a071314d883 100644 --- a/compiler/mir/mirgen.nim +++ b/compiler/mir/mirgen.nim @@ -766,7 +766,7 @@ proc genArgs(c: var TCtx, n: PNode) = c.add empty(c, n[i]) elif i == 1 and not fntyp[0].isEmptyType() and not isHandleLike(t) and - classifyBackendView(fntyp[0]) != bvcNone: + classifyViewType(fntyp[0]) == immutableView: # the procedure returns a view, but the first parameter is not something # that resembles a handle. We need to make sure that the first argument # (which the view could be created from), is passed by reference From 7bf13691b23a3610c7c70c70ac73b4d2817a9d05 Mon Sep 17 00:00:00 2001 From: zerbina <100542850+zerbina@users.noreply.github.com> Date: Wed, 11 Sep 2024 00:35:05 +0000 Subject: [PATCH 3/3] tests: add a test --- ...timmutable_borrow_from_first_parameter.nim | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 tests/lang_experimental/views/timmutable_borrow_from_first_parameter.nim diff --git a/tests/lang_experimental/views/timmutable_borrow_from_first_parameter.nim b/tests/lang_experimental/views/timmutable_borrow_from_first_parameter.nim new file mode 100644 index 00000000000..a9c1e72f3b6 --- /dev/null +++ b/tests/lang_experimental/views/timmutable_borrow_from_first_parameter.nim @@ -0,0 +1,27 @@ +discard """ + description: ''' + Ensure a view borrowing from the first parameter can be safely returned + from a procedure. + ''' + targets: c js vm + knownIssue.js vm: "The first parameter isn't passed by reference" +""" + +block direct_lent_view_primitive: + # test borrowing from primitive-type parameter with a direct view + proc test(x: int): lent int = + x + + var x = 0 + doAssert addr(test(x)) == addr(x) + +block object_lent_view_primitive: + # test borrowing from primitive-type parameter with an indirect view + type Object = object + x: lent int + + proc test(x: int): Object = + Object(x: x) + + var x = 0 + doAssert addr(test(x).x) == addr(x)