fdeazeve created this revision.
Herald added a project: All.
fdeazeve requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The CMake variable LLDB_HAS_LIBCXX is passed to
`llvm_canonicalize_cmake_booleans`, which transforms TRUE/FALSE into 1/0
and "" to 0.

In particular, this means that the configuration script for LLDB API's
test always has _some_ value for the `has_libcxx` configuration:

  config.has_libcxx = '@LLDB_HAS_LIBCXX@'

When deciding whether a libcxx exist, the testing scripts would only
check for the existence of `has_libcxx`, but not not for its value. In
other words, `if ('0')` is true in python.

This was caught once D132940 <https://reviews.llvm.org/D132940> was merged and 
most tests started to use
libcxx by default if `has_libcxx` is true.  Prior to that, no failures
were seen because only tests are marked with
`@add_test_categories(["libc++"])` would require a libcxx, and these
would be filtered out on builds without the libcxx target.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133639

Files:
  lldb/test/API/lit.cfg.py


Index: lldb/test/API/lit.cfg.py
===================================================================
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -91,6 +91,13 @@
   This allows us to check if the attribute exists before trying to access 
it."""
   return getattr(config, attr, None)
 
+def is_configured_and_true(attr):
+  """Returns true if and only if the configuration attribute exists and it is
+  set to'1'.
+
+  This is useful when checking CMake variables that have been cannonicalized to
+  0/1."""
+  return getattr(config, attr, None) == '1'
 
 def delete_module_cache(path):
   """Clean the module caches in the test build directory.
@@ -171,7 +178,7 @@
   dotest_cmd += ['--env', 'LLVM_TOOLS_DIR=' + config.llvm_tools_dir]
 
 # If we have a just-built libcxx, prefer it over the system one.
-if is_configured('has_libcxx') and platform.system() != 'Windows':
+if is_configured_and_true('has_libcxx') and platform.system() != 'Windows':
   if is_configured('llvm_include_dir') and is_configured('llvm_libs_dir'):
     dotest_cmd += ['--libcxx-include-dir', 
os.path.join(config.llvm_include_dir, 'c++', 'v1')]
     dotest_cmd += ['--libcxx-library-dir', config.llvm_libs_dir]


Index: lldb/test/API/lit.cfg.py
===================================================================
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -91,6 +91,13 @@
   This allows us to check if the attribute exists before trying to access it."""
   return getattr(config, attr, None)
 
+def is_configured_and_true(attr):
+  """Returns true if and only if the configuration attribute exists and it is
+  set to'1'.
+
+  This is useful when checking CMake variables that have been cannonicalized to
+  0/1."""
+  return getattr(config, attr, None) == '1'
 
 def delete_module_cache(path):
   """Clean the module caches in the test build directory.
@@ -171,7 +178,7 @@
   dotest_cmd += ['--env', 'LLVM_TOOLS_DIR=' + config.llvm_tools_dir]
 
 # If we have a just-built libcxx, prefer it over the system one.
-if is_configured('has_libcxx') and platform.system() != 'Windows':
+if is_configured_and_true('has_libcxx') and platform.system() != 'Windows':
   if is_configured('llvm_include_dir') and is_configured('llvm_libs_dir'):
     dotest_cmd += ['--libcxx-include-dir', os.path.join(config.llvm_include_dir, 'c++', 'v1')]
     dotest_cmd += ['--libcxx-library-dir', config.llvm_libs_dir]
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits]... Felipe de Azevedo Piovezan via Phabricator via lldb-commits
    • [Lldb-com... Felipe de Azevedo Piovezan via Phabricator via lldb-commits
    • [Lldb-com... Jonas Devlieghere via Phabricator via lldb-commits
    • [Lldb-com... Felipe de Azevedo Piovezan via Phabricator via lldb-commits
    • [Lldb-com... Felipe de Azevedo Piovezan via Phabricator via lldb-commits
    • [Lldb-com... Jonas Devlieghere via Phabricator via lldb-commits
    • [Lldb-com... Felipe de Azevedo Piovezan via Phabricator via lldb-commits
    • [Lldb-com... Jonas Devlieghere via Phabricator via lldb-commits
    • [Lldb-com... Felipe de Azevedo Piovezan via Phabricator via lldb-commits

Reply via email to