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

Function inlining does not work with guard clauses #70

Closed
JeanJPNM opened this issue Jun 15, 2022 · 5 comments · Fixed by #80, #81 or #87
Closed

Function inlining does not work with guard clauses #70

JeanJPNM opened this issue Jun 15, 2022 · 5 comments · Fixed by #80, #81 or #87
Labels
bug Something isn't working enhancement New feature or request help wanted Extra attention is needed

Comments

@JeanJPNM
Copy link
Collaborator

JeanJPNM commented Jun 15, 2022

I noticed that function inlining doesn't work properly with code like this:

function parseItem(n) {
  if(n === 1) return Items.copper
  if(n === 1) return Items.lead
  return null;
}

print(parseItem(1))

Produces:

set &t0:3:0 @copper
set &t0:3:0 @lead
set &t0:3:0 null
print &t0:3:0
end

While the expected output was:

set &t0:3:0 @copper
print &t0:3:0
end

The example source has repeated if comparisons to show that all of the return statements got inlined, which is
not expected and has extremely high potential to cause silent issues.

@JeanJPNM JeanJPNM added the bug Something isn't working label Jun 15, 2022
@weisrc
Copy link
Collaborator

weisrc commented Jun 15, 2022

This is no bug, it is just not implemented.

@weisrc weisrc added the enhancement New feature or request label Jun 15, 2022
@JeanJPNM JeanJPNM added the help wanted Extra attention is needed label Jun 16, 2022
@JeanJPNM JeanJPNM linked a pull request Aug 7, 2022 that will close this issue
@JeanJPNM
Copy link
Collaborator Author

The fix only works with return statements that return a value, empty return statements still have the same issue.

@weisrc
Copy link
Collaborator

weisrc commented Aug 15, 2022

This is quite troublesome, I think the best to handle this adding a new field on the function to stop when a return statement has been inlined.

@weisrc weisrc reopened this Aug 15, 2022
@weisrc
Copy link
Collaborator

weisrc commented Aug 15, 2022

I just read the code, seems like you already did what I thought was best. I will investigate further. Do you have any clue why this behaviour occurs?

@JeanJPNM
Copy link
Collaborator Author

Probably because there is no instruction to mark as an "always running" return, which reminds me that I made a mistake a while back because those jumps I called "reduntant" were really important on other circumstances, like an early return. So yeah, I'm gonna revert some of those changes and it should work again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
2 participants