[clang] 592e935 - [clang-repl] Fix REPL_EXTERNAL_VISIBILITY and building libclang-cpp.dll for MinGW configurations

[Martin Storsjö via cfe-commits]

Author: Martin Storsjö
Date: 2023-05-28T13:16:53+03:00
New Revision: 592e935e115ffb451eb9b782376711dab6558fe0

URL: https://github.com/llvm/llvm-project/commit/592e935e115ffb451eb9b782376711dab6558fe0
DIFF: https://github.com/llvm/llvm-project/commit/592e935e115ffb451eb9b782376711dab6558fe0.diff

LOG: [clang-repl] Fix REPL_EXTERNAL_VISIBILITY and building libclang-cpp.dll for MinGW configurations

This fixes two issues that are observed after
5111286f06e1e10f24745007a45a830760f1790c:

For builds with GCC with LLVM_LINK_LLVM_DYLIB=ON, we previously got
build errors, as libclang-cpp.dll suddenly only contained the
functions that were marked dllexport via REPL_EXTERNAL_VISIBILITY,
instead of all symbols as expected.

For MinGW builds with Clang, building previously succeeded (as it
used either the __attribute__((visibility("default"))) annotation or
nothing at all), and the functions were exported from libclang-cpp.dll
if that was built, but the unit test failed (as neither of those cases
made the functions exported from an EXE).

Don't use the visibility attributes on MinGW targets for these purposes;
setting default visibility only makes a difference if building with
e.g. -fvisibility=hidden, but it doesn't make the symbols exported
from an EXE.

Differential Revision: https://reviews.llvm.org/D151620

Added: 
    

Modified: 
    clang/include/clang/Interpreter/Value.h
    clang/tools/clang-shlib/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Interpreter/Value.h b/clang/include/clang/Interpreter/Value.h
index 4df4367030ecd..c380cd91550de 100644
--- a/clang/include/clang/Interpreter/Value.h
+++ b/clang/include/clang/Interpreter/Value.h
@@ -52,18 +52,24 @@ class ASTContext;
 class Interpreter;
 class QualType;
 
-#if __has_attribute(visibility) &&                                             \
-    (!(defined(_WIN32) || defined(__CYGWIN__)) ||                              \
-     (defined(__MINGW32__) && defined(__clang__)))
+#if defined(_WIN32)
+// REPL_EXTERNAL_VISIBILITY are symbols that we need to be able to locate
+// at runtime. On Windows, this requires them to be exported from any of the
+// modules loaded at runtime. Marking them as dllexport achieves this; both
+// for DLLs (that normally export symbols as part of their interface) and for
+// EXEs (that normally don't export anything).
+// For a build with libclang-cpp.dll, this doesn't make any 
diff erence - the
+// functions would have been exported anyway. But for cases when these are
+// statically linked into an EXE, it makes sure that they're exported.
+#define REPL_EXTERNAL_VISIBILITY __declspec(dllexport)
+#elif __has_attribute(visibility)
 #if defined(LLVM_BUILD_LLVM_DYLIB) || defined(LLVM_BUILD_SHARED_LIBS)
 #define REPL_EXTERNAL_VISIBILITY __attribute__((visibility("default")))
 #else
 #define REPL_EXTERNAL_VISIBILITY
 #endif
 #else
-#if defined(_WIN32)
-#define REPL_EXTERNAL_VISIBILITY __declspec(dllexport)
-#endif
+#define REPL_EXTERNAL_VISIBILITY
 #endif
 
 #define REPL_BUILTIN_TYPES                                                     \

diff  --git a/clang/tools/clang-shlib/CMakeLists.txt b/clang/tools/clang-shlib/CMakeLists.txt
index de9daba6716a9..aa7fcd1efed45 100644
--- a/clang/tools/clang-shlib/CMakeLists.txt
+++ b/clang/tools/clang-shlib/CMakeLists.txt
@@ -53,3 +53,11 @@ add_clang_library(clang-cpp
 if (NOT APPLE AND NOT MINGW)
   target_link_options(clang-cpp PRIVATE LINKER:-Bsymbolic-functions)
 endif()
+if (MINGW OR CYGWIN)
+  # The clang-cpp DLL is supposed to export all symbols (except for ones
+  # that are explicitly hidden). Normally, this is what happens anyway, but
+  # if there are symbols that are marked explicitly as dllexport, we'd only
+  # export them and nothing else. Therefore, add --export-all-symbols to
+  # make sure we export all symbols despite potential dllexports.
+  target_link_options(clang-cpp PRIVATE LINKER:--export-all-symbols)
+endif()