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

thecl: fix remaining r/r conflicts and warnings #65

Merged
merged 13 commits into from
Nov 8, 2019
Merged

thecl: fix remaining r/r conflicts and warnings #65

merged 13 commits into from
Nov 8, 2019

Conversation

DankRank
Copy link
Member

@DankRank DankRank commented Nov 7, 2019

this fixes the two final r/r conflicts mentioned in #64

I don't know if sub call as instruction and sub call as expression are really equivalent, so someone please test it.

I also fixed all of the warnings gcc gave me, so now you can finally build thtk without it giving you three screenfuls of "const qualifier discarded"

@Priw8
Copy link
Member

Priw8 commented Nov 7, 2019

This breaks some things, since there is a significant difference between sub calls as expressions and sub calls as instructions. The issues range from throwing incorrect errors to generating push instructions when they shouldn't be generated.

void noRet() {
    nop();
}

int testRet() {
    return 20;
}

void main() {
    noRet(); // throws "sub used in expression can not have void return type: noRet"
    int a = testRet(); // this one is fine: testRet sets the $I3 variable to 20, then $I3 is pushed to stack and assigned to a, as expected
    testRet(); // this one is NOT fine, since it also pushes $I3, which is not actually used anywhere later
}

Same kind of issues also happen with inline subs. This is something I mentioned here earlier.

@DankRank
Copy link
Member Author

DankRank commented Nov 7, 2019

fixed it similarly to how i fixed instruction parameters

@DankRank DankRank merged commit fcf651a into master Nov 8, 2019
@DankRank DankRank deleted the rr-expr branch November 8, 2019 21:05
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