issue-#16: TRAP in DevAnalyzer.UseObj
- DGDanforth
- Posts: 1061
- Joined: Tue Sep 17, 2013 1:16 am
- Location: Palo Alto, California, USA
- Contact:
Re: #7 → #16 TRAP in DevAnalyzer.UseObj
Good,
The solution of #16 looks good.
I'll start a vote tomorrow.
-Doug
The solution of #16 looks good.
I'll start a vote tomorrow.
-Doug
- DGDanforth
- Posts: 1061
- Joined: Tue Sep 17, 2013 1:16 am
- Location: Palo Alto, California, USA
- Contact:
Re: #7 → #16 TRAP in DevAnalyzer.UseObj
Chris wrote
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
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
where
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
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.
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).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.
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
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
Code: Select all
PROCEDURE P(i: INTEGER);
...
END P;
Alternatively, Iterators could be used
Code: Select all
it.First;
WHILE it.Next(i) DO ... END;
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.
-
- Posts: 204
- Joined: Wed Sep 18, 2013 10:06 pm
- Contact:
Re: #7 → #16 TRAP in DevAnalyzer.UseObj
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).DGDanforth wrote:To guarantee the loop variable is not modified one would need that variable to be used in a "read only" environment such aswhereCode: Select all
FOR i := 0 TO n-1 DO P(i) END
Code: Select all
PROCEDURE P(i: INTEGER); ... END P;
-
- Posts: 1700
- Joined: Tue Sep 17, 2013 12:21 am
- Location: Russia
Re: #7 → #16 TRAP in DevAnalyzer.UseObj
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.
-
- Posts: 204
- Joined: Wed Sep 18, 2013 10:06 pm
- Contact:
Re: #7 → #16 TRAP in DevAnalyzer.UseObj
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.Ivan Denisov wrote:Changing variable inside loop does not prohibited, so we should not do it read only.
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):
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.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.
-
- Posts: 1700
- Joined: Tue Sep 17, 2013 12:21 am
- Location: Russia
Re: issue-#16: TRAP in DevAnalyzer.UseObj
According the voting this issue can be merged to master.
- Josef Templ
- Posts: 2047
- Joined: Tue Sep 17, 2013 6:50 am
Re: issue-#16: TRAP in DevAnalyzer.UseObj
I have pulled the changes to master and closed the issue.
I have also removed some of the no longer needed topic branches.
- Josef
I have also removed some of the no longer needed topic branches.
- Josef
-
- Posts: 1700
- Joined: Tue Sep 17, 2013 12:21 am
- Location: Russia
Re: issue-#16: TRAP in DevAnalyzer.UseObj
Well. I like such working strategy. It works smooth with your helpJosef 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.