labath added a comment. In http://reviews.llvm.org/D19998#423541, @aprantl wrote:
> In http://reviews.llvm.org/D19998#423277, @labath wrote: > > > I am glad to see more testing of the modules debugging. I have a couple of > > small comments though: > > > > - `-fmodules`: Why is it not being added to CXXFLAGS? Is this how clang is > > supposed to be invoked? (I am not very familiar clang modules) > > > C++ modules are still a work in progress and not supported on all platforms > (particularly on Darwin due to the way the system module maps interact with > libc++, see https://llvm.org/bugs/show_bug.cgi?id=26928 for examples). On the > platforms where C++ modules work well (such as Linux) on the other hand, > module debugging hasn't been productized so far. Due to the way module > debugging reuses DWO mechanisms I don't expect it to work without some > fine-tuning. Thank you for the comprehensive explanation. This information is interesting. Does that mean that in case of tests with c++ source code, there will be no difference in the test executables between modules and non-modules versions of the tests? I am wondering whether we should avoid running the modules tests in this case... I just did a quick count and we seem to have 129 C source files vs. 262 C++ files, so the difference is not actually as big as I expected, but it is still a substantial amount of test time going to waste. Maybe it's not that important though, we can possibly optimize that later, if it turns out to be necessary. > > And one meta-comment not directly related to this patch: > > > We already run most of the tests two times. Now we will be doing it once > > more, which will increase the test times even more. I think it's important > > for each debug info format to have good coverage, but I also feel that > > there are tests which have nothing to do with debug info (or their > > connection to debug info is only very peripheral), and it does not make to > > sense to slow down the tests runs by running those tests so many times. We > > already have a (not very elegant, but working) mechanism to avoid this > > (`NO_DEBUG_INFO_TESTCASE` member). I propose that we be more aggressive in > > using it for new tests which do not specifically test debug info. Also when > > looking at existing tests, we should re-evaluate whether the test really > > needs to be run that many times (right now, the largest candidate that > > comes to mind is TestConcurrentEvents, but I am sure there are others I > > can't think of by name now). > > > That sounds like an all-around good idea, but probably out of scope for this > patch. Yes, I am definitely not requesting you make any changes like that here. I using just testing the waters about what people think about this idea. It's a bit of a hijack though, so sorry about that. We can move the discussion outside, if it starts to get more involved. In http://reviews.llvm.org/D19998#423562, @tfiala wrote: > > I propose that we be more aggressive in using it for new tests which do > > not specifically test debug info. > > > I totally agree, but I also caution that, as a debugger, the heart of much of > what we do does use debug info. So we need to be careful to make sure that > we don't disable the fan-out across all debug styles if we are in fact using > debug info. I guess the question here is what do we consider as "using the debug info". To take TestConcurrentEvents as an example, it certainly *uses* debug info, in the sense that it sets a breakpoint at a certain line, and sets some variables, but I don't think this is what the test is about. It is about testing that the debugger handles the situation where there are multiple events happening in the target concurrently, which should not be related to the debug info at all. Any failure it this test which would be caused by a debug info problem should already be caught by some other test which actually targets these things. At least, that's my opinion... ================ Comment at: packages/Python/lldbsuite/test/plugins/builder_base.py:144 @@ -143,1 +143,3 @@ +def buildDwarf(sender=None, architecture=None, compiler=None, dictionary=None, clean=True): + """Build the binaries with dwarf debug info.""" ---------------- shouldn't this be `buildModules` or something? http://reviews.llvm.org/D19998 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
