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

Fix empty macro yield value be nil instead of nop #10448

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

straight-shoota
Copy link
Member

Fixes #10406

@straight-shoota
Copy link
Member Author

It seems this broke something in the compiler... 👀

@HertzDevil
Copy link
Contributor

Will begin; end work?

@straight-shoota
Copy link
Member Author

I'll try that. Not sure if it helps.

@straight-shoota
Copy link
Member Author

Still fails. But this looks like a parser bug, actually.

@straight-shoota
Copy link
Member Author

Okay the implementation was just wrong because it replaced every Nop when it should only replace Nop in a Yield.

The actual stack overflow is just #10409 and it seems like the stacktrace incorrectly points to the parser as the last frames.

@HertzDevil
Copy link
Contributor

My suggestion to use begin; end is due to #5258, by the way; an empty {{ yield }} would then become an empty Expressions (though the same needs to be done for single-expression blocks too).

@straight-shoota
Copy link
Member Author

That distinction doesn't really matter here because it's never expressed in the AST. It just happens in the method that stringifies the generated macro code. In the result there is no more yield that would be expected to be an Expressions.
I don't care much either way, though.

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

Successfully merging this pull request may close these issues.

Empty macro yield causes broken code
2 participants