-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[libclang][Cygwin] Use __declspec(dllexport) for libclang on Cygwin #147122
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
Conversation
This is needed for Cygwin build without LLVM_LINK_LLVM_DYLIB, otherwise causes a linker error 'export ordinal too large'.
@llvm/pr-subscribers-clang Author: Tomohiro Kashiwada (kikairoya) ChangesThis is needed for Cygwin build without Full diff: https://github.com/llvm/llvm-project/pull/147122.diff 1 Files Affected:
diff --git a/clang/include/clang-c/Platform.h b/clang/include/clang-c/Platform.h
index 67c1fff8ff783..8d341ddd6f8eb 100644
--- a/clang/include/clang-c/Platform.h
+++ b/clang/include/clang-c/Platform.h
@@ -22,7 +22,7 @@ LLVM_CLANG_C_EXTERN_C_BEGIN
#ifndef CINDEX_NO_EXPORTS
#define CINDEX_EXPORTS
#endif
-#ifdef _WIN32
+#if defined(_WIN32) || defined(__CYGWIN__)
#ifdef CINDEX_EXPORTS
#ifdef _CINDEX_LIB_
#define CINDEX_LINKAGE __declspec(dllexport)
|
Before merge this, requires #147108 |
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions h -- clang/include/clang-c/Platform.h View the diff from clang-format here.diff --git a/clang/include/clang-c/Platform.h b/clang/include/clang-c/Platform.h
index 8d341ddd6..1c28ccf81 100644
--- a/clang/include/clang-c/Platform.h
+++ b/clang/include/clang-c/Platform.h
@@ -23,13 +23,13 @@ LLVM_CLANG_C_EXTERN_C_BEGIN
#define CINDEX_EXPORTS
#endif
#if defined(_WIN32) || defined(__CYGWIN__)
- #ifdef CINDEX_EXPORTS
- #ifdef _CINDEX_LIB_
- #define CINDEX_LINKAGE __declspec(dllexport)
- #else
- #define CINDEX_LINKAGE __declspec(dllimport)
- #endif
- #endif
+#ifdef CINDEX_EXPORTS
+#ifdef _CINDEX_LIB_
+#define CINDEX_LINKAGE __declspec(dllexport)
+#else
+#define CINDEX_LINKAGE __declspec(dllimport)
+#endif
+#endif
#elif defined(CINDEX_EXPORTS) && defined(__GNUC__)
#define CINDEX_LINKAGE __attribute__((visibility("default")))
#endif
|
diff --git a/clang/include/clang-c/Platform.h b/clang/include/clang-c/Platform.h
index 8d341ddd6f8e..1c28ccf81981 100644
--- a/clang/include/clang-c/Platform.h
+++ b/clang/include/clang-c/Platform.h
@@ -23,13 +23,13 @@ LLVM_CLANG_C_EXTERN_C_BEGIN
#define CINDEX_EXPORTS
#endif
#if defined(_WIN32) || defined(__CYGWIN__)
- #ifdef CINDEX_EXPORTS
- #ifdef _CINDEX_LIB_
- #define CINDEX_LINKAGE __declspec(dllexport)
- #else
- #define CINDEX_LINKAGE __declspec(dllimport)
- #endif
- #endif
+#ifdef CINDEX_EXPORTS
+#ifdef _CINDEX_LIB_
+#define CINDEX_LINKAGE __declspec(dllexport)
+#else
+#define CINDEX_LINKAGE __declspec(dllimport)
+#endif
+#endif
#elif defined(CINDEX_EXPORTS) && defined(__GNUC__)
#define CINDEX_LINKAGE __attribute__((visibility("default")))
#endif |
Can you elaborate on why this is needed - what happens without it - wouldn't that issue be happening already now in regular win32 builds so far? |
The linker reports undefined reference to clang_install_aborting_llvm_fatal_error_handler, and cannot run testsuite.
For regular Win32 targets, this unittest is disabled. llvm-project/clang/unittests/CMakeLists.txt Lines 98 to 102 in 4406a45
|
Thanks, that explains why this hasn't been noticed before! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks.
But we should wait for #147108 to land first before merging this.
This is needed for Cygwin build without
-DLLVM_LINK_LLVM_DYLIB=ON
, otherwise causes a linker error 'export ordinal too large'.