[clang] [Clang] Fix visibility macros for c-index-test (PR #171054)
kkent030315 wrote: > > Perhaps some of `if (MSVC)` blocks should be changed to `(WIN32 AND NOT > > MINGW)`. > > `(MSVC OR CMAKE_CXX_SIMULATE_ID STREQUAL "MSVC")` might be better. > [gitlab.kitware.com/cmake/cmake/-/issues/19724](https://gitlab.kitware.com/cmake/cmake/-/issues/19724) I don't think this is suitable for this, let me do some tests. (I did the similar before submitting this PR, didnt worked iirc) https://github.com/llvm/llvm-project/pull/171054 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix visibility macros for c-index-test (PR #171054)
kkent030315 wrote: @kikairoya Thank you for the detailed info! I appreciate. > I think the problem is that libclang.dll is exporting C++ interfaces; that > looks unintended. Yes, it is. In the case we're not modifying dllimport attributes in `Compiler.h` what would you think is the best/better solution to fix `c-index-test`? https://github.com/llvm/llvm-project/pull/171054 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix visibility macros for c-index-test (PR #171054)
kikairoya wrote: > Perhaps some of `if (MSVC)` blocks should be changed to `(WIN32 AND NOT > MINGW)`. `(MSVC OR CMAKE_CXX_SIMULATE_ID STREQUAL "MSVC")` might be better. https://gitlab.kitware.com/cmake/cmake/-/issues/19724 https://github.com/llvm/llvm-project/pull/171054 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix visibility macros for c-index-test (PR #171054)
kikairoya wrote: > How so? Can you be more specific how this would break dylib builds, and what > exactly is in the progress? > > As mentioned in the comment dllimport attribute is not strictly required, > it's safe to remove it. For code symbols, the attribute isn't strictly required, just suboptimal; lacking of `dllimport` causes additional jumps. However, for data symbols, it's mandatory (except for MinGW auto-import feature). See also: https://github.com/llvm/llvm-project/issues/149639#issuecomment-3114257062. I'm not sure about the current progress of dylib builds for Windows. `CLANG_ABI` is supposed to annotate `clang-cpp.dll` (which exposes C++ interfaces), but not `libclang.dll` (which exposes C interfaces). I think the problem is that `libclang.dll` is exporting C++ interfaces; that looks unintended. The issue report uses a GNU style command line for the windows-msvc target, but such that configuration doesn't seem to have been tested well. Perhaps some of `if (MSVC)` blocks should be changed to `(WIN32 AND NOT MINGW)`. https://github.com/llvm/llvm-project/pull/171054 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix visibility macros for c-index-test (PR #171054)
kkent030315 wrote: Apparently `-DCLANG_LINK_CLANG_DYLIB=ON` is still a lot of mess right now, couldn't get it built on main. ``` RuntimeLibcallInfo.h:37:3: error: attribute 'dllexport' cannot be applied to member of 'dllexport' class 37 | LLVM_ABI static AnalysisKey Key; ``` https://github.com/llvm/llvm-project/pull/171054 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix visibility macros for c-index-test (PR #171054)
kkent030315 wrote: > I believe this breaks dylib builds (`-DCLANG_LINK_CLANG_DYLIB=ON`), which is > still in progress. `__declspec(dllimport)` is essential for that. How so? Can you be more specific how this would break dylib builds, and what exactly is in the progress? As mentioned in the comment dllimport attribute is not strictly required, it's safe to remove it. https://github.com/llvm/llvm-project/pull/171054 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix visibility macros for c-index-test (PR #171054)
kikairoya wrote: I believe this breaks dylib builds (`-DCLANG_LINK_CLANG_DYLIB=ON`), which is still in progress. `__declspec(dllimport)` is essential for that. https://github.com/llvm/llvm-project/pull/171054 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix visibility macros for c-index-test (PR #171054)
kkent030315 wrote: Probably related to #163349, I'm still not yet sure if this is the best solution. Please let me know if you have better ideas, thanks. https://github.com/llvm/llvm-project/pull/171054 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix visibility macros for c-index-test (PR #171054)
kkent030315 wrote:
This also eliminates 5000+ unnecessary symbol exports in `clang.dll` and
`clang.exe` on non-static builds.
To test this change turn `CLANG_INCLUDE_TESTS` and `LLVM_BUILD_STATIC` off.
This patch should only take an effects on Windows builds.
```
cmake -S llvm -B build -G Ninja -DLLVM_ENABLE_PROJECTS="clang"
-DCMAKE_BUILD_TYPE=Release -DLLVM_TARGETS_TO_BUILD="X86" -DLLVM_BUILD_TESTS=OFF
-DCLANG_INCLUDE_TESTS={OFF,ON} -DLLVM_BUILD_STATIC={OFF,ON}
```
https://github.com/llvm/llvm-project/pull/171054
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix visibility macros for c-index-test (PR #171054)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: None (kkent030315)
Changes
Fixes #152083 - duplicate symbols in `c-index-test`
- Eliminated the use of `CLANG_EXPORTS` for Clang component libraries.
- Removed unnecessary dllimport attributes from `Compiler.h`. They aren't
strictly required (the linker will resolve them).
---
Full diff: https://github.com/llvm/llvm-project/pull/171054.diff
2 Files Affected:
- (modified) clang/cmake/modules/AddClang.cmake (-3)
- (modified) clang/include/clang/Support/Compiler.h (+2-2)
``diff
diff --git a/clang/cmake/modules/AddClang.cmake
b/clang/cmake/modules/AddClang.cmake
index 4059fc3e986c7..9dcc77d5a5184 100644
--- a/clang/cmake/modules/AddClang.cmake
+++ b/clang/cmake/modules/AddClang.cmake
@@ -115,9 +115,6 @@ macro(add_clang_library name)
if(TARGET "obj.${name}")
target_compile_definitions("obj.${name}" PUBLIC CLANG_BUILD_STATIC)
endif()
- elseif(TARGET "obj.${name}" AND NOT ARG_SHARED AND NOT ARG_STATIC)
-# Clang component libraries linked to clang-cpp are declared without
SHARED or STATIC
-target_compile_definitions("obj.${name}" PUBLIC CLANG_EXPORTS)
endif()
set(libs ${name})
diff --git a/clang/include/clang/Support/Compiler.h
b/clang/include/clang/Support/Compiler.h
index e1ae3eda4ccc2..1aab64e11521c 100644
--- a/clang/include/clang/Support/Compiler.h
+++ b/clang/include/clang/Support/Compiler.h
@@ -45,8 +45,8 @@
#define CLANG_TEMPLATE_ABI
#define CLANG_EXPORT_TEMPLATE __declspec(dllexport)
#else
-#define CLANG_ABI __declspec(dllimport)
-#define CLANG_TEMPLATE_ABI __declspec(dllimport)
+#define CLANG_ABI
+#define CLANG_TEMPLATE_ABI
#define CLANG_EXPORT_TEMPLATE
#endif
#elif defined(__ELF__) || defined(__MINGW32__) || defined(_AIX) ||
\
``
https://github.com/llvm/llvm-project/pull/171054
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Fix visibility macros for c-index-test (PR #171054)
https://github.com/kkent030315 created
https://github.com/llvm/llvm-project/pull/171054
Fixes #152083 - duplicate symbols in `c-index-test`
- Eliminated the use of `CLANG_EXPORTS` for Clang component libraries.
- Removed unnecessary dllimport attributes from `Compiler.h`. They aren't
strictly required (the linker will resolve them).
>From b78486abe00fe4a06594bbbc2abdb244c66bb4c8 Mon Sep 17 00:00:00 2001
From: kkent030315
Date: Mon, 8 Dec 2025 04:55:11 +0900
Subject: [PATCH] [Clang] Fix visibility macros for c-index-test
Fixes #152083 - duplicate symbols in `c-index-test`
- Eliminated the use of `CLANG_EXPORTS` for Clang component libraries.
- Removed unnecessary dllimport attributes from `Compiler.h`. They aren't
strictly required (the linker will resolve them).
---
clang/cmake/modules/AddClang.cmake | 3 ---
clang/include/clang/Support/Compiler.h | 4 ++--
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/clang/cmake/modules/AddClang.cmake
b/clang/cmake/modules/AddClang.cmake
index 4059fc3e986c7..9dcc77d5a5184 100644
--- a/clang/cmake/modules/AddClang.cmake
+++ b/clang/cmake/modules/AddClang.cmake
@@ -115,9 +115,6 @@ macro(add_clang_library name)
if(TARGET "obj.${name}")
target_compile_definitions("obj.${name}" PUBLIC CLANG_BUILD_STATIC)
endif()
- elseif(TARGET "obj.${name}" AND NOT ARG_SHARED AND NOT ARG_STATIC)
-# Clang component libraries linked to clang-cpp are declared without
SHARED or STATIC
-target_compile_definitions("obj.${name}" PUBLIC CLANG_EXPORTS)
endif()
set(libs ${name})
diff --git a/clang/include/clang/Support/Compiler.h
b/clang/include/clang/Support/Compiler.h
index e1ae3eda4ccc2..1aab64e11521c 100644
--- a/clang/include/clang/Support/Compiler.h
+++ b/clang/include/clang/Support/Compiler.h
@@ -45,8 +45,8 @@
#define CLANG_TEMPLATE_ABI
#define CLANG_EXPORT_TEMPLATE __declspec(dllexport)
#else
-#define CLANG_ABI __declspec(dllimport)
-#define CLANG_TEMPLATE_ABI __declspec(dllimport)
+#define CLANG_ABI
+#define CLANG_TEMPLATE_ABI
#define CLANG_EXPORT_TEMPLATE
#endif
#elif defined(__ELF__) || defined(__MINGW32__) || defined(_AIX) ||
\
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
