www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Is remove safe using foreach

reply lili <lilijreey 126.com> writes:
```
int[string] aa = ["ok":1, "aaa":2, "ccc":3, "ddd":4];
	foreach (k ; aa.byKey)
	{
		if (k == "aaa") {
		aa.remove(k);
		aa["ww"] = 33;
		}

		if (k == "ww") {
		aa.remove(k);
		aa["vv"] = 33;
		}
	}

writeln(aa); // output ["ok":1, "ddd":4, "vv":33, "ccc":3] is ok
```
Dec 12 2022
parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 12/12/22 12:23 PM, lili wrote:

 ```
 int[string] aa = ["ok":1, "aaa":2, "ccc":3, "ddd":4];
      foreach (k ; aa.byKey)
      {
          if (k == "aaa") {
          aa.remove(k);
          aa["ww"] = 33;
          }
 
          if (k == "ww") {
          aa.remove(k);
          aa["vv"] = 33;
          }
      }
 
 writeln(aa); // output ["ok":1, "ddd":4, "vv":33, "ccc":3] is ok
 ```
 


Removing keys while iterating is not supported. It will break, in 
confusing ways, and possibly include a null pointer dereference.

Instead, either iterate over `aa.keys`, which makes a copy of the keys 
into an array, store the keys to remove for later processing, or 
construct a new AA with the indicated keys removed.

-Steve
Dec 12 2022
next sibling parent reply lili <lilijreey 126.com> writes:
is foreach Syntactic sugar?, like for-range in C++, if it is, 
compiler how implement
Dec 12 2022
parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 12/12/22 7:54 PM, lili wrote:

 is foreach Syntactic sugar?, like for-range in C++, if it is, compiler 
 how implement
 
 


Yes it is syntax sugar. The lowering depends on what the item you're 
iterating is.

For an associative array `byKey`, it is converting the AA into a range 
of keys, and for a range, the compiler does:

foreach(k; aa.byKey)

=>

for(auto r = aa.byKey, auto k = r.front; !r.empty; r.popFront)

(note that the declaration isn't valid syntax in D, but the compiler can 
handle it)

How it's lowered isn't technically important, just know that it iterates 
over each key.

-Steve
Dec 12 2022
parent Steven Schveighoffer <schveiguy gmail.com> writes:
On 12/12/22 8:45 PM, Steven Schveighoffer wrote:


 for(auto r = aa.byKey, auto k = r.front; !r.empty; r.popFront)


err... forgot the continual front assignment
I think it's more like:

for(auto r = aa.byKey; !r.empty; r.popFront) {
    auto k = r.front;
    // foreach body
}

-Steve
Dec 12 2022
prev sibling parent reply Per =?UTF-8?B?Tm9yZGzDtnc=?= <per.nordlow gmail.com> writes:
On Monday, 12 December 2022 at 17:29:00 UTC, Steven Schveighoffer 
wrote:

 Removing keys while iterating is not supported. It will break, 
 in confusing ways, and possibly include a null pointer 
 dereference.


IRC, the specs says that it's an error to modify a foreach 
aggregate but the compiler curretly doesn't diagnose it.
Dec 13 2022
next sibling parent Per =?UTF-8?B?Tm9yZGzDtnc=?= <per.nordlow gmail.com> writes:
On Tuesday, 13 December 2022 at 11:22:35 UTC, Per Nordlöw wrote:

 IRC, the specs says that it's an error to modify a foreach 
 aggregate but the compiler curretly doesn't diagnose it.


I believe it should.
Dec 13 2022
prev sibling parent Steven Schveighoffer <schveiguy gmail.com> writes:
On 12/13/22 6:22 AM, Per Nordlöw wrote:

 On Monday, 12 December 2022 at 17:29:00 UTC, Steven Schveighoffer wrote:

 Removing keys while iterating is not supported. It will break, in 
 confusing ways, and possibly include a null pointer dereference.


 
 IRC, the specs says that it's an error to modify a foreach aggregate but 
 the compiler curretly doesn't diagnose it.


It does say this, but the explanation is misleading. The aggregate must 
be *loop invariant*, which means that modifying the aggregate must not 
affect the current iteration state. However, the exact words are:


 The aggregate itself must not be resized, reallocated, free'd, 


reassigned or destructed while foreach is iterating over the elements.

However, comically, just after the example it has this note:


 Note: Resizing or reassigning a dynamic or associative array during 


foreach is still  safe.

Which both contradicts the direct words in the rule, and is wrong in the 
case of resizing an associative array (which can easily mess up the 
foreach iteration).

In reality, the only thing that should be disallowed is invalidating the 
foreach iteration as defined on the original. How this is enforced is 
implementation defined by the *object itself*. The compiler cannot know 
how iteration interacts with an aggregate, so it is on the aggregate to 
define the rules.

As an example, I specifically allowed removal of the currently iterating 
element in my container library, dcollections. The mechanism used was 
opApply with a ref bool that you would set if you wanted to remove the 
element after that loop iteration. This did not change which elements 
were iterated over, so it was loop invariant.

Array and AA iteration rules should be clearly defined. And the general 
rules should be adjusted to delegate iteration restrictions to the 
aggregate, provided the iteration is still loop invariant.

I'll see if I can create a PR to reword the restrictions.

-Steve
Dec 13 2022