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

pop local variables based on their size #57

Merged
merged 2 commits into from
Jan 28, 2025

Conversation

MaxHearnden
Copy link
Contributor

This caused a program as simple as

struct test {
  int testa;
  int testb;
};

int main() {
  struct test;
  return 0;
}

to fail.

@stikonas stikonas requested review from oriansj and stikonas October 27, 2024 16:49
Copy link
Collaborator

@stikonas stikonas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, though test checksums will need updating.

Can you update them or should I do it? It is mostly:

  • Build mescc-tools and add them to PATH (in particular we need hex2 and M1).
  • make clean test

If it doesn't pass, run

  • make Generate-test-answers
  • sha256sum test/test1000/proof > test/test1000/proof.answer
  • make clean test

cc_core.c Outdated Show resolved Hide resolved
@gtker gtker mentioned this pull request Jan 17, 2025
@oriansj
Copy link
Owner

oriansj commented Jan 19, 2025

We don't push variables on the stack based on size; so popping variables on the stack will result in an unbalanced stack.

So you'll have to make our pushes onto the stack match the variable size (rather than fixed to register size) if you want this behavior in M2-Planet

@MaxHearnden
Copy link
Contributor Author

@MaxHearnden
Copy link
Contributor Author

I've rebased (in case there are any significant code base changes) and updated the test answers

@stikonas
Copy link
Collaborator

I've rebased (in case there are any significant code base changes) and updated the test answers

Thanks, I'll test this tomorrow.

@stikonas
Copy link
Collaborator

@MaxHearnden were you actually able to compile that test program at any point.

Now I'm getting a.c:7:struct 'test' already has definition. but even if I rebase you commit on older M2-Planet, I still get different error: forbidden character in local variable name.

@stikonas stikonas merged commit a7529c4 into oriansj:master Jan 28, 2025
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.

3 participants