Bug 11929 – Disallow `ref` in front tuple expansion in foreach over range

Status
NEW
Severity
normal
Priority
P3
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2014-01-15T00:46:34Z
Last change time
2024-12-13T18:16:03Z
Keywords
accepts-invalid
Assigned to
No Owner
Creator
Denis Shelomovskii
Moved to GitHub: dmd#18758 →

Comments

Comment #0 by verylonglogin.reg — 2014-01-15T00:46:34Z
As currently `ref` is allowed in front tuple expansion such dangerous code is accepted and fails only in runtime: --- import std.range; void main() { int[] arr = new int[1]; foreach(ref a, b; zip(arr, new int[1])) ++a; assert(arr[0] == 1); } --- IMO such code should reject to compile as I see no general use for access to tuple element by `ref`. Broken after such change code most likely is invalid. Also front tuple expansion in foreach over range isn't documented. See Issue 7361.
Comment #1 by maxim — 2014-01-15T01:39:33Z
Are you arguing against ref parameters in for/foreach loop in general? I see no reason for it because the root of the problem is in putting ref (and there is no reason for it), so it is PEBKAC.
Comment #2 by verylonglogin.reg — 2014-01-15T06:24:11Z
(In reply to comment #1) > Are you arguing against ref parameters in for/foreach loop in general? I see > no reason for it because the root of the problem is in putting ref (and there > is no reason for it), so it is PEBKAC. Thanks for the sarcasm. Probably I should add more description: Ranges of tuples generally contains generated somehow based on source ranges tuples (e.g. `zip` and `group`) so access to tuple elements by `ref` is a nonsense. As D type system doesn't provide head const I see no way to mark tuple elements non-`ref`able so I suggest to disallow such access in general as I'm not aware of cases where such functionality is needed. And even if there are such cases IHO its support doesn't worth potential confusion.
Comment #3 by maxim — 2014-01-15T07:25:04Z
(In reply to comment #2) > > Ranges of tuples generally contains generated somehow based on source ranges > tuples (e.g. `zip` and `group`) so access to tuple elements by `ref` is a > nonsense. As D type system doesn't provide head const I see no way to mark > tuple elements non-`ref`able so I suggest to disallow such access in general as > I'm not aware of cases where such functionality is needed. And even if there > are such cases IHO its support doesn't worth potential confusion. I still don't understand the scope of proposed limitation. Foreach over ranges or foreach in general? By the way, what you are asking can be reformulated as: void foo(ref int i) { i = 0; } void main() { int i = 1; i.foo(); //oops, i = 0, let's ban ref attribute in function parameters } Why one should put 'ref' (supposedly unconsciously?) and then complain about it?
Comment #4 by verylonglogin.reg — 2014-01-15T23:57:32Z
(In reply to comment #3) > (In reply to comment #2) > > > > Ranges of tuples generally contains generated somehow based on source ranges > > tuples (e.g. `zip` and `group`) so access to tuple elements by `ref` is a > > nonsense. As D type system doesn't provide head const I see no way to mark > > tuple elements non-`ref`able so I suggest to disallow such access in general as > > I'm not aware of cases where such functionality is needed. And even if there > > are such cases IHO its support doesn't worth potential confusion. > > I still don't understand the scope of proposed limitation. Foreach over ranges > or foreach in general? Neither. For tuple expansion in `foreach` over range only as written in description. > By the way, what you are asking can be reformulated as: > > void foo(ref int i) { i = 0; } > > void main() > { > int i = 1; > i.foo(); //oops, i = 0, let's ban ref attribute in function parameters > } > > Why one should put 'ref' (supposedly unconsciously?) and then complain about > it? Your example is about a requirement to explicitly specify `ref` in function call expression like in C#, it's another already discussed issue. Function calls has nothing to do with this bug report. In this issue it's proposed to disable `ref` in front tuple expansion case because in general created tuple is a temporary. Also I noted the issue exists in `foreach` over ranges in general, not just in tuple expansion case so I filed Issue 11934 for range problem and Issue 11935 that is a better solution for the problem described in this one.
Comment #5 by robert.schadek — 2024-12-13T18:16:03Z
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/18758 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB