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: labels error handling #1877

Merged
merged 4 commits into from
Apr 8, 2024
Merged

fix: labels error handling #1877

merged 4 commits into from
Apr 8, 2024

Conversation

petar-dambovaliev
Copy link
Contributor

@petar-dambovaliev petar-dambovaliev commented Apr 2, 2024

fixes this

Added error handling on TRANS_LEAVE during preprocessing, for break and continue statements that have labels.

@github-actions github-actions bot added the 📦 🤖 gnovm Issues or PRs gnovm related label Apr 2, 2024
Copy link

codecov bot commented Apr 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 45.06%. Comparing base (d54ca62) to head (2fd4d2d).
Report is 6 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1877      +/-   ##
==========================================
- Coverage   47.54%   45.06%   -2.48%     
==========================================
  Files         388      464      +76     
  Lines       61242    68027    +6785     
==========================================
+ Hits        29117    30658    +1541     
- Misses      29686    34800    +5114     
- Partials     2439     2569     +130     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@petar-dambovaliev petar-dambovaliev marked this pull request as ready for review April 3, 2024 09:15
@petar-dambovaliev petar-dambovaliev requested review from zivkovicmilos and removed request for thehowl April 3, 2024 09:15
Copy link
Member

@zivkovicmilos zivkovicmilos 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 💯

I've left some nitties, otherwise we are good to go

Thank you for the fix 🙏

gnovm/pkg/gnolang/preprocess.go Outdated Show resolved Hide resolved
gnovm/pkg/gnolang/preprocess.go Outdated Show resolved Hide resolved
gnovm/pkg/gnolang/preprocess.go Outdated Show resolved Hide resolved
gnovm/pkg/gnolang/preprocess.go Outdated Show resolved Hide resolved
@petar-dambovaliev petar-dambovaliev merged commit df0a53c into master Apr 8, 2024
190 checks passed
Comment on lines +26 to +39
code: `
package test
func main(){}
func invalidLabel() {
FirstLoop:
for i := 0; i < 10; i++ {
}
for i := 0; i < 10; i++ {
break FirstLoop
}
}
`,
output: `cannot find branch label "FirstLoop"`,
},
Copy link
Member

Choose a reason for hiding this comment

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

For next time, please place tests like these in gnovm/tests/files. Goes for all tests which simply run some code and try to test the output or any panic, like these ones

If you're testing specific functions or doing unit tests, then they should go in gnolang, but if they are tests which just run some code against the GnoVM then they should go there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

bug: no error handling for invalid label
3 participants