Skip to content

[llvm] DLLExport Twine #127853

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

Closed
wants to merge 1 commit into from
Closed

Conversation

andrurogerz
Copy link
Contributor

@andrurogerz andrurogerz commented Feb 19, 2025

Overview

Annotate the llvm::Twine class so it is explicitly included in LLVM's public interface. When building LLVM as a Windows DLL, this annotation exports Twine's public methods from the DLL.

Background

This change is required as part of the overall project to build LLVM as a Windows DLL described in #109483. Without this change, LLVM tools fail to link.

Validation

Built LLVM with MSVC on Windows 11:

cmake -B build -S llvm -G Ninja -DCMAKE_BUILD_TYPE=Debug -DLLVM_ENABLE_PROJECTS="clang" -DLLVM_OPTIMIZED_TABLEGEN -DLLVM_BUILD_LLVM_DYLIB=ON -DLLVM_BUILD_LLVM_DYLIB_VIS=ON -DLLVM_LINK_LLVM_DYLIB=ON -DCLANG_LINK_CLANG_DYLIB=ON
ninja -C build llvm-rc llvm-ar

Verified the annotated interface no longer appears in the list of unresolved external symbols.

@andrurogerz andrurogerz marked this pull request as ready for review February 19, 2025 21:57
@llvmbot
Copy link
Member

llvmbot commented Feb 19, 2025

@llvm/pr-subscribers-llvm-adt

Author: Andrew Rogers (andrurogerz)

Changes

Overview

Annotate the llvm::Twine class so it is explicitly included in LLVM's public interface. When building LLVM as a Windows DLL, this annotation exports Twine's public methods from the DLL.

Background

This change is required as part of the overall project to build LLVM as a Windows DLL described in #109483. Without this change, LLVM tools fail to link.

Validation

Built LLVM with MSVC on Windows 11:

cmake -B build -S llvm -G Ninja -DCMAKE_BUILD_TYPE=Debug -DLLVM_ENABLE_PROJECTS="clang" -DLLVM_OPTIMIZED_TABLEGEN -DLLVM_BUILD_LLVM_DYLIB=ON -DLLVM_BUILD_LLVM_DYLIB_VIS=ON -DLLVM_LINK_LLVM_DYLIB=ON -DCLANG_LINK_CLANG_DYLIB=ON
ninja -C build llvm-rc llvm-ar

Verified the annotated interface no longer appears in the list of unresolved external symbols.


Full diff: https://github.com/llvm/llvm-project/pull/127853.diff

1 Files Affected:

  • (modified) llvm/include/llvm/ADT/Twine.h (+2-1)
diff --git a/llvm/include/llvm/ADT/Twine.h b/llvm/include/llvm/ADT/Twine.h
index 1f1fd1967efbc..e50916f6d003f 100644
--- a/llvm/include/llvm/ADT/Twine.h
+++ b/llvm/include/llvm/ADT/Twine.h
@@ -11,6 +11,7 @@
 
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cassert>
 #include <cstdint>
@@ -78,7 +79,7 @@ namespace llvm {
   /// so we provide two additional methods (and accompanying operator+
   /// overloads) to guarantee that particularly important cases (cstring plus
   /// StringRef) codegen as desired.
-  class Twine {
+  class LLVM_ABI Twine {
     /// NodeKind - Represent the type of an argument.
     enum NodeKind : unsigned char {
       /// An empty string; the result of concatenating anything with it is also

@andrurogerz
Copy link
Contributor Author

@compnerd, @vgvassilev

@andrurogerz andrurogerz changed the title DLLExport Twine [llvm] DLLExport Twine Feb 19, 2025
@nikic
Copy link
Contributor

nikic commented Feb 20, 2025

Do not submit these one by one. Consolidating on the discussion in #127850.

@nikic nikic closed this Feb 20, 2025
@andrurogerz andrurogerz deleted the DLLExport-Twine branch February 26, 2025 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants