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

Added basic support for array initialization. Fixes #250 #256

Merged
merged 2 commits into from
Oct 23, 2017
Merged

Added basic support for array initialization. Fixes #250 #256

merged 2 commits into from
Oct 23, 2017

Conversation

yulvil
Copy link
Contributor

@yulvil yulvil commented Oct 21, 2017

This change is Reviewable

@yulvil
Copy link
Contributor Author

yulvil commented Oct 21, 2017

@elliotchance I can remove the commented test cases if you want. They need to be fixed at a later time.

@codecov
Copy link

codecov bot commented Oct 21, 2017

Codecov Report

Merging #256 into master will increase coverage by 0.35%.
The diff coverage is 91.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #256      +/-   ##
==========================================
+ Coverage      81%   81.35%   +0.35%     
==========================================
  Files         134      134              
  Lines        5448     5471      +23     
==========================================
+ Hits         4413     4451      +38     
+ Misses        841      827      -14     
+ Partials      194      193       -1
Impacted Files Coverage Δ
transpiler/variables.go 83.22% <91.66%> (+1.4%) ⬆️
ast/position.go 82.33% <0%> (+0.75%) ⬆️
transpiler/operators.go 85.11% <0%> (+1.19%) ⬆️
types/cast.go 59.15% <0%> (+1.4%) ⬆️
transpiler/transpiler.go 91.34% <0%> (+1.44%) ⬆️
types/resolve.go 83.87% <0%> (+3.22%) ⬆️
ast/init_list_expr.go 88.88% <0%> (+33.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30ddad5...26e03fb. Read the comment docs.

@elliotchance elliotchance changed the title Added basic support for array initialization #250 Added basic support for array initialization. Fixes #250 Oct 23, 2017
@elliotchance
Copy link
Owner

Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, 3 unresolved discussions.


tests/array.c, line 44 at r1 (raw file):

// void test_intarr_init2()
// {
//     int a[4] = {10, 20};

If I saw someone do that in code I would expect that it was a mistake. What is the expected behaviour of this in C? Please open an issue with these examples so they can be removed (rather than commented out).


tests/array.c, line 75 at r1 (raw file):

}

// currently transpiles to: var arr []string

In previous versions I treated char * as a Go string until I realised that wasn't going to work. It looks like there is still one case left over where it is resolving to a string instead of []byte.


tests/array.c, line 95 at r1 (raw file):

// currently transpiles to: var a []interface {} = []interface{}{nil, nil}
// void test_structarr()

Based on my previous comment. It is better to put these examples in a Github issue so they can be found and fixed rather than keeping them as code that is inactive.


Comments from Reviewable

@yulvil
Copy link
Contributor Author

yulvil commented Oct 23, 2017

Review status: 1 of 2 files reviewed at latest revision, 3 unresolved discussions.


tests/array.c, line 44 at r1 (raw file):

Previously, elliotchance (Elliot Chance) wrote…

If I saw someone do that in code I would expect that it was a mistake. What is the expected behaviour of this in C? Please open an issue with these examples so they can be removed (rather than commented out).

Done.


tests/array.c, line 75 at r1 (raw file):

Previously, elliotchance (Elliot Chance) wrote…

In previous versions I treated char * as a Go string until I realised that wasn't going to work. It looks like there is still one case left over where it is resolving to a string instead of []byte.

Done.


tests/array.c, line 95 at r1 (raw file):

Previously, elliotchance (Elliot Chance) wrote…

Based on my previous comment. It is better to put these examples in a Github issue so they can be found and fixed rather than keeping them as code that is inactive.

Done.


Comments from Reviewable

@elliotchance
Copy link
Owner

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@elliotchance elliotchance merged commit f43c66d into elliotchance:master Oct 23, 2017
@yulvil yulvil deleted the arrayinit branch October 23, 2017 06:31
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.

2 participants