Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow negative constants as operand (works for equb/equw/equd!) #82

Open
0xC0DE6502 opened this issue Aug 31, 2022 · 10 comments
Open

Allow negative constants as operand (works for equb/equw/equd!) #82

0xC0DE6502 opened this issue Aug 31, 2022 · 10 comments

Comments

@0xC0DE6502
Copy link

Using a negative constant as an operand issues an error whereas negative constants work just fine for equb/w/d. I think this should be allowed for operands as well.

Examples:
lda #-3, lda #not(1), etc will give the error "Constant cannot be negative"
Workarounds: lda #256-3, lda #lo(-3), lda #1 eor 255, etc.

On the other hand:
equb -3, equw not(1), etc is all just fine and generates exactly what you'd expect.

@ZornsLemma
Copy link
Collaborator

ZornsLemma commented Aug 31, 2022

I think this is a good idea, if we can still successfully generate errors for things which don't sensibly fit in 8 bits. So "lda #-3" should work but "lda #-130" should not. Then again, I just tried it and "equb -130" works, so maybe it's inconsistent not to allow this for immediate operands. Would it break a lot of code if we disallowed things like "equb -130"?

Edit: Just played around a bit more, and both "lda #256" and "equb 256" generate an error, i.e. we do check that things "fit" with positive numbers in both cases. This makes checks on "too large negative numbers" feel more reasonable from a consistency point of view.

Edit: I think -130 is a reasonable example of an "invalid" 8-bit negative value, but I'm starting to second-guess myself. Maybe pretend I'm using something like -257 if it matters. :-)

@0xC0DE6502
Copy link
Author

At the very least the behaviour of equb/w/d should be consistent with operands. I like how equb/etc is implemented at the moment. Integers just "wrap around" the byte, word or double word boundary. For instance: equw -65535 is equal to equw 1. This makes sense to me from a low level point of view, but it could just be me :)

If you start treating integers like real signed integers then e.g. lda #128 should issue a warning/error because 128 is technically out of range for 8bit signed integers.

@ZornsLemma
Copy link
Collaborator

To my mind, 128 is OK because we don't know if it's being treated as signed or unsigned, but -130 isn't because this isn't possible in either signed or unsigned representations. I appreciate to some extent this is just down to personal taste.

In practical terms, what I really care about is that something like:

.table_start
    EQUB 1:EQUW foo
    EQUB 100:EQUW bar
    ... ; arbitrary amount of data
.table_end

    LDY #0
.loop
    LDA table_start,Y
    STA &2000,Y
    INY
    CPY #(table_end-table_start)
    BNE loop

generates an error rather than quietly assembling nonsense if the CPY # operand doesn't fit in 8 bits as the table grows during development.

@SteveFosdick
Copy link

There is nothing wrong with trusting the programmer. An assembly language programmer will obvious know about wrap around and sign changing. On the other hand the assembler has the potential to spot a mistake the programmer has not. The complication is that there is often not enough information to tell if the programmer intended a value to be treated as unsigned or signed so if limits are to be imposed, or at least warnings issued, the allowed range for an 8-bit quantity really needs to be -128 to +255.

@SteveFosdick
Copy link

Also +1 for ZornsLemma's point about calculated values - this is where they are most likely to overflow without the programmer being aware.

@ZornsLemma
Copy link
Collaborator

ZornsLemma commented Aug 31, 2022

Following on from SteveFosdick's point, my feeling is - ignoring possible unknown problems with existing code - we should validate in the range -128 to 255 inclusive for bytes and -32768 to 65535 inclusive for words, behaving consistently with immediate operands and equ*. This allows things like "equw -1" (which I agree is reasonable and use myself) but will catch values that don't make sense as signed or unsigned. If the programmer is doing something unusual/clever they always have the option to do something like "AND &FFFF" to force the value into range so beebasm lets it through.

Edit: I forgot what 0xC0DE's example was - this wouldn't allow equw -65535 without forcing it in some way. I agree this makes a certain logical sense, but personally I wouldn't write that under normal circumstances and it wouldn't bother me to have to use "AND &FFFF" to get beebasm to allow it if I did it deliberately rather than by accident.

@ZornsLemma
Copy link
Collaborator

A compromise option would of course be to keep the inconsistency between immediate operands and equates - which we already have, so we're not making things worse - and just extend the range of acceptable values for immediate operands to -128 to 255 inclusive.

@mungre
Copy link
Collaborator

mungre commented Aug 31, 2022

FWIW, the current behaviour is consistent with BBC BASIC.

+1 on checking calculated values. I'm wary of permitting negative immediate operands because a negative result might work well as 255 but it might just be wrong.

I'd be inclined to leave it alone but I don't feel particularly strongly about this.

@0xC0DE6502
Copy link
Author

I appreciate there may be unexpected consequences of changing behaviour like this and I have my personal preferences as well as workarounds in beebasm. I do like the compromise as suggested by ZornsLemma though.

Isn't it weird and unexpected though that e.g. lda #not(%0000_0001) gives the "negative constant" error as well? I would expect it to have the same effect as lda #&fe.

@chriskillpack
Copy link
Collaborator

I've been following this discussion and on stardot and I think this is a future improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants