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

convert several helper macros into ordinary functions #617

Merged

Conversation

Nathan-Fenner
Copy link
Contributor

This should be a no-behavior change. Here are a few of the macros defined in Brogue.h converted into regular functions.

Some of them are inline statics declared in the same spot as the old macro; others need to be moved into .c files because they rely on globals being in scope.

@tmewett
Copy link
Owner

tmewett commented Nov 18, 2023

briefly remind me of the value add here? ergonomics?

@Nathan-Fenner
Copy link
Contributor Author

Tooling understands functions better than it understands macros, and it's just generally simpler. I want to do some refactorings involving these functions and the ones that use them and it's currently tricky.

  • Macros have weird scoping rules, because variables bind at expansion instead of definition
  • Macros can't (easily) define local variables
  • Macros don't type-check their arguments
  • Macros will duplicate any function calls in their arguments if they use those values multiple times (with no good workarounds because they can't define local variables)

Since semantically, none of these are actually doing macro-ish things that can't be accomplished exactly the same by regular functions, I think it's better to replace them with functions. It makes subsequent refactoring (mostly more pos) easier too.

@tmewett tmewett merged commit 33de9a1 into tmewett:release Nov 19, 2023
1 check passed
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

Successfully merging this pull request may close these issues.

2 participants