Lazarus

Free Pascal => General => Topic started by: lucamar on May 02, 2019, 06:19:01 pm

Title: About For loop variable in nested scopes
Post by: lucamar on May 02, 2019, 06:19:01 pm
First time I see this, so I thought I would better ask, just in case :)

I have a method proc which uses quite a lot of nested little helpers to avoid cluttering the main block. It's something like this (please, note the marked lines):

Code: Pascal  [Select][+][-]
  1. procedure TMainForm.ListKeyDown(Sender: TObject; var Key: Word;
  2.   Shift: TShiftState);
  3. var
  4.   i, Max,
  5.   NewIndex: Integer;
  6.   AList: TStringList;
  7.  
  8.   function NoShift  : Boolean; begin Result := Shift = [];      end;
  9.   function WithAlt  : Boolean; begin Result := Shift = [ssAlt]; end;
  10.   function WithCtrl : Boolean; begin Result := Shift = [ssCtrl]; end;
  11.   function WithShift: Boolean; begin Result := Shift = [ssShift]; end;
  12.  
  13.   function IsCopyCombo: Boolean;
  14.   begin Result := (Key in [VK_C, VK_INSERT]) and WithCtrl; end;
  15.  
  16.   function IsCutCombo: Boolean;
  17.   begin {Like IsCopyCombo} end;
  18.  
  19.   function IsAndCanPaste: Boolean;
  20.   begin {Similar to the previous two} end;
  21.  
  22.   procedure DeleteSelection;
  23.   { Adjusts ItemIndex to keep it on the same or, if deleted, the next file.}
  24.   {Without var i: integer the compiler gives me:
  25.      fmain.pas(471,9) Error: Illegal counter variable
  26.    in the  "for i := FileList.ItemIndex downto 0 do"}
  27.   var
  28.     i: Integer;
  29.   begin
  30.     NewIndex := FileList.ItemIndex;
  31.     for i := FileList.ItemIndex downto 0 do
  32.       if (FileList.Selected[i]) then
  33.         if i <= NewIndex then Dec(NewIndex);
  34.     FileList.DeleteSelected;
  35.     FileList.ItemIndex := NewIndex;
  36.   end;
  37.  
  38.   procedure DoCopy;
  39.   begin { Same error with a "for i :=" loop } end;
  40.  
  41.   procedure DoCutDelete;
  42.   begin { No loop her } end;
  43.  
  44.   procedure DoPaste;
  45.   begin { "for i :=" loop, same error } end;
  46.  
  47. begin
  48. { main block with a case statement checking various key combos
  49.   some of which call the appropiate nested proc from those above}
  50. end;

Is this "Illegal counter variable" a new error in 2.0(.2?) or is it just the first time I have an encounter with it (doubtful as it sounds)?

And what does it mean? The outer i is in scope and is of the correct type
Title: Re: About For loop variable in nested scopes
Post by: marcov on May 02, 2019, 06:38:58 pm
This is compiler, and so FPC 3.0.4, and older Lazaruses based on 3.0.4 should react the same.

Strictly speaking it is not an error, since the innermost i is clearly meant, so that should be ok. However even while legal, it is kind of obfuscating.

Maybe mode objfpc is simply trying to be stricter than Delphi here, you might also want to test with {$mode delphi}
Title: Re: About For loop variable in nested scopes
Post by: Bart on May 02, 2019, 06:57:06 pm
Quote from: lucamar
{Without var i: integer the compiler gives me:
     fmain.pas(471,9) Error: Illegal counter variable

I think Lucamar means this:

Code: Pascal  [Select][+][-]
  1. program test;
  2.  
  3. {$ifdef fpc}
  4. {$mode delphi} //mode does not matter
  5. {$h+}
  6. {$endif}
  7.  
  8. procedure Proc;
  9. var
  10.   i: integer;
  11.   procedure Nested;
  12.   //var
  13.   //  i: integer;
  14.   begin
  15.     for i := 1 to 2 do ;
  16.   end;
  17. begin
  18. end;
  19.  
  20. begin
  21. end.

Fpc complains: "test.pas(15,9) Error: Illegal counter variable"
This is Delphi compatibe (D7): test.pas(15) Error: For loop control variable must be simple local variable.

I could not find this rule in the docs.

Bart

Title: Re: About For loop variable in nested scopes
Post by: Thausand on May 02, 2019, 07:38:40 pm
I not sure but think is because this http://wiki.freepascal.org/User_Changes_3.0#For-in_loop_variable_cannot_be_assigned_to_anymore

Is possible use old compiler.

I ask is compatible delphi then why no need $mode delphi and work other mode ?

@Bart i not can find read help to.

Maybe is logic but work before and not now. Is no good program no use local loop variable but what if want write bad program ?  :D (i have old program other dialect pascal and use many global loop variable (and assignment) and now have write again new).

Maybe i make suggest: if is then have in document for read ?
Title: Re: About For loop variable in nested scopes
Post by: lucamar on May 02, 2019, 07:53:25 pm
I could not find this rule in the docs.

Neither could I, which was what got me thinking.


This is compiler, and so FPC 3.0.4, and older Lazaruses based on 3.0.4 should react the same.

Strictly speaking it is not an error, since the innermost i is clearly meant, so that should be ok. However even while legal, it is kind of obfuscating.

You're right, of course: compiler, not Lazarus. But I had never seen it before. Not that I make a rule of using index vars like that, but I'm almost sure I've used that "style" some other times (in tests, probably) without problems. Rather curious, in fact.

Note that the inner i's were added because the error. In fact, after I added the var i: Integer to all those nested procs I got a Warning: variable "i" never used ("anymore" I would have added :D) for the outer one.

It's not that it matters much. Just one of those things you have to keep in the background of your mind while coding until they become automatic. But it wold be nice if it were mentioned in the docs, say in the Language Reference for example. In the meantime I'll see if I can add a note in the wiki page.
Title: Re: About For loop variable in nested scopes
Post by: furious programming on May 02, 2019, 08:22:51 pm
IMO this is correct behavior and compilation error should be expected. All because the outer variable can be used to iterate another loop (in the outer subroutine), so if you use it to iterate loop in the nested subroutine, then it's value will be modified in two places, and this is bad idea — the counter value will be difficult to predict:

Code: Pascal  [Select][+][-]
  1. {$MODE OBJFPC}{$LONGSTRINGS ON}
  2.  
  3. procedure Main();
  4. var
  5.   I: Integer = 0;
  6.  
  7.   procedure Nested();
  8.   begin
  9.     for I := 1 to 2 do
  10.       WriteLn('  Nested');
  11.   end;
  12.  
  13. begin
  14.   for I := 0 to 3 do
  15.   begin
  16.     WriteLn('Main');
  17.     Nested();
  18.   end;
  19. end;
  20.  
  21. begin
  22.   Main();
  23.   ReadLn();
  24. end.

But still we can replace the for loop into the other and the code will compile:

Code: Pascal  [Select][+][-]
  1. {$MODE OBJFPC}{$LONGSTRINGS ON}
  2.  
  3. procedure Main();
  4. var
  5.   I: Integer = 0;
  6.  
  7.   procedure Nested();
  8.   begin
  9.     while I in [1 .. 2] do
  10.     begin
  11.       WriteLn('  Nested');
  12.       I += 1;
  13.     end;
  14.   end;
  15.  
  16. begin
  17.   for I := 0 to 3 do
  18.   begin
  19.     WriteLn('Main');
  20.     Nested();
  21.   end;
  22. end;
  23.  
  24. begin
  25.   Main();
  26.   ReadLn();
  27. end.

The output will be correct, but will look like invalid:

Code: Pascal  [Select][+][-]
  1. Main
  2. Main
  3.   Nested
  4.   Nested

It's better not to do something like that. Just KISS. 8)


And one thing about modes — if you developing a project in specified dialect, then you should use one dialect in the whole project, in each of it's unit. Using another dialect to fix bad code design is a horrible idea.
Title: Re: About For loop variable in nested scopes
Post by: marcov on May 02, 2019, 08:32:42 pm
Yes. Must be local var, or global to a main program.

The whole for statement is about making it optimizable (e.g.by declaring that the lower and upper bounds must be evaluable before going into the loop), so this is only logical.

Note to furious programming: the nested for would mean changing the loopvar inside the outer FOR by the inner for, which is another such bordercondition. So that doesn't strictly require the "local I only" border condition.

The "must be local var" bordercondition is probably more pragmatic, to avoid having to sort out a whole lot of very complicated, and not terribly useful (since it is a brake on performance) cases.
Title: Re: About For loop variable in nested scopes
Post by: furious programming on May 02, 2019, 08:39:53 pm
Note to furious programming: the nested for would mean changing the loopvar inside the outer FOR by the inner for, which is another such bordercondition. So that doesn't strictly require the "local I only" border condition.

Yep, but in my examples I look at it as a language user, not as a language designer or compiler developer. IMO creating such a nested horror is an obfuscation and it is unlikely to use such a code structure, even if it compiles.
Title: Re: About For loop variable in nested scopes
Post by: lucamar on May 02, 2019, 08:47:10 pm
IMO this is correct behavior and compilation error should be expected. All because the outer variable can be used to iterate another loop (in the outer subroutine), so if you use it to iterate loop in the nested subroutine, then it's value will be modified in two places, and this is bad idea — the counter value will be difficult to predict

I know the theory behind  this behaviour (and the practical reasons which Marco exposed). It just seemed strange that I'd never encountered before. Although ... fact is that I've seen it now only because I'd just started refactoring the code by extracting common parts to inner procs, as the previous step to separating them totally, and I hit F9 to make double-sure I had left nothing dangling in there and to start testing.

Quote
And one thing about modes — if you developing a project in specified dialect, then you should use one dialect in the whole project, in each of it's unit. Using another dialect to fix bad code design is a horrible idea.

Completely in accord with you. I only use Delphi mode for dual compiler code or immediately after converting a Delphi project, and once this last is up and running in Lazarus it gets the full objfpc treatment :)

I don't know where all that talk about modes come from, though :-\
Title: Re: About For loop variable in nested scopes
Post by: marcov on May 02, 2019, 08:51:17 pm
  But it wold be nice if it were mentioned in the docs, say in the Language Reference for example.

Then file a bug.

Quote
In the meantime I'll see if I can add a note in the wiki page.

Please not in the graveyard of obsolete documentation :_)
Title: Re: About For loop variable in nested scopes
Post by: lucamar on May 02, 2019, 08:55:46 pm
Quote
In the meantime I'll see if I can add a note in the wiki page.

Please not in the graveyard of obsolete documentation :_)

OK, if you say so I'll forget about it. Just thought "better a bad place that no place at all". :)
Title: Re: About For loop variable in nested scopes
Post by: marcov on May 02, 2019, 08:58:16 pm
OK, if you say so I'll forget about it. Just thought "better a bad place that no place at all". :)

Well, too often there is simply no difference :). But hey, if users don't think it is important enough to report, then probably it is not important enough to fix either.

Seriously, I consider the wiki to be proving grounds for the next circle of subjects that the docs don't cover yet. But if something is part of the docs, fixes should go to the docs directly. Double administrating that makes no sense.
Title: Re: About For loop variable in nested scopes
Post by: lucamar on May 02, 2019, 09:04:37 pm
Seriously, I consider the wiki to be proving grounds for the next circle of subjects that the docs don't cover yet. But if something is part of the docs, fixes should go to the docs directly. Double administrating that makes no sense.

Yes, but it takes time for the docs to be fixed and distributed, while it takes around a minute to add a small note to the page about "for loops" in the wiki.

Also, note that I didn't say I'll post to the wiki instead  of repoting to mantis.
Title: Re: About For loop variable in nested scopes
Post by: ASBzone on May 02, 2019, 10:17:27 pm
IMO this is correct behavior and compilation error should be expected. All because the outer variable can be used to iterate another loop (in the outer subroutine), so if you use it to iterate loop in the nested subroutine, then it's value will be modified in two places, and this is bad idea — the counter value will be difficult to predict:


But wouldn't that be true if you used a global variable for the loop counter?   (This doesn't generate an error, though)
Title: Re: About For loop variable in nested scopes
Post by: furious programming on May 02, 2019, 10:42:20 pm
@ASBzone: global variable can not be used to iterate any for loop, if such a loop is inside subroutine:

Code: Pascal  [Select][+][-]
  1. {$MODE OBJFPC}{$LONGSTRINGS ON}
  2.  
  3. var
  4.   I: Integer = 0;
  5.  
  6. procedure Main();
  7.  
  8.   procedure Nested();
  9.   begin
  10.     for I := 1 to 2 do
  11.       WriteLn('  Nested');
  12.   end;
  13.  
  14. begin
  15.   for I := 0 to 3 do
  16.   begin
  17.     WriteLn('Main');
  18.     Nested();
  19.   end;
  20. end;
  21.  
  22. begin
  23.   Main();
  24.   ReadLn();
  25. end.

Selected lines gives compilation error: Illegal counter variable.

So, to iterate the for loop, the iterator must be a variable in the same scope. For subroutine it is local variable and only local variable in the same scope. Global variable can be used only inside the main block of the program.
Title: Re: About For loop variable in nested scopes
Post by: Bart on May 02, 2019, 10:51:21 pm
Well, too often there is simply no difference :). But hey, if users don't think it is important enough to report, then probably it is not important enough to fix either.

Filed as issue #35476 (https://bugs.freepascal.org/view.php?id=35476).

ETA: and fixed.

Bart
Title: Re: About For loop variable in nested scopes
Post by: lucamar on May 02, 2019, 11:48:52 pm
Filed as issue #35476 (https://bugs.freepascal.org/view.php?id=35476).

Thanks, Bart. Beat me to it while I was suppering :)
Title: Re: About For loop variable in nested scopes
Post by: ASBzone on May 03, 2019, 12:51:44 am
@ASBzone: global variable can not be used to iterate any for loop, if such a loop is inside subroutine:

Got it.  Thanks
TinyPortal © 2005-2018