[PATCH] D113435: [Clang][Emscripten]: Don't default to -fvisibility=hidden

[Sam Clegg via Phabricator via cfe-commits]

sbc100 created this revision.
Herald added subscribers: wingo, sunfish, dschuff.
sbc100 requested review of this revision.
Herald added subscribers: cfe-commits, aheejin.
Herald added a project: clang.

Emscripten wants `-fvisibility=default` by default.  See:
https://github.com/emscripten-core/emscripten/pull/15413

For more disussion around what the default should be see
https://github.com/WebAssembly/tool-conventions/issues/176.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113435

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/wasm-toolchain.c


Index: clang/test/Driver/wasm-toolchain.c
===================================================================
--- clang/test/Driver/wasm-toolchain.c
+++ clang/test/Driver/wasm-toolchain.c
@@ -12,6 +12,10 @@
 // RUN:   | FileCheck -check-prefix=FVISIBILITY_DEFAULT %s
 // FVISIBILITY_DEFAULT-NOT: hidden
 
+// Emscripten is different in that it defaults to default, like other 
non-WebAssembly targets.
+// RUN: %clang %s -### -target wasm32-unknown-emscripten 2>&1 \
+// RUN:   | FileCheck -check-prefix=FVISIBILITY_DEFAULT %s
+
 // A basic C link command-line with unknown OS.
 
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=/foo %s 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2288,8 +2288,12 @@
 
 void Clang::AddWebAssemblyTargetArgs(const ArgList &Args,
                                      ArgStringList &CmdArgs) const {
-  // Default to "hidden" visibility.
-  if (!Args.hasArg(options::OPT_fvisibility_EQ,
+  const llvm::Triple &T = getToolChain().getTriple();
+  // Default to "hidden" visibility for WebAssembly targets except
+  // for emscripten which uses the default for compatibility with
+  // existing code.
+  if (T.getOS() != llvm::Triple::Emscripten &&
+      !Args.hasArg(options::OPT_fvisibility_EQ,
                    options::OPT_fvisibility_ms_compat)) {
     CmdArgs.push_back("-fvisibility");
     CmdArgs.push_back("hidden");



Index: clang/test/Driver/wasm-toolchain.c
===================================================================
--- clang/test/Driver/wasm-toolchain.c
+++ clang/test/Driver/wasm-toolchain.c
@@ -12,6 +12,10 @@
 // RUN:   | FileCheck -check-prefix=FVISIBILITY_DEFAULT %s
 // FVISIBILITY_DEFAULT-NOT: hidden
 
+// Emscripten is different in that it defaults to default, like other non-WebAssembly targets.
+// RUN: %clang %s -### -target wasm32-unknown-emscripten 2>&1 \
+// RUN:   | FileCheck -check-prefix=FVISIBILITY_DEFAULT %s
+
 // A basic C link command-line with unknown OS.
 
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2288,8 +2288,12 @@
 
 void Clang::AddWebAssemblyTargetArgs(const ArgList &Args,
                                      ArgStringList &CmdArgs) const {
-  // Default to "hidden" visibility.
-  if (!Args.hasArg(options::OPT_fvisibility_EQ,
+  const llvm::Triple &T = getToolChain().getTriple();
+  // Default to "hidden" visibility for WebAssembly targets except
+  // for emscripten which uses the default for compatibility with
+  // existing code.
+  if (T.getOS() != llvm::Triple::Emscripten &&
+      !Args.hasArg(options::OPT_fvisibility_EQ,
                    options::OPT_fvisibility_ms_compat)) {
     CmdArgs.push_back("-fvisibility");
     CmdArgs.push_back("hidden");

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits