[libcxx-commits] [PATCH] D120162: [libcxx][test] Test visibility option before using it

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.
Herald added a project: All.


================
Comment at: libcxx/test/libcxx/debug/extern-templates.sh.cpp:22
 // option which clang doesn't accept on Windows.)
 // UNSUPPORTED: windows
 
----------------
I checked and this test basically passes vacuously when we don't enforce hidden visibility. So instead, I would either mark the test as truly failing on AIX for lack of being able to control visibility (if that's the case -- IDK how AIX works when it comes to visibility):

```
// AIX doesn't have a notion of visibility, so this test is vacuous.
// UNSUPPORTED: whatever-triple-you-use-for-aix
```

Or, if there *is* a notion of visibility and it's simply that the compiler doesn't support `-fvisibility=hidden`, we should:

1. File a bug for the compiler to start supporting the option, and 

2. Use this diff instead:

```
diff --git a/libcxx/test/libcxx/debug/extern-templates.sh.cpp b/libcxx/test/libcxx/debug/extern-templates.sh.cpp
index b43f4d10af5c..c635674a6606 100644
--- a/libcxx/test/libcxx/debug/extern-templates.sh.cpp
+++ b/libcxx/test/libcxx/debug/extern-templates.sh.cpp
@@ -21,12 +21,12 @@
 // option which clang doesn't accept on Windows.)
 // UNSUPPORTED: windows
 
-// XFAIL: LIBCXX-AIX-FIXME
-
-// RUN: %{cxx} %{flags} %{compile_flags} %s %{link_flags} -fPIC -DTU1 -D_LIBCPP_DEBUG=1 -fvisibility=hidden -shared -o %t.lib
-// RUN: cd %T && %{cxx} %{flags} %{compile_flags} %s ./%basename_t.tmp.lib %{link_flags} -DTU2 -D_LIBCPP_DEBUG=1 -fvisibility=hidden -o %t.exe
+// RUN: %{cxx} %{flags} %{compile_flags} %s %{link_flags} -fPIC -DTU1 -D_LIBCPP_DEBUG=1 -shared -o %t.lib
+// RUN: cd %T && %{cxx} %{flags} %{compile_flags} %s ./%basename_t.tmp.lib %{link_flags} -DTU2 -D_LIBCPP_DEBUG=1 -o %t.exe
 // RUN: %{exec} %t.exe
 
+#pragma GCC visibility push(hidden)
+
 #include <cassert>
 #include <cstdio>
 #include <sstream>
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120162/new/

https://reviews.llvm.org/D120162