rgbds icon indicating copy to clipboard operation
rgbds copied to clipboard

Implement [[ inline fragments ]]

Open Rangi42 opened this issue 4 years ago • 20 comments

Fixes #500

Anywhere you would normally use an address, such as dw Label or ld hl, Label or call Label, you can use an [[ inline fragment ]] instead. Each inline fragment becomes its own SECTION FRAGMENT to be combined with the current section.

Rangi42 avatar Jan 24 '21 03:01 Rangi42

~~I made it require a newline after the opening [[ because each line of code in the block has to have a newline after it, so the ]] ends up on its own line anyway.~~ After #842 fixes the EOF-newline lexer hack, this can allow inline fragments without newlines.

Rangi42 avatar Jan 24 '21 03:01 Rangi42

I made it require a newline after the opening [[ because each line of code in the block has to have a newline after it, so the ]] ends up on its own line anyway.

This is a shame; single lines of code (such as a simple [[ db "Hello world!" ]]) would look much better without newlines, particularly as macro (or dw) arguments.

aaaaaa123456789 avatar Jan 24 '21 04:01 aaaaaa123456789

I made it require a newline after the opening [[ because each line of code in the block has to have a newline after it, so the ]] ends up on its own line anyway.

This is a shame; single lines of code (such as a simple [[ db "Hello world!" ]]) would look much better without newlines, particularly as macro (or dw) arguments.

Mildly agree, but I don't see a non-hacky way for ]] to pass two tokens at once, T_NEWLINE and T_OP_2RBRACK.

Rangi42 avatar Jan 24 '21 04:01 Rangi42

The proper fix is to remove the lexer hack of forcing a newline at EOF, and allowing the last line not to be followed by a newline.

ISSOtm avatar Jan 25 '21 10:01 ISSOtm

The proper fix is to remove the lexer hack of forcing a newline at EOF, and allowing the last line not to be followed by a newline.

Currently the line_directive rules (for macrodef, rept, for, if, elif, else, endc) depend on trailing newlines. To avoid this PR also being a refactor of those rules (which are "DEFINITELY one of the more FRAGILE parts of the codebase"), I'd suggest allowing [[ single-line inline fragments ]] as a separate PR. That way this one can focus on the core addition of [[ code ]] turning into a SECTION FRAGMENT.

Edit: The newline issue has already been fixed in master, so this PR can support single-line fragments now. :)

Rangi42 avatar Jan 29 '21 21:01 Rangi42

More generally, after seeing some of the code written using this, I'm not fond of the syntax. It creates extra indentation, which would be fine if "column 1" wasn't a thing... And it gets really confusing as soon as it's nested.

That's my opinion, I'd like to hear others'.

ISSOtm avatar Feb 11 '21 23:02 ISSOtm

"Column 1" doesn't have to be a thing for labels: #635 addresses that, though it still requires constants to be defined in column 1 and macros to be invoked past column 1.

Rangi42 avatar Feb 12 '21 00:02 Rangi42

#635 has problems, and won't be merged for a while due to the required deprecations.

ISSOtm avatar Feb 12 '21 00:02 ISSOtm

Weren't the problems due to allowing non-local labels without colons? All of these cases should be unambiguous:

Label: db 1
.localA db 2
.localB db 3
    Label2: db 4
    .localA db 5
    .localB db 6
X EQU 7
    mac EQU 7 ; passes 'EQU 7' as \1
Y = 8
S EQUS "hi"
rsset
Q rl 2
    mac rl 2 ; passes 'rl 2' as \1

Rangi42 avatar Feb 12 '21 00:02 Rangi42

This is not what it currently does; though this would be better continued there.

ISSOtm avatar Feb 12 '21 00:02 ISSOtm

Regarding the syntax, my examples in the test cases are more to just demonstrate many technically valid things using it. I think ax6's example is more representative of usage in practice (just imagine double-brackets there), and even though it has nested inline fragments, I would find it more readable than the equivalent with named labels. You do have to use good judgment to not nest the fragments with many levels of indentation just because it's technically possible.

Rangi42 avatar Feb 12 '21 02:02 Rangi42

@aaaaaa123456789 @mid-kid @pinobatch @AntonioND @NieDzejkob @JL2210

What do you think of the syntax for these? (Please see discussion above and in #500.)

Rangi42 avatar Feb 17 '21 02:02 Rangi42

So taking the example you linked to, I've adapted it to the current syntax (and adapted the whitespace because I prefer it this way):

    ld      hl, .table - 2
.loop
    inc     hl
    inc     hl
    cp      [hl]
    inc     hl
    jr      nc, .loop
    ld      a, [hli]
    ld      h, [hl]
    ld      l, a
    jp      hl

.table
    dbw     50, [[
        ld      hl, [[
            db      "Try harder!", 0
        ]]
        jp      ShowMessage
    ]]
    dbw     120, [[
        ld      hl, [[
            db      "Good work.", 0
        ]]
        call    ShowMessage
        ld      hl, SND_CHIME
        jp      PlaySound
    ]]
    dbw     255, [[
        ld      hl, [[
            db       "Excellent work!", 0
        ]]
        call    ShowMessage
        ld      hl, SND_FANFARE
        jp      PlaySound
    ]]

Personally I think the syntax is fine. I generally indent my asm code as many levels as loops I have, so this feels ok.

However, yes, I think that a future PR should allow one-liners, as it certainly helps readability. It's not needed to do it in this PR.

    ld      hl, .table - 2
.loop
    inc     hl
    inc     hl
    cp      [hl]
    inc     hl
    jr      nc, .loop
    ld      a, [hli]
    ld      h, [hl]
    ld      l, a
    jp      hl

.table
    dbw     50, [[
        ld      hl, [[ db "Try harder!", 0 ]]
        jp      ShowMessage
    ]]
    dbw     120, [[
        ld      hl, [[ db "Good work.", 0 ]]
        call    ShowMessage
        ld      hl, SND_CHIME
        jp      PlaySound
    ]]
    dbw     255, [[
        ld      hl, [[ db "Excellent work!", 0 ]]
        call    ShowMessage
        ld      hl, SND_FANFARE
        jp      PlaySound
    ]]

AntonioND avatar Feb 17 '21 17:02 AntonioND

However, yes, I think that a future PR should allow one-liners, as it certainly helps readability. It's not needed to do it in this PR.

This PR does allow one-liners. :) The "EOF-newline" lexer hack was removed in b3c0db218d564bbc9a801d8635591fd969c36e82 so things like ld hl, [[ db "Good work.", 0 ]] are fine.

Rangi42 avatar Feb 17 '21 17:02 Rangi42

Due to many complications with removing the lexer hack, revolving around "lexer-sensitive" directives being input at EOF (see cd072d5e6a9031ae326e9d95c7680bde94a88d81 and tests introduced by #750), and a feature-freeze so that testing can take place, this is likely postponed for after the next release.

ISSOtm avatar Feb 21 '21 02:02 ISSOtm

Now that #750 has merged, this PR would need updating to no longer support single line [[ inline fragments ]]. It should wait until a solution exists.

Rangi42 avatar Mar 03 '21 01:03 Rangi42

Although the lexer's EOF-newline hack has been removed, this still needs to inject a token to end the contents of an inline fragment, in case there's no newline (for example, dw [[ db "Hello", 0 ]]). (The facility to inject a token already exists for the sake of lexing $ff00 + c.) It can't reuse T_EOB for that because that has extra meaning (it ends a line and causes yylex to call yywrap.)

(The alternative, I think, would be to implement fragment_body as an altered copy of asmfile that allows the last line to not have a terminator. This would be more repetitive and fragile than the T_EOL solution; and a line-ending token that doesn't end its buffer or file might be useful for other things, like #805.)

Rangi42 avatar Apr 20 '21 17:04 Rangi42

Some alternate delimiter suggestions besides [[ nop ]]: [: nop :], [| nop |], <[ nop ]>, <| nop |>, <: nop :>. (Trivia: C has <: and :> as alternative tokens for [ and ].)

Rangi42 avatar Apr 23 '21 19:04 Rangi42

Following Discord discussion with @ISSOtm , this would need to be compatible with expressions even after lazy evaluation, user-defined functions, and OPCODE are implemented, so it's on hold until those.

Rangi42 avatar May 30 '21 17:05 Rangi42

Oh hey, asmotor has had these since 2019:

Speaking of strings, code and data literals can be used to reduce clutter and improve readability. To load the register a0 with the address of a string, you might do

lea { DC.B "This is a string",0 },a0

or to produce the address of a chunk of code

jsr {
	moveq #0,d0
	rts
}

(It doesn't allow {interpolation} outside of strings, so curly braces are available for that.)

The implementation is similar to this PR: when it sees an opening brace, push the section stack, create a new section (albeit with a new section not a fragment of the current one) and a label, parse until the closing brace, pop the stack, and evaluate as the new label. That makes me somewhat more confident in it (not that asmotor has a large user base to stress-test it, but they at least aren't handling some error case I missed).

Rangi42 avatar Dec 22 '23 21:12 Rangi42