[PATCH] D8652: [libcxx] Create internal namespace that hides all symbols.

Eric Fiselier eric at efcs.ca
Sat Jul 18 09:19:56 PDT 2015


EricWF marked 3 inline comments as done.
EricWF added a comment.

Address @danalbert's comments.


================
Comment at: test/libcxx/test/config.py:640
@@ +639,3 @@
+        if os.path.isfile(libcxx_lib):
+            pass
+        elif self.target_info.platform() == 'darwin':
----------------
danalbert wrote:
> Why can't we check the static lib? The symbols will still be hidden either way.
We can check a static library. This branch selects `libc++.a` for checking if it exists. Otherwise we try `libc++.dylib` or `libc++.so`.

================
Comment at: test/libcxx/test/config.py:643
@@ +642,3 @@
+            libcxx_lib = os.path.join(libcxx_root, 'libc++.dylib')
+        elif libcxx_lib:
+            libcxx_lib = os.path.join(libcxx_root, 'libc++.so')
----------------
danalbert wrote:
> This is always true.
Good catch. That should be an 'else'.

================
Comment at: test/libcxx/test/config.py:670
@@ +669,3 @@
+        # and self.env['PATH'].  Also add libcxx/utils/sym_check to the path.
+        path_list = list(sys.path)
+        path_list += self.lit_config.path
----------------
danalbert wrote:
> Why `sys.path` (`PYTHONPATH`)?
Probably a mistake. I thought `sys.path` and `os.environ['PATH']` were essentially the same. I'll remove it.


http://reviews.llvm.org/D8652







More information about the cfe-commits mailing list