The plan for the lit tests sounds reasonable to me. I would also remove
LLDB_TEST_C/CXX_COMPILER entirely so that we can reduce confusion since they’re
only used for the lit tests, right?
My only concern is that I’ve been told that there are people who will build
lldb with a different compiler than the tests – so the properties for
LLDB_TEST_C/CXX_COMPILER might actually be used especially in cases where clang
is not built alongside lldb.
Thanks,
-Stella
From: Zachary Turner <ztur...@google.com>
Sent: Tuesday, November 13, 2018 4:16 PM
To: Stella Stamenova <sti...@microsoft.com>
Cc: reviews+d54009+public+0e164460da8f1...@reviews.llvm.org; pa...@labath.sk;
chris.biene...@me.com; dccitali...@gmail.com; aleksandr.ura...@jetbrains.com;
jdevliegh...@apple.com; abidh....@gmail.com; teempe...@gmail.com;
ki.s...@gmail.com; mgo...@gentoo.org; d...@su-root.co.uk; jfbast...@apple.com;
lldb-commits@lists.llvm.org; l...@inglorion.net
Subject: Re: [PATCH] D54009: Refactor LLDB lit configuration files
Ok so for dotest, it seems to be ignoring the config.cc and config.cxx
entirely. So we can theoretically do whatever we want with it, or change
around the directory structure so that it's more like:
lldb
* lit
* * Dotest
* * Unit
* * Tests
and put the config.cc / config.cxx logic under Tests. That's a large change
though and probably not worth making such a large change right away.
dotest tests manually construct the command line directly in CMake via this
`LLDB_DOTEST_ARGS_PROPERTY` global property, and then in lldb/lit/Suite/lit.cfg
we have this line:
dotest_cmd = [config.dotest_path, '-q']
dotest_cmd.extend(config.dotest_args_str.split(';'))
So pretty much everythign the parent lit file has done is totally ignored.
With that in mind, **for the lit tests only** I propose dropping support for
non-clang compilers and ignoring LLDB_TEST_C_COMPILER and
LLDB_TEST_CXX_COMPILER (you can still have a custom path to clang executable
via an environment variable, which is consistent with how clang's test suite
works).
Note that when you run ninja check-lldb-lit you will now get messages that tell
you the exact path to the clang executable, so you can see what the PATH
resolution is doing.
On Tue, Nov 13, 2018 at 4:02 PM Zachary Turner
<ztur...@google.com<mailto:ztur...@google.com>> wrote:
On Tue, Nov 13, 2018 at 3:47 PM Stella Stamenova
<sti...@microsoft.com<mailto:sti...@microsoft.com>> wrote:
I am not sure if that’s the right solution for a couple of reasons:
1. As far as I can tell only clang calls use_clang (and only lld calls
use_lld), while the other projects such as lld and llvm rely on the environment
to be setup correctly
2. Lld also has tests that invoke clang-cl and they pass – while the ones in
LLDB do not, so the invocation of use_clang is not necessary for the tests to
pass (maybe?)
3. LLDB allows us to specify whether to use gcc or clang as well as the path
and it can also have a test compiler specified via LLDB_USE_TEST_*_COMPILER, so
we should first decide what scenarios we want to support before trying to make
this work and possibly making it even more confusing and complicated
Do you know what the answer for 3) is? What compilers are valid to specify for
the lit/suite/unittests via the various parameters?
For the unit tests, I don't think we ever specify a compiler, or we don't ever
read the value. Because a unit test shouldn't be compiling anything, it's a
different kind of test.
For the dotest suite, specifying the compiler is important and it can
definitely be gcc, but I don't think this uses the same method of going through
config.cc. In fact, I'm not sure how it determines what compiler to use at the
moment, as it's been a number of years since I've looked at the dotest suite.
For the lit tests, I'm inclined to say we should keep things simple and only
support clang for now, and add support for new compilers such as gcc if and
when someone actually wants it. Otherwise YAGNI.
Definitely that time will come, but it doesn't make sense to support it
immediately if nobody is using it today and nobody is planning to enable it
immediately.
So I'm tempted to say that perhaps we should just call llvm_config.use_clang()
and llvm_config.use_lld() and ignore LLDB_TEST_COMPILER, which in my experience
has only been a source of unnecessary complexity that never actually gets used
in practice.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
