JDevlieghere added inline comments.
================
Comment at: lldb/test/API/lit.cfg.py:181-184
+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]
----------------
fdeazeve wrote:
> JDevlieghere wrote:
> > Are there other places where this helper would be useful? If not I think we
> > can check the value directly. With the helper we lose a little bit of
> > signal (if it returns false, was it not configured or set to false?). I
> > don't think it's a big deal, but those two things combined make me favor
> > inlining the check.
> I agree that inlining is better here, but note that `config.has_libcxx`
> doesn't work, this is still going to return `True`: `config.has_libcxx` is a
> string, not a boolean, so anything `!= None` -- in particular '0' -- will
> evaluate to true.
>
> There are two ways to inline this:
>
> 1.`is_configured('has_libcxx') and config.has_libcxx == '1'`
> 2.`is_configured('has_libcxx') == '1'`
>
> Actually, since `has_libcxx` comes from a canonicalized variable (in the
> sense described in the commit message), we could also just do:
>
> 3.`config.has_libcxx == '1'`
>
>
> What do you think?
>
> Related: in a separate patch I think we should change the implementation of
> `is_configured` to compare against None, so that it returns a boolean.
Aha, if `has_libcxx` is a string then that's a bug. It should be `0` or `1`
like `config.shared_libs` and `config.lldb_enable_python`. Seems like someone
(me) unintentionally added quotes.
This should fix that:
```
-config.has_libcxx = '@LLDB_HAS_LIBCXX@'
+config.has_libcxx = @LLDB_HAS_LIBCXX@
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133639/new/
https://reviews.llvm.org/D133639
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
