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.

Sign up for GitHub

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

Jump to bottom

Experimental codegen for types of different stack sizes #14620

Closed
wants to merge 1 commit into from
Closed

Experimental codegen for types of different stack sizes #14620

wants to merge 1 commit into from

Conversation

r0qs
Copy link
Member

@r0qs r0qs commented Oct 16, 2023

Fixes #14569

@r0qs r0qs self-assigned this Oct 16, 2023
@cameel cameel force-pushed the newAnalysis branch 2 times, most recently from e86a606 to bde0288 Compare October 17, 2023 11:47
@r0qs r0qs force-pushed the experimental-codegen branch from 056c521 to 374f1d6 Compare October 18, 2023 10:57
@ekpyron ekpyron mentioned this pull request Oct 30, 2023
Merged
@cameel cameel force-pushed the experimental-codegen branch from 374f1d6 to 3fba354 Compare October 31, 2023 11:50
@cameel
Copy link
Member

cameel commented Oct 31, 2023

I just rebased this on top of the current state of newAnalysis.

@cameel cameel mentioned this pull request Oct 31, 2023
Merged
@cameel cameel force-pushed the experimental-codegen branch from 3fba354 to 982c318 Compare November 1, 2023 10:26
@r0qs r0qs force-pushed the experimental-codegen branch 3 times, most recently from 628404b to 0977921 Compare November 7, 2023 16:20
@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Nov 30, 2023
@ethereum ethereum deleted a comment from github-actions bot Nov 30, 2023
@r0qs r0qs removed the stale The issue/PR was marked as stale because it has been open for too long. label Nov 30, 2023
@r0qs r0qs force-pushed the experimental-codegen branch from 0156dd7 to 62cb690 Compare December 11, 2023 20:00
@r0qs r0qs force-pushed the newAnalysis branch 2 times, most recently from 8d311b9 to 194b114 Compare December 18, 2023 15:55
@r0qs r0qs force-pushed the experimental-codegen branch from 62cb690 to b606a19 Compare December 18, 2023 16:38
Base automatically changed from newAnalysis to develop December 18, 2023 20:15
@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Jan 2, 2024
@r0qs r0qs force-pushed the experimental-codegen branch 4 times, most recently from 7a88351 to 9b48bb6 Compare February 6, 2024 12:48
ekpyron
ekpyron previously approved these changes Feb 6, 2024
View reviewed changes
Copy link
Member

@ekpyron ekpyron left a comment

Choose a reason for hiding this comment

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

https://github.com/ethereum/solidity/pull/14620/files#r1479729240 should still switch back to an anonymous namespace at least - but apart from that I'd say we could merge this

@ekpyron
Copy link
Member

ekpyron commented Feb 6, 2024

As for the syntax tests failing on test/libsolidity/syntaxTests/experimental/builtin/builtin_type_definition.sol: that's not an issue of this PR, but of the test itself.
It's generally invalid to declare a variable of type void as done in let v:void there, so we should change the test.
We could either remove that variable from the test or change it to a function - let v:void->void should be fine.

In a separate PR we can then at some point extend analysis to produce proper errors on declaring variables of types without stack representation, but that's out of scope for this PR.

@r0qs r0qs force-pushed the experimental-codegen branch 2 times, most recently from 22cb3c7 to e53dc56 Compare February 6, 2024 13:30
@r0qs r0qs marked this pull request as ready for review February 6, 2024 13:37
@r0qs
Copy link
Member Author

r0qs commented Feb 6, 2024

As for the syntax tests failing on test/libsolidity/syntaxTests/experimental/builtin/builtin_type_definition.sol: that's not an issue of this PR, but of the test itself. It's generally invalid to declare a variable of type void as done in let v:void there, so we should change the test. We could either remove that variable from the test or change it to a function - let v:void->void should be fine.

In a separate PR we can then at some point extend analysis to produce proper errors on declaring variables of types without stack representation, but that's out of scope for this PR.

Also, now I'm asserting instead of throwing a error as suggested here #14620 (comment) so tests that test will also fail because of the integer declaration: let i: integer; or should this also be a function?

@r0qs r0qs force-pushed the experimental-codegen branch 2 times, most recently from 2979e19 to bb285c3 Compare February 22, 2024 12:48
@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Mar 8, 2024
@ethereum ethereum deleted a comment from github-actions bot Mar 11, 2024
@r0qs r0qs removed the stale The issue/PR was marked as stale because it has been open for too long. label Mar 11, 2024
@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Mar 25, 2024
@ethereum ethereum deleted a comment from github-actions bot Mar 25, 2024
@r0qs r0qs removed the stale The issue/PR was marked as stale because it has been open for too long. label Mar 25, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 9, 2024

This pull request is stale because it has been open for 14 days with no activity.
It will be closed in 7 days unless the stale label is removed.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Apr 9, 2024
@r0qs r0qs removed the stale The issue/PR was marked as stale because it has been open for too long. label Apr 15, 2024
@r0qs r0qs force-pushed the experimental-codegen branch from bb285c3 to a89b4b6 Compare April 15, 2024 09:04
@github-actions GitHub Actions
Copy link

This pull request is stale because it has been open for 14 days with no activity.
It will be closed in 7 days unless the stale label is removed.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Apr 29, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented May 7, 2024

This pull request was closed due to a lack of activity for 7 days after it was stale.

@github-actions github-actions bot closed this May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ekpyron ekpyron ekpyron left review comments

Assignees

@r0qs r0qs

Labels
closed-due-inactivity experimental stale The issue/PR was marked as stale because it has been open for too long.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Experimental Solidity codegen for types of different stack sizes.
3 participants
@r0qs @cameel @ekpyron