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

Implement [[ inline fragments ]] #716

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Rangi42
Copy link
Contributor

@Rangi42 Rangi42 commented Jan 24, 2021

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
Copy link
Contributor Author

Rangi42 commented Jan 24, 2021

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.

@aaaaaa123456789
Copy link
Member

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.

@Rangi42
Copy link
Contributor Author

Rangi42 commented Jan 24, 2021

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 Rangi42 force-pushed the inline-blocks branch 3 times, most recently from faeb5d5 to bc6bd22 Compare January 24, 2021 22:51
@ISSOtm
Copy link
Member

ISSOtm commented Jan 25, 2021

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.

@Rangi42 Rangi42 changed the title Implement [[ inline code blocks ]] Implement [[ inline fragments ]] Jan 27, 2021
@Rangi42
Copy link
Contributor Author

Rangi42 commented Jan 29, 2021

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 Rangi42 force-pushed the inline-blocks branch 7 times, most recently from ddbb53f to f5b93f4 Compare February 11, 2021 17:46
@Rangi42 Rangi42 marked this pull request as ready for review February 11, 2021 17:46
ISSOtm
ISSOtm previously requested changes Feb 11, 2021
src/asm/parser.y Show resolved Hide resolved
src/asm/parser.y Outdated Show resolved Hide resolved
src/asm/parser.y Outdated Show resolved Hide resolved
src/asm/parser.y Outdated Show resolved Hide resolved
src/asm/rgbasm.5 Outdated Show resolved Hide resolved
src/asm/rgbasm.5 Outdated Show resolved Hide resolved
src/asm/rgbasm.5 Outdated Show resolved Hide resolved
src/asm/rgbasm.5 Outdated Show resolved Hide resolved
src/asm/section.c Outdated Show resolved Hide resolved
src/asm/section.c Outdated Show resolved Hide resolved
@ISSOtm
Copy link
Member

ISSOtm commented Feb 11, 2021

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'.

@Rangi42
Copy link
Contributor Author

Rangi42 commented Feb 12, 2021

"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.

@ISSOtm
Copy link
Member

ISSOtm commented Feb 12, 2021

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

@Rangi42
Copy link
Contributor Author

Rangi42 commented Feb 12, 2021

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

@ISSOtm
Copy link
Member

ISSOtm commented Feb 12, 2021

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

@Rangi42 Rangi42 force-pushed the inline-blocks branch 2 times, most recently from 5f07aca to be3186f Compare February 12, 2021 01:31
@Rangi42
Copy link
Contributor Author

Rangi42 commented Feb 12, 2021

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
Copy link
Contributor Author

Rangi42 commented Feb 17, 2021

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

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

@Rangi42 Rangi42 linked an issue May 30, 2021 that may be closed by this pull request
@Rangi42 Rangi42 force-pushed the inline-blocks branch 2 times, most recently from e760ae5 to 33ddac8 Compare June 27, 2021 19:16
@Rangi42 Rangi42 added this to the v0.6.0 milestone Jul 4, 2021
@Rangi42 Rangi42 added the WIP This PR is a work in progress label Jul 4, 2021
@Rangi42 Rangi42 marked this pull request as draft July 4, 2021 21:17
@Rangi42 Rangi42 force-pushed the inline-blocks branch 2 times, most recently from 7a5fdd5 to f384216 Compare August 21, 2022 04:21
@Rangi42 Rangi42 marked this pull request as ready for review August 21, 2022 04:31
@Rangi42 Rangi42 dismissed ISSOtm’s stale review August 30, 2022 21:25

Changes made, pending lazy evaluation

@Rangi42 Rangi42 removed the WIP This PR is a work in progress label Aug 31, 2022
@Rangi42 Rangi42 added the WIP This PR is a work in progress label Nov 7, 2023
@Rangi42 Rangi42 marked this pull request as draft November 7, 2023 00:47
@Rangi42
Copy link
Contributor Author

Rangi42 commented Dec 22, 2023

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 Rangi42 removed this from the v0.10.0 milestone Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Typically new features; lesser priority than bugs rgbasm This affects RGBASM rgblink This affects RGBLINK WIP This PR is a work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Literals (inline section fragments)
4 participants