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

Move c_macros generation into the enums crate #676

Merged
merged 7 commits into from
Sep 29, 2021

Conversation

Luni-4
Copy link
Collaborator

@Luni-4 Luni-4 commented Sep 22, 2021

  • Move c_macros generation to the enums crate in order to avoid recreating each c_macro file whenever the rust-code-analysis library is built.
  • Replace phf_set with a const array

In this way macros for the C language are not generated during
rust-code-analysis build
@Luni-4 Luni-4 requested a review from marco-c September 22, 2021 15:27
@codecov-commenter
Copy link

codecov-commenter commented Sep 22, 2021

Codecov Report

Merging #676 (55c0ae9) into master (b9d2ab7) will increase coverage by 0.01%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #676      +/-   ##
==========================================
+ Coverage   36.96%   36.98%   +0.01%     
==========================================
  Files          47       49       +2     
  Lines        6262     6268       +6     
  Branches      934      933       -1     
==========================================
+ Hits         2315     2318       +3     
- Misses       3306     3309       +3     
  Partials      641      641              
Impacted Files Coverage Δ
src/c_langs_macros/c_specials.rs 0.00% <0.00%> (ø)
src/lib.rs 100.00% <ø> (ø)
src/preproc.rs 0.00% <0.00%> (ø)
src/c_langs_macros/c_macros.rs 100.00% <100.00%> (ø)
src/c_macro.rs 83.07% <100.00%> (ø)

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 b9d2ab7...55c0ae9. Read the comment docs.

@marco-c
Copy link
Collaborator

marco-c commented Sep 22, 2021

Can we remove the phf dependency as part of this or is it still necessary?

@Luni-4
Copy link
Collaborator Author

Luni-4 commented Sep 22, 2021

Can we remove the phf dependency as part of this or is it still necessary?

I would have done that in a following PR, but I can remove it in this PR

@marco-c
Copy link
Collaborator

marco-c commented Sep 22, 2021

I would have done that in a following PR, but I can remove it in this PR

Yes, thanks, I'd prefer that

@marco-c
Copy link
Collaborator

marco-c commented Sep 27, 2021

This looks good to me, @calixteman do you want to take a look?

@marco-c marco-c requested a review from calixteman September 27, 2021 07:36
Copy link
Collaborator

@calixteman calixteman left a comment

Choose a reason for hiding this comment

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

LGTM too. r+
Thanks for doing that.

@marco-c marco-c merged commit 473b40b into mozilla:master Sep 29, 2021
@Luni-4 Luni-4 deleted the move-enums-macros branch September 29, 2021 11:50
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.

4 participants