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

Trailing commas in data declaration adds byte with value $00 #45

Open
dionoid opened this issue May 5, 2020 · 16 comments
Open

Trailing commas in data declaration adds byte with value $00 #45

dionoid opened this issue May 5, 2020 · 16 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@dionoid
Copy link
Member

dionoid commented May 5, 2020

Trailing commas in a data declaration line seems to effectively add a 0 value.
E.g. this code:

PF0Table  ; table 0
        .byte  #%10000000,
        .byte  #%10000000,
        .byte  #%10000000,
        .byte  #%11000000,
(...)

will generate 8 bytes.
This is not expected. You would expect an error from dasm, or maybe have dasm ignore it.

@thrust26
Copy link
Member

thrust26 commented May 5, 2020

Hm, is that even valid syntax? Everything behind the ; should be a comment.

@msaarna
Copy link
Member

msaarna commented May 5, 2020

Maybe it's the forum formatting, but the issue is definitely a bug, and it's been with us a while. Prior to git I've been hit by it myself a few times.
".byte 3,3," will create "030300" in ROM.

@dionoid
Copy link
Member Author

dionoid commented May 5, 2020

Sorry, I made a code formatting mistake. Please see the modified post.
Someone on AtariAge ran into this issue, and I also just been hit by it :-)
I'd like to see if I can fix it myself.

@dionoid dionoid self-assigned this May 5, 2020
@thrust26
Copy link
Member

thrust26 commented May 5, 2020

This will generate 5 (not 8) bytes, correct?

@Rhialto
Copy link

Rhialto commented May 5, 2020

I would not say it is a bug. Other assemblers do this as well. Empty values default to 0.
For the macro-11 assembler for example (one of my favourites) the manual on page 3-17 of
http://www.bitsavers.org/pdf/dec/pdp11/rsx11/RSX11M_V4.1_Apr83/4_ProgramDevelopment/AA-V027A-TC_macro11_Mar83.pdf says at the top "A missing term, expression or external symbol is interpreted as a zero". The macro-11 clone, which I maintain, has explicit tests for this: https://gitlab.com/Rhialto/macro11/-/blob/master/tests/test-word-comma.lst.ok

In particular, you would want

        .byte 1,,3

to be valid and produce the 3 bytes 1, 0, 3. So by the same reasoning,

        .byte 1,,

should produce 1, 0, 0.

So I would say it is a common feature and not a bug.

@andrew-davie
Copy link
Member

In my world, a trailing comma producing an extra value is a bug. It is common to have data with trailing comma in some higher level languages. These do not produce an extra value. It is an unexpected, and undesirable side-effect of dasm, IMHO.
I would prefer to see this fixed.
I do not agree with the above that ".byte 1,,3" should be valid syntax. A comma without a subsequent value, in any situation, should be an error.

@dionoid
Copy link
Member Author

dionoid commented May 6, 2020

I also don't agree that .byte 1,,3 should be a valid syntax. The benefit of not having to type a '0' doesn't outweigh the frustration caused by accidentally typing an extra comma and not being notified by the assembler.
I think a lot of developers (including me) expect a single trailing comma just to be ignored, but I'm okay if DASM would fail on this, which is the most safe way to fix this IMO.

@dionoid dionoid changed the title trailing commas in data declaration adds byte with value $00 Trailing commas in data declaration adds byte with value $00 May 6, 2020
@thrust26
Copy link
Member

thrust26 commented May 6, 2020

I would agree here too. But I am a bit afraid that this might break existing code.

@dionoid
Copy link
Member Author

dionoid commented May 6, 2020

That's why I think the safest solution is to have DASM fail on trailing comma's. Then it won't generate wrong output, and allows the developer to fix this.
@msaarna Mike: do you think this will affect the data-generation features of bB?

@andrew-davie
Copy link
Member

I am not so keen on this "break existing code" argument for stopping correct behaviour in the assembler. There are likely to be very few programs which would break, and in any case the assembler would warn of, and indicate the error, and it would be very easy to fix the offending code. In my view, this should be fixed correctly and not worry about "breaking" existing usage. Particularly for this issue!

@thrust26
Copy link
Member

thrust26 commented May 6, 2020

If the change creates an error for such cases I am fine.

@msaarna
Copy link
Member

msaarna commented May 6, 2020

Changing the missing-value-as-zero behaviour doesn't affect bB. Its parser strikes any trailing commas in data statements, before turning them into the dasm .byte pseudo-ops. It doesn't otherwise generate any assembly with missing values.

@Rhialto
Copy link

Rhialto commented May 6, 2020 via email

@thomas374b
Copy link
Contributor

If someone is going to fix this, please check that your correction will have no side effects. For example
sta ,X
sta 0,X
are different opcodes. The first is indexed X adressing (with zero offset), the latter indexed X+offset adressing. The comma serves here to avoid interpreting 'X' as a symbol. Both are 'legal' statements.

@thrust26
Copy link
Member

Maybe this should be fixed with an option? Which his disabled by default, but can be enabled if old code should break.

@thrust26 thrust26 added bug Something isn't working enhancement New feature or request labels Apr 13, 2021
@thomas374b
Copy link
Contributor

see issue #67 ... Lets make StrictMode the default! ;-)
if old code fails => give option --no-strict

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants