labath added inline comments.

================
Comment at: lit/tools/lldb-mi/interpreter/cli-support/breakpoint-set.test:4
+#
+# RUN: %cxx -o %t %p/inputs/main.cpp -g
+# RUN: %lldbmi %t < %s | FileCheck %s
----------------
aprantl wrote:
> stella.stamenova wrote:
> > apolyakov wrote:
> > > stella.stamenova wrote:
> > > > apolyakov wrote:
> > > > > stella.stamenova wrote:
> > > > > > One thing to consider here is that any extra parameters passed with 
> > > > > > -E to the test suite will not propagate to lit at the moment.
> > > > > > 
> > > > > > I ran into this with some internal testing where we need to pass 
> > > > > > parameters to the compiler - all of the lldb suite tests (c++ and 
> > > > > > c) build correctly, but any lit tests that directly invoke the 
> > > > > > compiler do not because the parameters do not get propagated all 
> > > > > > the way.
> > > > > Could you give an example of extra parameters? I didn't see them 
> > > > > before so I don't completely understand you.
> > > > -E "--sysroot=path/to/sys/root -lc++abi -lunwind"
> > > Ok, I think we won't use something like it here. Thank you.
> > I think you misunderstood my concern - let's say I have a machine on which 
> > I run these tests for a particular architecture that depends on passing 
> > these arguments to the tests, so that clang (cxx) correctly complies c++ 
> > files. *Before* your change, these arguments would have been propagated to 
> > the test in the lldb suite and the code would have build correctly. *After* 
> > your change, the code will no longer build correctly.
> > 
> > Essentially, by making these tests lit tests, you are removing support for 
> > passing these arguments to the compiler (since the lldb suite supports them 
> > and lit does not). It might still be worth making these lit tests for the 
> > sake of other benefits, but then such targets will end up having to XFAIL 
> > the tests.
> > 
> > If these tests really need to become lit tests and invoke the compiler, I 
> > think you also need to add support for passing these arguments to the 
> > compiler.
> > 
> I think the best way to solve this is by adding the platform-specific flags 
> to the expansion of `%cxx` in the lit configuration. Would that work here?
I am wondering how much do we actually need the lldb-mi tests to run on exotic 
(non-host) platforms/architectures. My take on these tests is that they should 
test the functionality from the lldb-mi protocol, and down (only) to the SB API 
calls. Anything below SB is a black box, which is assumed to be working (tested 
via other means). In particular, these tests should not care about whether the 
binary is built with libc++abi or split-dwarf or something else. The default 
debuggable executable produced by the given compiler should be enough.

In such a world, there is not much value in running the tests on other 
architectures, as (hopefully) all of those differences are hidden inside 
liblldb. The existing lldb-mi tests already do not support all the features 
that other dotest tests do (e.g. running the tests remotely). If getting this 
working  it's just the matter of adding something to the expansion of %cxx, 
then sure, why not... But otherwise I would not spend too much time engineering 
a very generic solution.


================
Comment at: 
lit/tools/lldb-mi/interpreter/cli-support/settings-set-target-run-after.test:13
+
+# FIXME: --arg1 causes an error.
+setting set target.run-args arg1 "2nd arg" third_arg fourth="4th arg"
----------------
Try:
```
setting set -- target.run-args --your --args --with --dashes
```


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D50525



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to