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

fix: missing length check in value declaration #2206

Merged
merged 19 commits into from
Jul 30, 2024
Merged

fix: missing length check in value declaration #2206

merged 19 commits into from
Jul 30, 2024

Conversation

omarsy
Copy link
Contributor

@omarsy omarsy commented May 26, 2024

Closes #2137

Contributors' checklist...
  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests
  • Added new benchmarks to generated graphs, if any. More info here.

@github-actions github-actions bot added the 📦 🤖 gnovm Issues or PRs gnovm related label May 26, 2024
@codecov Codecov
Copy link

codecov bot commented May 26, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 96.36364% with 2 lines in your changes missing coverage. Please review.

Project coverage is 55.02%. Comparing base (0e3c050) to head (10c7aaa).

Files Patch % Lines
gnovm/pkg/gnolang/type_check.go 95.23% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2206      +/-   ##
==========================================
+ Coverage   54.99%   55.02%   +0.03%     
==========================================
  Files         595      595              
  Lines       79775    79826      +51     
==========================================
+ Hits        43872    43927      +55     
- Misses      32581    32582       +1     
+ Partials     3322     3317       -5
Flag Coverage Δ
contribs/gnodev 25.65% <ø> (-0.35%) ⬇️
contribs/gnofaucet 15.31% <ø> (+0.85%) ⬆️
contribs/gnokeykc 0.00% <ø> (ø)
contribs/gnomd 0.00% <ø> (ø)
gno.land 64.15% <ø> (ø)
gnovm 60.30% <96.36%> (+0.10%) ⬆️
misc/autocounterd 0.00% <ø> (ø)
misc/genproto 0.00% <ø> (ø)
misc/genstd 80.54% <ø> (ø)
misc/goscan 0.00% <ø> (ø)
misc/logos 17.38% <ø> (ø)
misc/loop 0.00% <ø> (ø)
tm2 54.45% <ø> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@omarsy omarsy marked this pull request as draft May 26, 2024 21:55
@thehowl
Copy link
Member

thehowl commented May 27, 2024

cc @ltzmaxwell, I don't think this is what you wanted from the issue

@omarsy omarsy mentioned this pull request May 28, 2024
Merged
7 tasks
omarsy
omarsy commented Jun 2, 2024
View reviewed changes
gnovm/tests/files/var18.gno Outdated Show resolved Hide resolved
@omarsy omarsy marked this pull request as ready for review June 2, 2024 15:21
@github-actions github-actions bot added 🧾 package/realm Tag used for new Realms or Packages. 📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related labels Jun 3, 2024
omarsy and others added 5 commits June 13, 2024 19:17
@omarsy @zivkovicmilos
Update gnovm/pkg/gnolang/preprocess.go
8732fe9 
Co-authored-by: Miloš Živković <milos@zmilos.com>
@omarsy @zivkovicmilos
refactor: assign on if
505ed37 
Co-authored-by: Miloš Živković <milos@zmilos.com>
@omarsy @zivkovicmilos
feat: improve var
27a9b36 
Co-authored-by: Miloš Živković <milos@zmilos.com>
@omarsy
Merge branch 'master' into fix/2137
0524299
@omarsy
feat: add new test case
d8236d2
@leohhhn leohhhn removed their request for review June 13, 2024 18:27
@petar-dambovaliev
Copy link
Contributor

@omarsy can you resolve the comments, if they are done so we can move forward with this PR?

@n0izn0iz
Copy link
Contributor

@petar-dambovaliev

from CONTRIBUTING.md:

The PR reviewer is in charge of resolving each conversation (hitting the Resolve Conversation button), after the PR creator implements or discusses the requested changes

@ltzmaxwell
Copy link
Contributor

LGTM

mvertes
mvertes approved these changes Jul 23, 2024
View reviewed changes
Copy link
Contributor

@mvertes mvertes 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 to me.

@omarsy omarsy requested a review from thehowl July 25, 2024 18:25
@thehowl thehowl linked an issue Jul 30, 2024 that may be closed by this pull request
Incorrect Variable Assignment #2637
Closed
thehowl
thehowl approved these changes Jul 30, 2024
View reviewed changes
gnovm/pkg/gnolang/type_check.go Show resolved Hide resolved
@thehowl thehowl merged commit ee83eaf into gnolang:master Jul 30, 2024
115 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ltzmaxwell ltzmaxwell ltzmaxwell approved these changes

@thehowl thehowl thehowl approved these changes

@mvertes mvertes mvertes approved these changes

@zivkovicmilos zivkovicmilos zivkovicmilos approved these changes

@petar-dambovaliev petar-dambovaliev petar-dambovaliev approved these changes

@moul moul Awaiting requested review from moul moul is a code owner

@piux2 piux2 Awaiting requested review from piux2 piux2 is a code owner

@deelawn deelawn Awaiting requested review from deelawn deelawn is a code owner

@gfanton gfanton Awaiting requested review from gfanton

@jaekwon jaekwon Awaiting requested review from jaekwon jaekwon is a code owner

Assignees

@omarsy omarsy

Labels
📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related 📦 🤖 gnovm Issues or PRs gnovm related 🧾 package/realm Tag used for new Realms or Packages.
Projects
💪 Bounties & Worx
Status: Done
🧙‍♂️gno.land core team
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Incorrect Variable Assignment missing length check in value declaration
8 participants
@omarsy @thehowl @ltzmaxwell @petar-dambovaliev @n0izn0iz @jaekwon @mvertes @zivkovicmilos