Bug 22916 – [dip1000] copy of ref return still treated as scope variable

Status
RESOLVED
Resolution
FIXED
Severity
normal
Priority
P3
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2022-03-24T11:10:00Z
Last change time
2023-02-20T10:55:44Z
Keywords
pull, safe
Assigned to
No Owner
Creator
Dennis
See also
https://issues.dlang.org/show_bug.cgi?id=23682

Comments

Comment #0 by dkorpel — 2022-03-24T11:10:00Z
This came up in a custom dynamic array type of mine. ``` // compile with -preview=dip1000 @safe: struct Arr { int** ptr; ref int* index() return scope { return *ptr; } void assign(int* p) scope { *ptr = p; } } void main() { scope Arr a; a.assign(a.index()); } ``` `index` is `return scope` since it returns an element of a scope array by ref. However, when it's passed to the `assign` function, a copy of the `ref` return is passed, implicitly dereferencing it so it shouldn't be scope anymore. The compiler still files this error though: > Error: scope variable `a` assigned to non-scope parameter `p` calling onlineapp.Arr.assign
Comment #1 by bugzilla — 2022-08-15T15:08:14Z
Here's the problem: ref int* index() return scope { return *ptr; } The `scope` applies to the `this` reference, in this case, `ptr`. Although the *ptr is not `scope`, because `scope` is not transitive, the compiler transfers the `scope` to the return value because it is told to. (This is a feature, not a bug.)
Comment #2 by bugzilla — 2022-08-15T15:17:46Z
I think the basic problem here is the code is trying to make scope transitive, and it just doesn't work. At some point the code will have to use @trusted code to make it work.
Comment #3 by dkorpel — 2022-08-15T15:25:43Z
(In reply to Walter Bright from comment #1) > Although > the *ptr is not `scope`, because `scope` is not transitive, the compiler > transfers the `scope` to the return value because it is told to. No, my `return scope` annotation is correct. *ptr becomes `scope` again because I return it by `ref`, which is effectively taking the address which cancels out the dereference. If you mark `index` just `scope`, the compiler correctly complains about `return *ptr`: > Error: scope variable `this` may not be returned
Comment #4 by dkorpel — 2022-08-15T15:30:05Z
(In reply to Walter Bright from comment #2) > I think the basic problem here is the code is trying to make scope > transitive, and it just doesn't work. No, my array struct works exactly like a dynamic array. It does not try to have scope pointers as array elements.
Comment #5 by bugzilla — 2022-08-26T18:07:51Z
Looking at: ref int* index() return scope { return *ptr; } the compiler sees it as: ref return scope T index(); which is interpreted as `ref` and `return scope` attached to `this`. `a` is `this`. Since `a` is `scope`, the return value of `a.index()` is also `scope`. This `scope` return value is then passed to `assign(int*)`, where the `int*` parameter is not `scope`. Assigning `scope` to `not scope` is an error and is correctly diagnosed by the compiler.
Comment #6 by dkorpel — 2022-08-26T20:27:47Z
(In reply to Walter Bright from comment #5) > which is interpreted as `ref` and `return scope` attached to `this`. `a` is > `this`. Since `a` is `scope`, the return value of `a.index()` is also > `scope`. And that's what the compiler does wrong. The `return scope` applies to the `ref` return, not the returned `int*` value. This can be showcased by this example: ``` ref int f(ref return scope int* x) @safe { return *x; } ``` Notice how the return value has no pointers, and yet the compiler will raise an error if you don't annotate if `return scope`. dmd will also infer this as `return scope` itself if you make it a template. The job of `return scope` here is to prevent returning `f(stackPointer)` by ref or escaping `&f(stackPointer)`, but this works: ``` int g() @safe { int x; int y = f(&x); // `y` is not `scope`, how could it be? return y; // 'escaping' the result of f() } ``` The only thing my original example does differently is giving the pointer payload a different type (`int*` instead of `int`), which shouldn't affect this example since it's the second layer of indirection, it shouldn't be affected by `scope` or `return scope`. By the way, I reduced the original example to `int**` because you usually request that, but it might be actually illuminating to see what the original code looked like: ``` void scoot(scope Array!string a) @safe { a[0] = a[1]; } ``` This obviously doesn't violate `scope` and it works with `string[]`, so it should also work with library array types, or it's poor design of dip1000.
Comment #7 by bugzilla — 2022-08-29T03:03:24Z
I should have looked at the cheat sheet, which has: ref X foo(ref return scope P p) ReturnRef-Scope which concurs with what you wrote.
Comment #8 by bugzilla — 2022-08-29T03:36:13Z
(The cheat sheet was out of date. My bad. Remember when we changed `return scope` to always mean ReturnScope, and `ref scope return` never means `Ref-ReturnScope`?) Let's change x to p for clarity: ref int f(ref return scope int* p) @safe { return *p; } This compiles as `ref` `return scope`. This protects the value of p. Not the address of p. This compiles successfully, as the value of `p` is what is returned. Now let's try not having `return scope` next to each other: ref int f(ref5 scope return int* p) @safe { return *p; } This compiles as `return ref` `scope`. It fails to compile with: Error: scope variable `p` may not be returned. Both are correct behavior. Note that `return scope` is *always* interpreted as returnScope. Also, the behavior is *always* determined by the function signature, not whatever the return expression is.
Comment #9 by bugzilla — 2022-08-29T03:53:11Z
> The only thing my original example does differently is giving the pointer payload a different type (`int*` instead of `int`), which shouldn't affect this example since it's the second layer of indirection, it shouldn't be affected by `scope` or `return scope`. That difference makes all the difference. The trouble is the code is trying to store a scope protected value `int* p` into the unprotected payload pointer `*ptr`. There's no way dip1000 can do that. You'll have to use @trusted code.
Comment #10 by dkorpel — 2022-08-29T08:36:05Z
(In reply to Walter Bright from comment #8) > Note that `return scope` is *always* interpreted as returnScope. > > Also, the behavior is *always* determined by the function signature, not > whatever the return expression is. This issue is not about whether my `Arr.index` function is ReturnRef-Scope or Ref-ReturnScope, it's always been clear from the start it's Ref-ReturnScope.
Comment #11 by dkorpel — 2022-08-29T09:39:35Z
(In reply to Walter Bright from comment #9) > That difference makes all the difference. No it does not. Whether I have a `scope int[]` or `scope string[]`, the scope only applies to the first indirection of the array, not the array elements. If I index a `scope string[]` and get a `scope string`, it's a bug. > The trouble is the code is trying to store a scope protected value `int* p` > into the unprotected payload pointer `*ptr`. There's no way dip1000 can do that. Of course not, but the issue is that it shouldn't be a scope protected value in the first place because it came from a dereferenced pointer, after which `scope` shouldn't apply anymore. This is deduced from the function signature using `return scope` by the way, NOT from the return expression. Please stop closing this issue based on false premises. I'm not incorrectly annotating the function `return scope`. I'm not trying to cheat transitive scope. I'm not asking to inspect the return expression to gain more information than the function signature provides. I'm not saying a scope pointer may be assigned to a non-scope parameter. > You'll have to use @trusted code. The whole point of dip1000 is that stack-allocated arrays become usable in @safe code. If we can't allow indexing or assigning array elements without @trusted, it defeats the purpose.
Comment #12 by dkorpel — 2022-10-10T11:12:30Z
I've found a workaround for this issue by the way: convert the `ref` return to a pointer and dereference that to get the non-scope value. ``` // compile with -preview=dip1000 @safe: struct Arr { int** ptr; ref int* index() return scope { return *ptr; } void assign(int* p) scope { *ptr = p; } } void main() { scope Arr a; a.assign(*&a.index()); auto tmp = &a.index(); a.assign(*tmp); } ```
Comment #13 by dkorpel — 2023-02-09T10:43:55Z
*** Issue 23682 has been marked as a duplicate of this issue. ***
Comment #14 by dlang-bot — 2023-02-11T12:25:27Z
@dkorpel created dlang/dmd pull request #14871 "Fix 22916 - copy of ref return still treated as scope variable" fixing this issue: - Fix 22916 - copy of ref return still treated as scope variable https://github.com/dlang/dmd/pull/14871
Comment #15 by dlang-bot — 2023-02-20T10:55:44Z
dlang/dmd pull request #14871 "Fix 22916 - copy of ref return still treated as scope variable" was merged into master: - 21375505508e2cc03395988de9702305c030378f by Dennis Korpel: Fix 22916 - copy of ref return still treated as scope variable https://github.com/dlang/dmd/pull/14871