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

Feature creep... macro to call another macro #100

Open
richardturnnidge opened this issue Nov 30, 2024 · 19 comments
Open

Feature creep... macro to call another macro #100

richardturnnidge opened this issue Nov 30, 2024 · 19 comments
Assignees
Labels
enhancement New feature or request

Comments

@richardturnnidge
Copy link

I had a cool way of making a bit of display code very easy, by calling a macro from another.

I made sure the one being called was first in the file.

I thought, great, this is easy.

Back of my mind, I wondered if it would work.

ez80asm gave no errors, so must be ok.

Didn't work.

Looked at LST file, did not include that bit of code, just jumped over it :-(

Read docs, not implemented :-(

Something for the future, when you have nothing else to do!

I re-wrote with functions instead...

@envenomator envenomator added the enhancement New feature or request label Nov 30, 2024
@envenomator
Copy link
Owner

Yup; it's not implemented, nor will it be, very probably. But maybe ;-)

@richardturnnidge
Copy link
Author

No worries, I thought I was being too clever... or optimistic!

You can trust me to try to use features that nobody else would even think existed.

@envenomator
Copy link
Owner

envenomator commented Dec 2, 2024

Well, it looks like I do have something cooked up that seems to work. Included some recursion limit and detection as well. If you'd like to test it out, I can push the branch out tomorrow. I'm going to avoid using the word 'probably' for a while, I guess

@envenomator
Copy link
Owner

@richardturnnidge
Copy link
Author

I will try it out today :-)

@richardturnnidge
Copy link
Author

richardturnnidge commented Dec 3, 2024

nested_macro_test.asm.zip
OK, quick test. Maybe more work that you thought...
Nested macros work with fixed values, but not if values need to be passed into them from the calling macro, eg:

This test calls:
PRINTCAPTION_AT_XY caption2, 2, 15,20
To print a caption with a colour at a x,y tab location.

This WILL work:

macro TAB_TO_XY x, y
ld a, 31
rst.lil $10
ld a, x
rst.lil $10
ld a, y
rst.lil $10
endmacro

macro SETCOLOUR colour
ld a, 17
rst.lil $10
ld a, colour
rst.lil $10
endmacro

macro PRINTCAPTION_AT_XY caption, colour, x, y
SETCOLOUR 5			;colour
TAB_TO_XY 3,5		;x, y 
ld hl, caption
call printString
endmacro

BUT where values should be passed from one macro to another does NOT work and gives error:

macro PRINTCAPTION_AT_XY caption, colour, x, y
SETCOLOUR colour
TAB_TO_XY x, y
ld hl, caption
call printString
endmacro

�[31mMacro [SETCOLOUR] in "nested_macro_test.asm" line 29 - Unknown identifier�[33m 'colour'
�[39m�[33m ld a, colour
�[39m�[33m SETCOLOUR colour
�[39m�[31mInvoked from "nested_macro_test.asm" line 90 as
�[33m PRINTCAPTION_AT_XY caption1, 1, 10,10

@envenomator
Copy link
Owner

Ahh.. the invocation of a macro instruction wasn't ever transformed into macro-local arguments before, because there was no 'level'.

@envenomator
Copy link
Owner

Can you try again with the latest commit in this branch?

@richardturnnidge
Copy link
Author

FABULOUS... initial testing seems to work.

NOTE, all testing done on Mac ez80asm build, not Agon.

First, I tested 1 level of nesting, using the following calls.

PRINTCAPTION_AT_XY caption1, 1, 0,0
PRINTCAPTION_AT_XY caption2, 2, 5,5
PRINTCAPTION_AT_XY caption3, 5, 10,10
PRINTCAPTION_AT_XY caption4, 7, 15,15

To macros:

macro TAB_TO_XY x, y
ld a, 31
rst.lil $10
ld a, x
rst.lil $10
ld a, y
rst.lil $10
endmacro

macro SETCOLOUR colour
ld a, 17
rst.lil $10
ld a, colour
rst.lil $10
endmacro

macro PRINTCAPTION_AT_XY caption, colour, x, y
SETCOLOUR colour
TAB_TO_XY x, y 
ld hl, caption
call printString
endmacro

Screenshot 2024-12-04 at 10 07 37


Then I tested with a second level nesting and all worked still :-)

PRINTCAPTION_AT_XY caption1, 2, 0,0
PRINTCAPTION_AT_XY caption2, 3, 5,5
PRINTCAPTION_AT_XY caption3, 4, 10,10
PRINTCAPTION_AT_XY caption4, 5, 15,15


macro TAB_TO_XY x, y
ld a, 31
rst.lil $10
ld a, x
rst.lil $10
ld a, y
rst.lil $10
endmacro

macro SETCOLOUR colour
ld a, 17
rst.lil $10
ld a, colour
rst.lil $10
endmacro

macro PRINTCAPTION_AT_XY caption, colour, x, y
TAB_TO_XY x, y 
PRINTCOLOURCAPTION colour, caption
endmacro

macro PRINTCOLOURCAPTION colour, caption
SETCOLOUR colour
PRINTCAPTION caption
endmacro

macro PRINTCAPTION caption
ld hl, caption
call printString
endmacro

Screenshot 2024-12-04 at 10 16 17

Well done!!!

Good that order of macros is not important, it finds the macro even if not yet defined.

I will add into my projects over time and should be a good feature for setting up graphics and data were we have fixed data.

I know ultimately it doesn't actually affect binary size, but it does (sometimes) make the code easier to write or to read.

It almost allows the creation of a whole new user defined language in z80 assembly !
However, I appreciate that when we need dynamic data, obviously, functions will be the way to go still.

@envenomator
Copy link
Owner

envenomator commented Dec 4, 2024

Nice. There are still a few things that need to be completed before I'll merge this feature into main. Currently the Macro level is already indicated in the listing, so that's done. I'll need to work on surfacing nested warnings next and the error it throws at a maximum depth. I think for now a sensible depth would be 4-8 nestings deep, let's set it at 8. The stackspace usage isn't very large, but we need to set a max depth to avoid unlimited recursion and crash

@richardturnnidge
Copy link
Author

8 levels deep should be plenty. I had to think hard to get a meaningful use case for 2 levels deep (if what I have done you would deem to be '2 levels' deep'), but I am sure it will arise.

@envenomator
Copy link
Owner

Warning/error surfacing complete, and now show exactly how they were called including expanded arguments. Included your testfile as automated test. Merged with the main branch as it looks solid.

@richardturnnidge
Copy link
Author

Cool, I will do a new build and try it in the morning.

@envenomator
Copy link
Owner

Thinking about things I might miss before doing another release with this code, I only just thought about this:
I don't allow global labels in a macro body, only local- and anonymous labels. When using local labels, upon expanding the macro twice inside the same outside label scope (for example after a global label, before the next global label), you'll get a collision warning that the local label has already been defined. If you invoke it the second time in another global label's scope, that won't happen.

With anonymous labels you won't have this issue, but if the macro is expanded using anonymous labels surrounding the location it's being expanded from, one might be jumping/referring to an unintended anonymous label.

What do you reckon a good solution would be going forward?

  1. Disable local labels within a macro, only allowing anonymous labels, provide a warning in the docs
  2. Create some additional scope code, that would somehow allow macro bodies to have their internal local label scope
  3. Do nothing and mark it as treacherous waters in the docs

Would love to hear your thoughts on this.

@richardturnnidge
Copy link
Author

I'll give it some more thought today, but my initial thoughts are....

So far, I have kept things simple and haven't used labels in macros at all, so the issue/question hasn't arisen. I would have to think of some use cases for making macros bigger that needed those labels. Also, note to self... Labels can be used to hold a variable, or as a jump point...

I had always considered macros as 'fixed' code, rather than really runtime, if that makes sense? As macros in my mind are not 'dynamic' code, but just expanded at assemble time, my gut reaction has been to keep them for simple routines that are just snippets of code that can be read easier with a name assigned to them, such as CLS, TABTO X, Y, etc.

OR, I used macros for a bunch of data to be sent to VDP, which purely builds a bigger string of data, but in an easy to read format.
eg. MAKEBITMAP W,H,ID, file

I have never (so far) made any macros where any decisions need to take place, so need to think of a use case example of that. I have not used JR z, xxxx etc inside a macro, which would be the reason for including local labels

eg, MAKE_RECTANGLE X,Y,X2,Y2 where the code puts predefined values into X, Y, etc.

Nor have I used a global 'variable' within a macro. However, they could to be. Whilst you cannot pass a variable value into a macro like a function, there is no reason that global 'variables' couldn't be used inside the expanded code. Therefore, global labels would be essential.

However, if the macro could refer to labels, then it could be used like a function.
BUT, then the question is, why do a macro if you can do a function?? A function if called more than once, will be more efficient on memory usage.

My assumption was that any labels created inside that macro would be local, because I think the docs said so. But I don't see why they should be. If truly expanded, maybe they should be treated as if they were a global label. ie, Only ever allow a label name to be used once, a global, would make sense to me, but I am not a hardened coder like some. Anonymous labels (which I have only just got my head around in my main body of code) could work, but I can see the issue if you make a call within a macro to another, by the time you expand, that may no longer be the next anon label @@. However, this could also be an issue with any code written, whether inside a macro or not.

So, maybe allow anon labels with the docs caveat of 'watch out', but allow global labels to be used, and cut out local labels?

@envenomator
Copy link
Owner

Thanks for your thoughts on this.
Global labels were never supported, because when macros are expanded twice they will collide with earlier expanded global labels. The same true for 'local' labels, but depends on the caller's scope, I came to realize. This is getting a little out of hand feature-wise. Scoping down labels to their expansion level is something that would be required to never let them collide during expansion. I'll think this over..

@richardturnnidge
Copy link
Author

richardturnnidge commented Dec 10, 2024

Good point about collision.

I would opt for keeping it simple, whatever that is!

I haven't thought of a good/easy use case yet, that couldn't be done better with a function call.

@envenomator envenomator self-assigned this Dec 12, 2024
@envenomator
Copy link
Owner

I'm releasing a new version in a few weeks, early next year. It will likely take a long, long while before I'll release another version after that. I consider this version to be functionality as complete as I want it to be.
I'll consider (small) feature requests and/or bug fixes before releasing.

@richardturnnidge
Copy link
Author

If I spot anything, or hear anything from anyone, will let you know. Other than that, it's a brilliant piece of work, something you should be really proud of achieving.

NOTE: I could not have done all the Agon work without your tools and support. I, and I know everyone else in the Agon community, really appreciate your work on the Agon system. If I ever stop Agon coding, I shall have to re-visit my ZX Spectrum...

I have by some miracle, managed to build ez80asm on my old Raspberry Pi400 today, just to see if I could. I put a new OS on the Pi first. I got Sublime Text working, so just need to link the two the same way I do it on my Mac. I need to get pySerial installed too for sending over UART. Not that I will ever develop on the Pi, just useful to know or document how it can be done.

Thank You.

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

No branches or pull requests

2 participants