asar icon indicating copy to clipboard operation
asar copied to clipboard

add macro-local functions

Open exodustx0 opened this issue 10 months ago • 3 comments

asar 1.90
arch 65816
lorom

macro ModifySomething(op)
	function isOp(x) = stringsequal("<op>", x)

	<op> $0000
	if isOp("inc")
		; Maybe inc does something we need to account for.
	elseif not(isOp("dec"))
		; Who knows what a non-inc/dec implies; this is a bug repro, not some profound source of logic.
	endif
endmacro

org $8000
%ModifySomething(inc, $00)
%ModifySomething(dec, $01)

This produces warning: (Wfeature_deprecated): DEPRECATION NOTIFICATION: Feature "overwriting a previously defined function" is deprecated and will be REMOVED in the future. Please update your code to conform to newer styles. Suggested work around: change the function name. [function isOp(x) = stringsequal("dec",x)] on the second %ModifySomething() call.

Functions are not treated as local in macros, even when it might look that way. This means defining a function inside a macro causes the function to be redefined, with the same name, every time the macro is invoked. Ideally, functions should be local to the macros they are defined in, for improved legibility. Or if that is too much, the quirk should at least be documented.

In the interim, the temporary workaround is to sandwich the function definition between warnings disable Wfeature_deprecated and warnings enable Wfeature_deprecated, and it'll work just fine. A less ugly workaround would've been undefing functions, but as far as I can see, there's no functionality for this in place.


Since the above reproducer doesn't really give the most insight into the potential use-case for macro-local functions, here's the full macro I wrote for a project I'm working on:

; Set o to "o op A".
; Supported ops: sta, stz, adc, sbc, inc, dec, and, ora, eor, trb, tsb, asl, lsr, rol, ror
macro ModifyOpt(op, o)
	; The warning disable/enable are somewhat horrendous, but they circumvent how
	; the fact that functions are *not* local to the macro causes any macro
	; call after the first to cause a deprecated feature warning for
	; "overwriting a previously defined function".
	warnings disable Wfeature_deprecated
	function isOp(x) = stringsequal("<op>", x)
	warnings enable Wfeature_deprecated

	ldx.w	#!o<o>
	stx		CmdBuffer+!pSetOpt_Index
	<op>	Opts, X
	if isOp("stz") || isOp("inc") || isOp("dec") || isOp("asl") || isOp("lsr") || isOp("rol") || isOp("ror") || isOp("trb") || isOp("tsb")
		; These ops modify mem, so we load to A.
		lda		Opts, X
	elseif isOp("adc") || isOp("sbc") || isOp("and") || isOp("ora") || isOp("eor")
		; These ops modify A, so we store to mem.
		sta		Opts, X
	elseif isOp("sta")
		; A and mem are already set.
	else
		error "Unsupported op '<op>'"
	endif
	sta		CmdBuffer+!pSetOpt_Data
	%SetCmd(SetOpt)
	jsr		SendCmd
endmacro

Hopefully this gets the point across; replacing isOp( with stringsequal("<op>", is rather ugly indeed. Using this macro increases the legibility of my code significantly. While I would say this issue is low-priority due to its workaroundability, I would love to see this fixed at some point. Care should be taken when implementing this, notably in how calling a function inside a macro scope would necessitate doing two function lookups, one macro-local and one global.

exodustx0 avatar Dec 30 '24 19:12 exodustx0

With the expectation of macro labels, all side effects within macros are global. This includes regular labels, defines, structs, functions, namespaces, all directives, and relative labels (possibly other things too).

Asar does not consider macros to be a scope. I'm not going to say this is strictly wrong, but I'm not sure this is reasonable to fix. On one hand, fixing functions wouldn't break. On the other this would suggest the others should also be fixed, and those could (and would) break significant amounts of existing code.

Another work around to suggestion: !isOp = stringsequals

!isOp(",....)

Open to more thoughts and opinions. For example a function macro syntax similar to label macro syntax.

(Typed on Mobile, apologies for typos)

p4plus2 avatar Dec 30 '24 21:12 p4plus2

While shortening the function name to check against op with is part of my use-case, it's mostly removing the need to include "<op>" inside every call that is my goal. Besides, if that was all, I'd use function isOp(x, op) = stringsequals(x, op) outside of the macro and be done with it.

Unfortunately, counter to the idea of macros not having a scope, macros are required to close every if-scope they open (but not, for example, namespace and struct scopes). This means I can't do this:

macro CondAny(op, x, cond, ...)
	!res #= 0
	for i = 0..sizeof(...)
		!res #= !res|<cond>("<x>", "<...[!i]>")
	endfor
	<op> !res
endmacro

macro DoOp(op)
	%CondAny(if, <op>, stringsequal, stz, inc, dec, asl, lsr, rol, ror, trb, tsb)
		db $01
	%CondAny(elseif, <op>, stringsequal, adc, sbc, and, ora, eor)
		db $02
	%CondAny(elseif, <op>, stringsequal, sta)
		db $03
	else
		error "Unsupported op '<op>'"
	endif
endmacro

I'd exclude the actual conditonal directive from the macro parameters, but a macro is only treated as such if it's alone on a line.

I'm rather fond of the idea of some kind of macro function syntax. I agree that that suggests the others should have macro syntax too, but then, there being macro labels already somewhat does that, in my mind.

exodustx0 avatar Dec 31 '24 00:12 exodustx0

While that would indeed be a useful feature, it would also open a huge can of gnarly edge cases.

The obvious implementation would be expanding macros in if 0 - but what if one of the macros is only defined inside another if 0? What if a macro has multiple definitions (possibly with different argument count), with an if to pick the right one? What if a macro is recursive, with an if statement on the base case? What if a macro is 1000 lines long and chewing through that noticably impacts the program's performance?

That's the kind of questions that must be answered before we can implement this kind of feature. And if there is no good answer, then we can't implement it.

(Yes, Asar is already full of features carelessly tossed together without considering such edge cases. We're trying to not make it worse.)

Macro-local functions, however, are a quite reasonable request.

To solve the dual-scope issue, I'd simply give them a dedicated syntax, like macro labels. That will also simplify the implementation - I think we can reuse most of the macro label handler.

(Could even do macro-local defines, though they can already be redefined freely, and aren't in the math parser, so they're more effort for less gain. Not useless if you're doing something recursive, but probably not worth it.)

Alcaro avatar Dec 31 '24 01:12 Alcaro