expr.c line 485 is assigning not testing.

[poetnerd]

Sorry to be raising this in an issue. With all the changes I was working through in my silence-warnings branch, it was only after I did my massive merge that I remembered a single line bug I wanted to report and fix.

Line 485 of expr.c reads:

if ((ch () != '+') & (ch () != '-') | nch() == '=')

I'm pretty sure the intent is not to perform bit masking, but to perform comparison. So the line should read:

if ((ch () != '+') && (ch () != '-') || nch() == '=')

[ncb85]

I can see & operation also in other hier methods. If I remember correctly it is used instead of logical AND for reasons, SmallC was to be self compiling, but SmallC did not support logical AND itself. The values of operands should ensure, that it actually behaves like logical AND.
Therefore change to logical operation && in single place might not be enough, more corrections and tests would be needed.

[poetnerd]

How very odd... clang only complained about that one line, and I didn’t read the code carefully. (I was focused on silencing the many errors and warnings.) Reading through expr.c, there are constructs like, “(k & FETCH)” where the intent is clearly testing against a bitmask. But indeed I do see bitmasks where the intent is comparison at lines 253, 282, 306, 311, 336, as well as 485. Line 634 is similar to 485 but with comparisons, not bitmasks. I suspect this is a style change that didn’t get fully implemented. I will update all the lines on a branch myself. What do you use as a test harness? I don’t have an 8080 platform. My interest in SmallC-85 is for a different target — the ancient 1970s vintage PDP-8. I’ve ported the original SmallC PDP-8 target to your codeline and that works fine for me. Unfortunately, the targeting is not clean and messes with many other modules. My plan is to express those deltas with an additional layer of abstraction. But the bottom line is that I can only confirm my changes compile, not that they function on the 8080 target platform.

…

On Dec 30, 2018, at 9:17 AM, Roman ***@***.***> wrote: I can see & operation also in other hier methods. If I remember correctly it is used instead of logical AND for reasons, that SmallC did not support logical AND itself. The values of operands should ensure, that it actually behaves like logical AND. Therefore change to logical operation && in single place might not be enough, more corrections and tests would be needed. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[ncb85]

About line 634. It has been long time since I was focused on SmallC. It really looks like I changed some test to logical while working on struct feature and similar. So someone should finish it properly.

I am using NCB85 SBC with 8085 CPU as test platform. Unfortunately I am rather busy with other projects, and use SmallC only few hours a year.

I would gladly accept patch unifying tests in expr.c and primary.c to use logical operations. I can't promise doing it myself any time soon.
I am using this test suite to test changes to compiler:
https://github.com/ncb85/utilis-and-examples/tree/master/cpm_utest_unit

As I could not test other targets I dropped them, but would also welcome joining them back. So PDP is very welcome.

[poetnerd]

Hi Roman, Thanks for the speedy response. Apologies for taking so long to reply myself. I TOTALLY get it about having many projects going. I have been working with the original author of the PDP-8 target to get it working on SmallC-85. It's now working, but the target is definitely not clean. I'd prefer to hold off trying to share it until I have a better handle on all the changes introduced outside code.c. The modules NOT touched is shorter than the list of those that were touched! There are even bits in main.c that changed. :-( I did the port without understanding it. The next step is to understand the differences, and abstract them out into something that interfaces to the targeting of what is in your tree. That said, the cleanup of expr.c and primary.c is something I have a handle on. I'll try and cook up a patch for just that. Hopefully before too much time elapses. BTW: Thank you VERY much for re-energizing SmallC-85. That code line is SO much cleaner than the original!

…

-Bill Cattey Roman wrote:

About line 634. It has been long time since I was focused on SmallC. It really looks like I changed some test to logical while working on struct feature and similar. So someone should finish it properly. I am using NCB85 SBC with 8085 CPU as test platform. Unfortunately I am rather busy with other projects, and use SmallC only few hours a year. I would gladly accept patch unifying tests in expr.c and primary.c to use logical operations. I can't promise doing it myself any time soon. As I could not test other targets I dropped them, but would also welcome joining them back. So PDP is very welcome. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#8 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAa3mkeYdNXqdxDZhYp0vUuyO330-xJ-ks5u-d4ygaJpZM4Zkw99>.