Skip to content

NFC: Move #includes out of extern "C" block #58

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

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

egorzhdan
Copy link

This fixes a clang compiler error when using these headers in C++ language mode:

libcmark_gfm/include/registry.h:8:1: error: import of C++ module 'libcmark_gfm' appears within extern "C" language linkage specification
#include "cmark-gfm.h"

rdar://109730567

Copy link

@QuietMisdreavus QuietMisdreavus left a comment

Choose a reason for hiding this comment

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

Nice catch, thanks!

@QuietMisdreavus
Copy link

Is there a way we can add this to the CMake test suite so i can catch this in the future when integrating upstream changes or checking new work?

@gabryon99
Copy link

gabryon99 commented Sep 5, 2024

Hello! Will this PR be merged in the codebase? We are trying to build a Swift dynamic library, to be used in C++, but the library depends on swift-cmark.

Thank you in advance and sorry for the disturb!

This fixes a clang compiler error when using these headers in C++ language mode:
```
libcmark_gfm/include/registry.h:8:1: error: import of C++ module 'libcmark_gfm' appears within extern "C" language linkage specification
#include "cmark-gfm.h"
```

rdar://109730567
@egorzhdan egorzhdan force-pushed the egorzhdan/include-inside-extern-c branch from 2403ee8 to 8a43e0a Compare September 10, 2024 15:57
@egorzhdan
Copy link
Author

Let me see if we could enable the Clang module verifier for this project.

@egorzhdan
Copy link
Author

Unfortunately it doesn't look like we can use Clang's module verifier for this project, since the module verifier currently only supports frameworks and doesn't support static libraries.

@QuietMisdreavus do you think we could land this change as is, or would you like to see an alternative way to test this?

@QuietMisdreavus
Copy link

We can just land it as-is and deal with issues whenever they come up. Thanks for the PR!

@QuietMisdreavus QuietMisdreavus merged commit b022b08 into gfm Sep 11, 2024
@QuietMisdreavus QuietMisdreavus deleted the egorzhdan/include-inside-extern-c branch September 11, 2024 20:56
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.

3 participants