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

Macro parameter identifier parsing appears too eager #327

Open
mellonpizza opened this issue Aug 20, 2024 · 2 comments
Open

Macro parameter identifier parsing appears too eager #327

mellonpizza opened this issue Aug 20, 2024 · 2 comments

Comments

@mellonpizza
Copy link

mellonpizza commented Aug 20, 2024

assert 0 <= 1, "We prove that 1 > 0, or maybe 1 == 0."
macro foobuz()
assert 0 <= 1, "Yes, 0 <= 1. Duh."
endmacro
macro footgun()
assert 0 <= 1, "Well, 1 > 0, but..."
endmacro
%foobuz()
%footgun()

This throws an error when invoking %footgun() that there is an "Invalid macro parameter name." It seems to be interpreting <= 1, "Well, 1 > as a parameter despite the vastly different contexts. Similar code written for 1.81 didn't run into this issue. I've encountered this happening on strings that contain < and > with unrelated symbols in the middle as well, so my assumption is detection of macro parameters in strings is far too eager than it was in 1.81.

Tested on both randombot and linux on version 1.91.

@randomdude999
Copy link
Collaborator

asar/src/asar/macro.cpp

Lines 206 to 211 in 5e0cdba

// RPG Hacker: Added checking for space here, because this code would consider
// if a < b && a > c
// a macro arg expansion. In practice, this is still a sloppy solution and is
// likely to fail in some edge case I can't think of right now. Should parse
// this in a much more robust way at some point...
if (*end==' ')

looks like we need to hack this condition some more.... and i have no idea what we're gonna do in asar 2.0, when if conditions become normal expressions and thus don't need to have the extra space there. i hate how overloaded these symbols are in general.......

@mellonpizza
Copy link
Author

worst case I'd not mind having escaped < > in strings as an option, though I figure that wouldn't be ideal in general. Otherwise something like an explicit evaluation context ( usually curly braces in other languages ) seems like itd be a breaking change which may be undesirable, but maybe ok for 2.0 if everyone agreed?

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

No branches or pull requests

2 participants