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

feat(codegen): add AsValue macro support for enums #286

Merged
merged 2 commits into from
Oct 25, 2020

Conversation

Wodann
Copy link
Collaborator

@Wodann Wodann commented Oct 23, 2020

Closes #275

@Wodann Wodann added the type: feat New feature or request label Oct 23, 2020
@Wodann Wodann added this to the Mun v0.3.0 milestone Oct 23, 2020
@Wodann Wodann requested a review from baszalmstra October 23, 2020 14:57
@Wodann Wodann self-assigned this Oct 23, 2020
@Wodann Wodann force-pushed the feat/enum-as-value-macro branch from 2cb5cbf to 41256ee Compare October 23, 2020 15:03
@codecov
Copy link

codecov bot commented Oct 23, 2020

Codecov Report

Merging #286 into master will decrease coverage by 0.05%.
The diff coverage is 69.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #286      +/-   ##
==========================================
- Coverage   79.39%   79.34%   -0.06%     
==========================================
  Files         220      220              
  Lines       13004    13069      +65     
==========================================
+ Hits        10324    10369      +45     
- Misses       2680     2700      +20     
Impacted Files Coverage Δ
crates/mun_codegen/src/ir/types.rs 37.50% <0.00%> (-37.50%) ⬇️
crates/mun_codegen/src/value/array_value.rs 77.77% <0.00%> (-3.62%) ⬇️
crates/mun_codegen/src/value/string.rs 21.42% <ø> (ø)
crates/mun_codegen/src/value/pointer_value.rs 64.28% <25.00%> (-15.72%) ⬇️
crates/mun_codegen/src/value/float_value.rs 30.00% <75.00%> (+30.00%) ⬆️
crates/mun_codegen/src/value/mod.rs 60.86% <80.00%> (+7.80%) ⬆️
crates/mun_codegen/src/mock.rs 82.22% <83.33%> (+0.17%) ⬆️
crates/mun_codegen/src/value/int_value.rs 66.66% <100.00%> (+20.23%) ⬆️

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 31bee34...3e66099. Read the comment docs.

@Wodann Wodann force-pushed the feat/enum-as-value-macro branch from 41256ee to f647f99 Compare October 23, 2020 15:31
Copy link
Collaborator

@baszalmstra baszalmstra left a comment

Choose a reason for hiding this comment

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

First little batch, Im gonna wait until this is rebased to get a better overview of the changes. :)

crates/mun_codegen/src/ir/types.rs Outdated Show resolved Hide resolved
crates/mun_codegen/src/ir/types.rs Outdated Show resolved Hide resolved
@Wodann
Copy link
Collaborator Author

Wodann commented Oct 24, 2020

I need to add trivial tests for as_bytes_and_ptrs as the test coverage is too low.

@Wodann Wodann force-pushed the feat/enum-as-value-macro branch 2 times, most recently from 0c8a0cf to 4cf4ab8 Compare October 25, 2020 11:47
Copy link
Collaborator

@baszalmstra baszalmstra left a comment

Choose a reason for hiding this comment

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

This is really nice and also provides a valuable start to add enums to Mun itself!

In general I think it would be good to split the generation of enums and structs in seperate functions, maybe add some more helper functions. This procedural macro function is now almost 700 lines long which IMO is way to long for a single function.

Also, again, some comments explaining what is going on would be nice. Maybe you could even add a little explaination in the docs what the problem is and how it is solved? (Either in the book or as doc comments)

crates/mun_codegen/src/value/string.rs Show resolved Hide resolved
crates/mun_codegen/src/value/string.rs Show resolved Hide resolved
crates/mun_codegen_macros/src/lib.rs Show resolved Hide resolved
crates/mun_codegen_macros/src/lib.rs Show resolved Hide resolved
crates/mun_codegen_macros/src/lib.rs Show resolved Hide resolved
crates/mun_codegen_macros/src/lib.rs Show resolved Hide resolved
@Wodann Wodann force-pushed the feat/enum-as-value-macro branch from 9a04816 to 6454e5f Compare October 25, 2020 18:36
@Wodann Wodann force-pushed the feat/enum-as-value-macro branch from 6454e5f to 3e66099 Compare October 25, 2020 20:01
@Wodann Wodann merged commit b6f6c40 into mun-lang:master Oct 25, 2020
@Wodann Wodann deleted the feat/enum-as-value-macro branch October 25, 2020 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feat New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for enums in the AsValue macro
2 participants