[clang] [libclang] Add missing dllexport annotation (PR #147108)
https://github.com/mstorsjo closed https://github.com/llvm/llvm-project/pull/147108 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libclang] Add missing dllexport annotation (PR #147108)
kikairoya wrote: > Yes, it's not mandatory but seems to be recommended. I will submit a new PR > later. https://github.com/llvm/llvm-project/pull/147278 https://github.com/llvm/llvm-project/pull/147108 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libclang] Add missing dllexport annotation (PR #147108)
kikairoya wrote: > LGTM, but can you amend the PR description with the extra info you gathered, > about why this hasn't been an issue in other existing configurations? Thanks. I've update the description. > > It seems `AND NOT CYGWIN` should be added here. > > That sounds like a good addition too, even if we're doing this as well? Yes, it's not mandatory but seems to be recommended. I will submit a new PR later. https://github.com/llvm/llvm-project/pull/147108 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libclang] Add missing dllexport annotation (PR #147108)
https://github.com/kikairoya edited https://github.com/llvm/llvm-project/pull/147108 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libclang] Add missing dllexport annotation (PR #147108)
https://github.com/mstorsjo approved this pull request. LGTM, but can you amend the PR description with the extra info you gathered, about why this hasn't been an issue in other existing configurations? https://github.com/llvm/llvm-project/pull/147108 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libclang] Add missing dllexport annotation (PR #147108)
mstorsjo wrote: > > > Still it may be good to figure out why this hasn't been an issue so far, > > > for whoever otherwise were using these dllexport annotations. > > > > > > I suspect it relates to module definition file (*.def). On MinGW, the > > symbol is properly exported without annotation. > > Regular win32 targets use `libclang.def` to choose symbols to be exported > (generated from `libclang-generic.exports` through `LLVM_EXPORT_SYMBOL_FILE`) > but it's disabled on most of Unix platforms. > > https://github.com/llvm/llvm-project/blob/53359252688692f2b0e25f529335848db94cc166/clang/tools/libclang/CMakeLists.txt#L95-L98 > > It seems `AND NOT CYGWIN` should be added here. That sounds like a good addition too, even if we're doing this as well? > Even if exported symbols are controlled by `libclang.def`, annotating > `__declspec(dllimport)` correctly is still preferred. Yeah, making that complete sounds good to me. https://github.com/llvm/llvm-project/pull/147108 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libclang] Add missing dllexport annotation (PR #147108)
kikairoya wrote: https://github.com/llvm/llvm-project/blob/53359252688692f2b0e25f529335848db94cc166/clang/tools/libclang/CMakeLists.txt#L88-L93 MSVC doesn't use `libclang.def` and doesn't export `clang_install_aborting_llvm_fatal_error_handler`. ``` $ llvm-readobj --coff-exports '/c/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/Llvm/bin/libclang.dll' | grep clang_install || echo not found not found ``` https://github.com/llvm/llvm-project/pull/147108 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libclang] Add missing dllexport annotation (PR #147108)
kikairoya wrote: > > Still it may be good to figure out why this hasn't been an issue so far, > > for whoever otherwise were using these dllexport annotations. > > I suspect it relates to module definition file (*.def). On MinGW, the symbol > is properly exported without annotation. Regular win32 targets use `libclang.def` to choose symbols to be exported (generated from `libclang-generic.exports` through `LLVM_EXPORT_SYMBOL_FILE`) but it's disabled on most of Unix platforms. https://github.com/llvm/llvm-project/blob/53359252688692f2b0e25f529335848db94cc166/clang/tools/libclang/CMakeLists.txt#L95-L98 It seems `AND NOT CYGWIN` should be added here. Even if exported symbols are controlled by `libclang.def`, annotating `__declspec(dllimport)` correctly is still preferred. https://github.com/llvm/llvm-project/pull/147108 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libclang] Add missing dllexport annotation (PR #147108)
kikairoya wrote: > > AFAIK the dllexport annotations have been added using some automated tools, > > so it may be good to find the person who added the other annotations, so it > > can be looked into why this was missing here, if the annotations otherwise > > were seemingly complete enough. > > Sorry, not sure if this applies here though, I guess the clang-c interface > has been available as DLL with dllexports even before, while the C++ > interfaces is what is getting dllexport attributes added. According to git-blame, CINDEX_LINKAGE has been in use for over 16 years. https://github.com/llvm/llvm-project/blame/a124a46748357b9e654adce7a50318a5f0648e48/clang/include/clang-c/Index.h > Still it may be good to figure out why this hasn't been an issue so far, for > whoever otherwise were using these dllexport annotations. I suspect it relates to module definition file (*.def). On MinGW, the symbol is properly exported without annotation. ``` $ llvm-readobj --coff-exports /ucrt64/bin/libclang.dll | grep clang_install Name: clang_install_aborting_llvm_fatal_error_handler ``` I'll try dig it. https://github.com/llvm/llvm-project/pull/147108 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libclang] Add missing dllexport annotation (PR #147108)
mstorsjo wrote: > AFAIK the dllexport annotations have been added using some automated tools, > so it may be good to find the person who added the other annotations, so it > can be looked into why this was missing here, if the annotations otherwise > were seemingly complete enough. Sorry, not sure if this applies here though, I guess the clang-c interface has been available as DLL with dllexports even before, while the C++ interfaces is what is getting dllexport attributes added. https://github.com/llvm/llvm-project/pull/147108 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libclang] Add missing dllexport annotation (PR #147108)
mstorsjo wrote: AFAIK the dllexport annotations have been added using some automated tools, so it may be good to find the person who added the other annotations, so it can be looked into why this was missing here, if the annotations otherwise were seemingly complete enough. https://github.com/llvm/llvm-project/pull/147108 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libclang] Add missing dllexport annotation (PR #147108)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Tomohiro Kashiwada (kikairoya) Changes All other declarations of clang-c already have CINDEX_LINKAGE. --- Full diff: https://github.com/llvm/llvm-project/pull/147108.diff 1 Files Affected: - (modified) clang/include/clang-c/FatalErrorHandler.h (+3-2) ``diff diff --git a/clang/include/clang-c/FatalErrorHandler.h b/clang/include/clang-c/FatalErrorHandler.h index 22f34fa815ccf..4f18980dea240 100644 --- a/clang/include/clang-c/FatalErrorHandler.h +++ b/clang/include/clang-c/FatalErrorHandler.h @@ -11,6 +11,7 @@ #define LLVM_CLANG_C_FATAL_ERROR_HANDLER_H #include "clang-c/ExternC.h" +#include "clang-c/Platform.h" LLVM_CLANG_C_EXTERN_C_BEGIN @@ -18,14 +19,14 @@ LLVM_CLANG_C_EXTERN_C_BEGIN * Installs error handler that prints error message to stderr and calls abort(). * Replaces currently installed error handler (if any). */ -void clang_install_aborting_llvm_fatal_error_handler(void); +CINDEX_LINKAGE void clang_install_aborting_llvm_fatal_error_handler(void); /** * Removes currently installed error handler (if any). * If no error handler is intalled, the default strategy is to print error * message to stderr and call exit(1). */ -void clang_uninstall_llvm_fatal_error_handler(void); +CINDEX_LINKAGE void clang_uninstall_llvm_fatal_error_handler(void); LLVM_CLANG_C_EXTERN_C_END `` https://github.com/llvm/llvm-project/pull/147108 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libclang] Add missing dllexport annotation (PR #147108)
https://github.com/kikairoya created https://github.com/llvm/llvm-project/pull/147108 All other declarations of clang-c already have CINDEX_LINKAGE. >From 53359252688692f2b0e25f529335848db94cc166 Mon Sep 17 00:00:00 2001 From: kikairoya Date: Sat, 28 Jun 2025 11:50:14 +0900 Subject: [PATCH] [libclang] Add missing dllexport annotation All other declarations of clang-c already have CINDEX_LINKAGE. --- clang/include/clang-c/FatalErrorHandler.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/include/clang-c/FatalErrorHandler.h b/clang/include/clang-c/FatalErrorHandler.h index 22f34fa815ccf..4f18980dea240 100644 --- a/clang/include/clang-c/FatalErrorHandler.h +++ b/clang/include/clang-c/FatalErrorHandler.h @@ -11,6 +11,7 @@ #define LLVM_CLANG_C_FATAL_ERROR_HANDLER_H #include "clang-c/ExternC.h" +#include "clang-c/Platform.h" LLVM_CLANG_C_EXTERN_C_BEGIN @@ -18,14 +19,14 @@ LLVM_CLANG_C_EXTERN_C_BEGIN * Installs error handler that prints error message to stderr and calls abort(). * Replaces currently installed error handler (if any). */ -void clang_install_aborting_llvm_fatal_error_handler(void); +CINDEX_LINKAGE void clang_install_aborting_llvm_fatal_error_handler(void); /** * Removes currently installed error handler (if any). * If no error handler is intalled, the default strategy is to print error * message to stderr and call exit(1). */ -void clang_uninstall_llvm_fatal_error_handler(void); +CINDEX_LINKAGE void clang_uninstall_llvm_fatal_error_handler(void); LLVM_CLANG_C_EXTERN_C_END ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
