issue-#16: TRAP in DevAnalyzer.UseObj

User avatar
DGDanforth
Posts: 1061
Joined: Tue Sep 17, 2013 1:16 am
Location: Palo Alto, California, USA
Contact:

Re: #7 → #16 TRAP in DevAnalyzer.UseObj

Post by DGDanforth »

Good,
The solution of #16 looks good.
I'll start a vote tomorrow.
-Doug
User avatar
DGDanforth
Posts: 1061
Joined: Tue Sep 17, 2013 1:16 am
Location: Palo Alto, California, USA
Contact:

Re: #7 → #16 TRAP in DevAnalyzer.UseObj

Post by DGDanforth »

Chris wrote
In order to cast an informed vote I would like to see a precise definition of the bug - just a single code example that fails does not adequately document the nature of the problem. As a minimum, something on the lines of (assuming my understanding is correct):

DevAnalyzer in Blackbox 1.6 Final identifies when an index variable is modified within a FOR loop.

However in a very specific rare case it results in a trap:

1. There is a nested FOR loop

AND

2. The same index variable is used for each FOR loop

AND

3. The index variable is assigned to another variable within the FOR loop


If the fix solves other instance of traps then they should also be clearly identified. I would also like to see some evidence that regression testing has been done using other relevant code samples to ensure that the fix does not have any unwelcome side effects.
I agree with Chris. This opens a can of worms. For any code fix, how can we know if it introduces other undesired side effects? If the fix is small and "obvious" then we can all agree. But if the fix has any degree of complexity then its consequences will not be obvious (at least to most of us with limited short term memory).

For the DevAnalyzer case one must ask "What is the goal of the analyzer"? Is it to point out invalid "semantics"? Is it to guarantee "correct" execution? What is the "post condition" sought?

Consider

Code: Select all

k := 0; j := 1;
FOR i := 0 TO n-1  DO i := j + k; Out.Int(i); Out.Ln; k := j; j := i; END
Which modifies the loop variable but produces the Fibonacci series. Is that "invalid code"?
To guarantee the loop variable is not modified one would need that variable to be used in a "read only" environment such as

Code: Select all

FOR i := 0 TO n-1  DO P(i) END
where

Code: Select all

PROCEDURE P(i: INTEGER);
...
END P;
To enforce that one would need to modify the CP syntax to only allow procedures with the correct signature in a FOR loop.
Alternatively, Iterators could be used

Code: Select all

it.First;
WHILE it.Next(i) DO ... END;
Where First and Next are methods of an Iterator type.
But that is up to the programmer.

Since DevAnalyzer only gives warnings (with the fix, that is) its misbehavior is not critical and those other, as yet undetected, cases will rear their heads when encountered.
cfbsoftware
Posts: 204
Joined: Wed Sep 18, 2013 10:06 pm
Contact:

Re: #7 → #16 TRAP in DevAnalyzer.UseObj

Post by cfbsoftware »

DGDanforth wrote:To guarantee the loop variable is not modified one would need that variable to be used in a "read only" environment such as

Code: Select all

FOR i := 0 TO n-1  DO P(i) END
where

Code: Select all

PROCEDURE P(i: INTEGER);
...
END P;
There may well be a solution which is less restrictive than that. Recently I modified the ARM Oberon compiler used in Astrobe to prohibit the modification of a FOR loop variable. The solution was simple because Oberon variable identifiers already have an internal read-only property (used for imported items). All I had to do was switch on the read-only property of the index variable while it was in the scope of the FOR loop. It is very likely that a similar technique could be used in Component Pascal as a similar read-only attribute is also available. I think I made use of it some years ago when I was experimenting with extending the usefulness of CP's IN parameters so they could be used to optionally specify scalar variable parameters (INTEGERS, REALS etc.) as read-only just like you can with structured parameters (ARRAY, RECORD).
Ivan Denisov
Posts: 1700
Joined: Tue Sep 17, 2013 12:21 am
Location: Russia

Re: #7 → #16 TRAP in DevAnalyzer.UseObj

Post by Ivan Denisov »

Changing variable inside loop does not prohibited, so we should not do it read only. The warning during the analysis is enough. The principal thing is that TRAP not appearing anymore.
cfbsoftware
Posts: 204
Joined: Wed Sep 18, 2013 10:06 pm
Contact:

Re: #7 → #16 TRAP in DevAnalyzer.UseObj

Post by cfbsoftware »

Ivan Denisov wrote:Changing variable inside loop does not prohibited, so we should not do it read only.
You are correct in that it is not explicitly prohibited in the Component Pascal Language report or in the Oberon Language report. However, that might just mean that the compiler implementer is not obliged to detect it.

Modifying a FOR lop counter is generally considered to be bad programming practice and is likely to result in obtuse code. The reason that it is detected by the analyser is that it is likely to be a programming error rather than an intentional action. The following directions on the use of the FOR loop appear in Wirth's book 'Programming in Oberon' (2011 Revision):
It is recommended that the for statement be used in simple cases only; in particular, no components of the expressions determining the range must be affected by the repeated statements, and, above all, the control variable itself must not be changed by the repeated statements.
I accept that it may be considered too strict to make it an error in the Blackbox compiler. However, I would strongly advise any programmer considering contravening the above directives to use either a WHILE loop or a REPEAT loop instead.
Ivan Denisov
Posts: 1700
Joined: Tue Sep 17, 2013 12:21 am
Location: Russia

Re: issue-#16: TRAP in DevAnalyzer.UseObj

Post by Ivan Denisov »

According the voting this issue can be merged to master.
User avatar
Josef Templ
Posts: 2047
Joined: Tue Sep 17, 2013 6:50 am

Re: issue-#16: TRAP in DevAnalyzer.UseObj

Post by Josef Templ »

I have pulled the changes to master and closed the issue.

I have also removed some of the no longer needed topic branches.

- Josef
Ivan Denisov
Posts: 1700
Joined: Tue Sep 17, 2013 12:21 am
Location: Russia

Re: issue-#16: TRAP in DevAnalyzer.UseObj

Post by Ivan Denisov »

Josef Templ wrote:I have pulled the changes to master and closed the issue.

I have also removed some of the no longer needed topic branches.
Well. I like such working strategy. It works smooth with your help :)
Post Reply