aprantl added a comment. 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. > - there is a `@skipUnlessClangModules` decorator in decorators.py. As far as > I can see, this patch should now make it obsolete. It seems that it can be > removed and all invocations replaced with `add_test_categories(["gmodules"])` Good catch. I didn't notice that! > 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. http://reviews.llvm.org/D19998 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
