issue-#126 Compiler TRAP for SYSTEM.VAL(LONGINT, intreg)

[Ivan Denisov]

I think that compromise solution is to adopt both fixes. Why? They are not contradictory. LouWy version will give correct error position during code parsing and will show errors in many places simultaneously. And Josef fix will catch unpredictable error sources with we did not find yet and provide extra barrier for TRAPS.

[Ivan Denisov]

I have found several places where expressions are used.

CommTCP: SYSTEM.VAL(WinApi.PtrSTR, SYSTEM.ADR(x) + beg)
Meta: SYSTEM.VAL(INTEGER, var.desc.base[0]) = sCharTyp)
StdDebug: SYSTEM.VAL(ArrayPtr, ad - 8)

So it seems that expressions are allowed.

I can not understand. If this code is allowed,

Code: Select all

    MODULE  Test2;
      IMPORT SYSTEM;
      VAR x: LONGINT; y: INTEGER;
    BEGIN
      x := SYSTEM.VAL(LONGINT, y);
    END  Test2.

and this is allowed also

Code: Select all

    MODULE  Test2;
      IMPORT SYSTEM;
      VAR x: LONGINT; y: INTEGER;
    BEGIN
      x := SYSTEM.VAL(LONGINT, 2.0 * y);
    END  Test2.

Why this code is not?

Code: Select all

    MODULE  Test2;
      IMPORT SYSTEM;
      VAR x: LONGINT; y: INTEGER;
    BEGIN
      x := SYSTEM.VAL(LONGINT, y * 2);
    END  Test2.

[cfbsoftware]

I agree with Ivan. If the call to SYSTEM.VAL is legal then it should not trap but it should also not result in an error message.

[Josef Templ]

Chris, I believe the discussion now is in a circle, i.e. we are again at the beginning.
What is this issue all about?

If you want to "convert" an INTEGER into a LONGINT, you can assign it directly or you can use LONG() inside an expression.
There is no need to use SYSTEM.VAL for that. Some people may think that using SYSTEM.VAL is an alternative to LONG and SHORT, but it is not.

SYSTEM.VAL is basically a re-interpretation of something as something else. Very low level and only for experts.
It is platform specific by definition.

If you have an INTEGER "expression" as operand, this is evaluated into a single 32-bit integer register.
Re-interpreting this as a register pair (two independent 32 bit registers) as needed for a LONGINT is not
possible and was not supported in BB 1.6 and is not considered a missing feature.
If you consider it as a missing feature, you must create a feature issue for that.
This issue is only about follow-up TRAPs after emitting the error message.

I really don't understand this big discussion about such a little issue.
It was non-trivial to find the bug but the bug fix is trivial and safe.
Anything beyond that, I strongly believe, is a much bigger issue and not suitable as a last-minute-change.

- Josef

[luowy]

@Ivan, luowy: why is the error reporting behavior in the context of this issue so important for you?

Searching for "DevCPM.err(" in the back-end yields:
Dev/Mod/CPC486.odc 30
Dev/Mod/CPV486.odc 15

so, I say your work is incomplete: do you check all of them? do them have same problem as Covert() has? Handling error follow strategy of backend is a big issue,not just for this TRAP only,it is better make another issue for it.
my fixup just for this " issue-#126 Compiler TRAP for SYSTEM.VAL(LONGINT, intreg)" ;the principle is: if scanner can detect the error,not throw to paser,if the parser can detect the error,not throw to backend; as earlier as possible for accurate error position.

"new code may introduce new bug",yes,maybe:Your feet have grown up,You have to change a new pair of shoes,Uncomfortable for a while is acceptable, that is life.we have several years of experience in it,don't worry, it under control.

I think that compromise solution is to adopt both fixes. Why? They are not contradictory. LouWy version will give correct error position during code parsing and will show errors in many places simultaneously. And Josef fix will catch unpredictable error sources with we did not find yet and provide extra barrier for TRAPS.

I agree.
There's nothing more important than finishing this discussion.

If you have an INTEGER "expression" as operand, this is evaluated into a single 32-bit integer register.
Re-interpreting this as a register pair (two independent 32 bit registers) as needed for a LONGINT is not
possible and was not supported in BB 1.6 and is not considered a missing feature.
If you consider it as a missing feature, you must create a feature issue for that.

yes, it is another thing,we known it already.

luowy

[luowy]

It was non-trivial to find the bug but the bug fix is trivial and safe.

agree,finding the bug is so long,two fixups take me about an hour.discuss it spent about a week.

without this trap message We May never know this problem,end user would not send us a compiler bug view,
instead，he is scratching his hair to find a bug in his source code,
Based on this consideration，I prefer add HALT(100) instead of "RETURN" to catch such uncertain errors,
but this is another topic,ignore it!

luowy

[Josef Templ]

Thanks for you comment. I have read it five times. I am understanding most of it. I will try to understand the all message.
...
How BlackBox can show many errors at the same time if this proposition is true?

Ivan, this shows that you still don't understand the inner working of the compiler and you must accept that
you are not an expert in this matter. This is not a problem but please act accordingly.

The answer to your question is:
The compiler consists of two phases, the front-end is the first phase.
It scans and parses the source code and builds up a so-called abstract syntax tree.
This tree is passed to the second phase, the back-end, which visits all nodes and
generates the object code.

Now comes the important part of this posting:
The code generator does not stop error reporting of the front-end.
All errors detected by the front-end are shown even when the code-generator detects an error.
This follows from the 2-phase approach and because the front-end runs first.
In order to avoid any TRAPs after finding an error in the code
generator, the code generator stops at the next statement. Thus, only the first error
detected by the code generator is shown, but in addition to all errors detected by the front-end.
If the front-end detects any errors, the code generator is not invoked at all.
This is also done in order to avoid any TRAPs in the code generator.

Is it possible that there was a fundamental misunderstanding regarding the error reporting behavior?

- Josef

[Josef Templ]

without this trap message We May never know this problem,end user would not send us a compiler bug view,
instead，he is scratching his hair to find a bug in his source code,
Based on this consideration，I prefer add HALT(100) instead of "RETURN" to catch such uncertain errors,
but this is another topic,ignore it!

luowy

luowy, I never ignore your ideas even if they are as bold as HALT(100) in the compiler.
Actually this brought me to another idea, which is probably closer to what a user would expect.
Here is the less bold idea:
We add another error message that has a more specific text, e.g.
"unsupported mode of second argument of SYSTEM.VAL"

Would that please you?

- Josef

[luowy]

Josef,
IMO,no new fixup need, we have discussed it completely，making your slecetion is good enough:
1,fontend
2,backend
3,frondend and backend
4,other

[Ivan Denisov]

Ivan, this shows that you still don't understand the inner working of the compiler and you must accept that
you are not an expert in this matter. This is not a problem but please act accordingly.

Now I understand the inner working of the compiler better. We have no much experts in compiler, so I had no choice. LuoWy reviewed your solution and reported that it is not sufficient. For this you act as you had worse day in you life. Without any arguments or the discussion you claimed that you are right and you are the only expert. So it was necessary to review your solution also. You can imagine that is was not very nice work and also took my time.

If the expressions are allowed as a second argument of VAL there should not be error message or TRAP. If we now apply the fix for final version we will never find the correct fix for the compiler. So I think, that HALT(100) is good idea. It will remind as, that the back-end is not ideal now. But for users it is good to fix it now in the parser side.

So I think, that we should adopt LuoWy's fix and put HALT(100) instead of RETURN in your fix.