View Issue Details

IDProjectCategoryView StatusLast Update
0035940FPCFCLpublic2019-09-03 15:46
ReporterheXAssigned ToMarco van de Voort 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.3.1Product Build 
Target VersionFixed in Version 
Summary0035940: fcl-stl - support for the standard enumerator class (for in / foreach)
Descriptionhttps://bugs.freepascal.org/view.php?id=23943

in this thread, a person wrote that he made such an implementation, but I don’t know how to contact him.
TagsNo tags attached.
Fixed in Revision42908,42909,42910
FPCOldBugId
FPCTarget-
Attached Files

Activities

heX

2019-08-08 12:35

reporter   ~0117590

Last edited: 2019-08-08 12:35

View 2 revisions

Current enumeration code:

var i: TMapFolderInfo.TIterator;
begin
  i := MapFolderInfo.Iterator();
  if i <> nil then
    try
      repeat
        i.Value.Free();
      until not i.Next;
    finally
      FreeAndNil(i);
    end;



Expected code:

  for i in MapFolderInfo do
    i.Free();

Thaddy de Koning

2019-08-09 09:48

reporter   ~0117600

Last edited: 2019-08-09 10:02

View 2 revisions

Your code is wrong:
Use a while loop, not a repeat loop.
The repeat doesn't take into account the event that there is no i.Next or something to free to start with.
A while loop would break immediately.

After that there is no problem with the code.

Thaddy de Koning

2019-08-09 09:51

reporter   ~0117601

Btw: Michael will read this.

Marco van de Voort

2019-08-09 14:22

manager   ~0117613

For such scheme to work, the iterator must be stack based (a record, like in my lightcontainers) or an interface.

ghashmap's iterators are classes however , so they must be manually freed.

Akira1364

2019-08-19 20:46

reporter   ~0117739

> ghashmap's iterators are classes however , so they must be manually freed.

If we're talking about making TIterator implement the actual GetCurrent / MoveNext set of functions, FPC automatically frees enumerators if they're normal classes that aren't reference counted via interfaces.

For example, this program does not cause a memory leak:

program Test;

{$mode Delphi}
{$PointerMath On}

type
  TItemsEnumerator<T> = class
  public type
    PT = ^T;
  private
    function GetCurrent: T; inline;
  public
    First, Last: PT;
    constructor Create(const AFirst, ALast: PT);
    function MoveNext: Boolean; inline;
    property Current: T read GetCurrent;
  end;

  TItems<T> = class
  private
    Data: array of T;
  public
    constructor Create(const Items: array of T);
    function GetEnumerator: TItemsEnumerator<T>; inline;
  end;

  function TItemsEnumerator<T>.GetCurrent: T;
  begin
    Result := First^;
  end;

  constructor TItemsEnumerator<T>.Create(const AFirst, ALast: PT);
  begin
    First := AFirst - 1;
    Last := ALast;
  end;

  function TItemsEnumerator<T>.MoveNext: Boolean;
  begin
    Result := First < Last;
    Inc(First);
  end;

  constructor TItems<T>.Create(const Items: array of T);
  var Len: SizeInt;
  begin
    Len := Length(Items);
    SetLength(Data, Len);
    Move(Items[0], Data[0], SizeOf(T) * Len);
  end;

  function TItems<T>.GetEnumerator: TItemsEnumerator<T>;
  begin
    Result := TItemsEnumerator<T>.Create(@Data[0], @Data[High(Data)]);
  end;

var
  I: Int32;
  Ints: TItems<Int32>;

begin
  Ints := TItems<Int32>.Create([1, 2, 3, 4, 5]);
  for I in Ints do WriteLn(I);
  Ints.Free;
end.

Personally I just always use record-based enumerators though for the sake of performance.

Marco van de Voort

2019-08-26 11:31

manager   ~0117847

Yes. And I usually use rtl-generics to begin with. But the behaviour should be documented.

Now it is just waiting till sb creates a patch.

Marco van de Voort

2019-09-03 11:57

manager   ~0117933

I added basic support for ghashmap + updated doc example in r42908.

Marco van de Voort

2019-09-03 15:46

manager   ~0117935

Last edited: 2019-09-03 15:46

View 2 revisions

Did some more work, consider it mostly done. If you have concrete other suggestions, please open a new ticket with info.

Issue History

Date Modified Username Field Change
2019-08-08 12:25 heX New Issue
2019-08-08 12:35 heX Note Added: 0117590
2019-08-08 12:35 heX Note Edited: 0117590 View Revisions
2019-08-09 09:48 Thaddy de Koning Note Added: 0117600
2019-08-09 09:51 Thaddy de Koning Note Added: 0117601
2019-08-09 10:02 Thaddy de Koning Note Edited: 0117600 View Revisions
2019-08-09 14:22 Marco van de Voort Note Added: 0117613
2019-08-19 20:46 Akira1364 Note Added: 0117739
2019-08-26 11:31 Marco van de Voort Note Added: 0117847
2019-09-03 11:57 Marco van de Voort Fixed in Revision => 42908
2019-09-03 11:57 Marco van de Voort FPCTarget => -
2019-09-03 11:57 Marco van de Voort Note Added: 0117933
2019-09-03 15:45 Marco van de Voort Fixed in Revision 42908 => 42908,42909,42910
2019-09-03 15:46 Marco van de Voort Assigned To => Marco van de Voort
2019-09-03 15:46 Marco van de Voort Status new => resolved
2019-09-03 15:46 Marco van de Voort Resolution open => fixed
2019-09-03 15:46 Marco van de Voort Note Added: 0117935
2019-09-03 15:46 Marco van de Voort Note Edited: 0117935 View Revisions