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

Add C support #38

Merged
merged 14 commits into from
May 8, 2023
Merged

Add C support #38

merged 14 commits into from
May 8, 2023

Conversation

believeinlain
Copy link
Contributor

PR that adds rudimentary support for C includes. Since C doesn't have a concept of "modules" we simply import items (which could be a function or variable) from files specified by path, with a boolean to distinguish system headers (such as #include <stdio.h>) from local headers (such as #include "foo/bar.h").

Note that it considers <stdio.h> and "stdio.h" to be different include files. We can let the C compiler sort out correctness - naming local headers with the same names as system headers that one is also using is incredibly bad practice.

Added working example and doc test.

Addresses Issue #22

@udoprog
Copy link
Owner

udoprog commented Apr 28, 2023

Looks great!

One thing I'm considering is whether the system header parameter might instead be better served by splitting it up into multiple function calls. But I'll have you decide since I don't use C codegen :).

@believeinlain
Copy link
Contributor Author

Ah, I didn't think of that! I think it would improve readability to have include_header and include_system_header rather than passing a boolean. I'll rework it.

@udoprog
Copy link
Owner

udoprog commented May 3, 2023

Are things going OK? Would you like me to merge as-is?

@believeinlain
Copy link
Contributor Author

Sorry, I kinda got caught up in other stuff. I split include into include and include_header and updated the related docs and tests. It should be good to merge now.

src/lang/c.rs Outdated Show resolved Hide resolved
src/lang/c.rs Outdated Show resolved Hide resolved
@believeinlain
Copy link
Contributor Author

Ah, good catch. I've removed the with-package config option as it's not relevant to C.

@udoprog udoprog merged commit f9b4f73 into udoprog:main May 8, 2023
@udoprog
Copy link
Owner

udoprog commented May 8, 2023

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants