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

[WIP] Add better C support #135

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

staticintlucas
Copy link
Contributor

Better C support as discussed in standardese/standardese#220.

Added:

  • restrict & _Atomic qualifiers
  • register storage class
  • C's _Thread_local as an alias for C++'s thread_local
  • Support for parsing C files as C so libclang doesn't complain when these are used

Also fixed an issue that was causing my compile_commands.json to parse incorrectly. It was treating ... -std=c17 filename.c as if filename.c was the value for -std=c17 rather than splitting -std=c17 and treating filename.c as a positional argument.

This PR is still a work in progress, but I think most of the changes required for C support are there if you want to review them. I have generally kept naming as is, so cpp_standard now includes both C and C++ standards, and cpp_cv has all 4 qualifiers, not just const and volatile.

I still have some issues using Standardese with some of my C code. For example:

/// docs
typedef enum { ... } name_t;

This generates documentation for the anonymous struct rather than the typedef. An easy workaround is:

enum _name { ... };
/// docs
typedef enum _name name_t;

But in this case the docs show using name_t = _name; which is only valid in C++, rather than a typedef which is standard C.

I haven't looked into these remaining issues yet; if they originate from foonathan/cppast or whether only standardese/standardese needs some more changes.

Copy link
Collaborator

@foonathan foonathan left a comment

Choose a reason for hiding this comment

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

Looks good so far, just one nitpick.

src/cpp_type.cpp Outdated Show resolved Hide resolved
@foonathan
Copy link
Collaborator

Thanks a lot already!

I'd like to split the changes up into separate PRs, to enable a cleaner commit history later on. Can you do one that adds basic C support, one that adds register, one for atomic, and so on.

But in this case the docs show using name_t = _name; which is only valid in C++, rather than a typedef which is standard C.

Yes, cppast's code generator uses using instead of typedef as that's easier to generate.

@staticintlucas
Copy link
Contributor Author

Sorry about the silence, but I've only now had another chance to look at this. I've added the logic required to generate typedefs for C, while still using using by for C++.

I still want to find a way to make docs on typedef struct { ... } name_t; work properly, since it's a fairly common idiom in C. After that when it's ready to merge I will split it all up into separate PRs.

src/code_generator.cpp Outdated Show resolved Hide resolved
@jyapayne
Copy link

jyapayne commented Jan 7, 2023

@staticintlucas Any progress on this? It would be really cool to use your hard work :)

@staticintlucas
Copy link
Contributor Author

Sorry, got distracted with other projects and completely forgot about this. I can create some PRs for what I've already implemented in the coming days.

I had not made any progress with typedef struct { ... } type_t (which I assume also affects enum), so that is still TODO

@jyapayne
Copy link

jyapayne commented Jan 9, 2023

@foonathan is there any reason you want him to create separate PR's rather than just cleaning up the commit history in this one with a rebase? Personally, I don't see the difference.

@foonathan
Copy link
Collaborator

@foonathan is there any reason you want him to create separate PR's rather than just cleaning up the commit history in this one with a rebase? Personally, I don't see the difference.

I usually rewrite the commit messages when merging. This is easier when it's separate PRs.

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.

None yet

3 participants