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

Validate constant expressions for global/offsets initialization #372

Closed
wants to merge 11 commits into from

Conversation

gumb0
Copy link
Collaborator

@gumb0 gumb0 commented Jun 4, 2020

More validation for number of instructions in const expr will be added in #377

I will squash test commits before merging.

@gumb0 gumb0 force-pushed the validate-globals-init branch from 1db5e85 to eab7cdd Compare June 4, 2020 10:16
@gumb0 gumb0 force-pushed the validate-globals-init branch 4 times, most recently from 8154174 to a7dff89 Compare June 4, 2020 14:42
@codecov
Copy link

codecov bot commented Jun 4, 2020

Codecov Report

Merging #372 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #372   +/-   ##
=======================================
  Coverage   99.32%   99.33%           
=======================================
  Files          42       42           
  Lines       12834    12902   +68     
=======================================
+ Hits        12748    12816   +68     
  Misses         86       86           

@gumb0 gumb0 force-pushed the validate-globals-init branch 2 times, most recently from ae03f96 to edb0e64 Compare June 4, 2020 18:12
@gumb0 gumb0 marked this pull request as ready for review June 4, 2020 18:16
@gumb0 gumb0 force-pushed the validate-globals-init branch from 9b915e1 to ee7a549 Compare June 8, 2020 10:24
@gumb0 gumb0 requested review from chfast and axic June 8, 2020 11:05
Copy link
Collaborator

@chfast chfast left a comment

Choose a reason for hiding this comment

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

This looks good, with two nits:

  1. Once you move a test to validation_test.cpp in the same commit where functional change is. Other time this is done in two separate commits.
  2. I'd rather use global.get, not global_get in comments and error messages.

@gumb0 gumb0 force-pushed the validate-globals-init branch from 704e2cf to 8b2a96b Compare June 8, 2020 15:03
@gumb0
Copy link
Collaborator Author

gumb0 commented Jun 8, 2020

  1. Once you move a test to validation_test.cpp in the same commit where functional change is. Other time this is done in two separate commits.

I made separate commits with the moves, now there're more test commits.

  1. I'd rather use global.get, not global_get in comments and error messages.

Changed.

lib/fizzy/parser.cpp Outdated Show resolved Hide resolved
lib/fizzy/parser.cpp Outdated Show resolved Hide resolved
@axic
Copy link
Member

axic commented Jun 8, 2020

Will review this later today.

@gumb0 gumb0 force-pushed the validate-globals-init branch 2 times, most recently from 346cf4f to 6e745d0 Compare June 9, 2020 09:43
@gumb0
Copy link
Collaborator Author

gumb0 commented Jun 9, 2020

Rebased.

@gumb0
Copy link
Collaborator Author

gumb0 commented Jun 24, 2020

Rebased.

@gumb0 gumb0 force-pushed the validate-globals-init branch from 56df4e8 to eda3aa9 Compare June 25, 2020 17:43
@gumb0 gumb0 force-pushed the validate-globals-init branch from eda3aa9 to 7ab30da Compare June 25, 2020 17:45
@gumb0 gumb0 force-pushed the validate-globals-init branch from 7ab30da to 6f1e9bb Compare June 25, 2020 17:49
@gumb0
Copy link
Collaborator Author

gumb0 commented Jun 25, 2020

Reordered commits.

@axic
Copy link
Member

axic commented Jun 26, 2020

Replaced by individual PRs.

@axic axic closed this Jun 26, 2020
@axic axic deleted the validate-globals-init branch June 26, 2020 15:41
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