labath added inline comments.
================ Comment at: lldb/lit/Modules/ELF/minidebuginfo-corrupt-xz.yaml:1 +# REQUIRES: system-linux, lzma + ---------------- kwk wrote: > labath wrote: > > system-linux shouldn't be required here. One of the reasons I wanted to > > have these tests is that they can run on any system, not just on linux. > @labath what about the other file > `minidebuginfo-set-and-hit-breakpoint.test`. Remove `system-linux` from there > as well? You need to restrict that test somehow, because it only has a chance for working on elf platforms (as you're compiling binaries for the host and running them). It probably *should* work on all elf platforms, but we currently don't have a good way of expressing that and we're just restricting to system-linux currently. If you feel adventurous you can try introducing a `system-elf` feature (or something), and then changing this test (and possibly others) to use that instead, but please do that as a separate patch. ================ Comment at: lldb/source/Host/common/LZMA.cpp:129 + uint64_t uncompressedSize = 0; + auto err = getUncompressedSize(InputBuffer, uncompressedSize); + if (err) ---------------- kwk wrote: > labath wrote: > > MaskRay wrote: > > > if (auto err = ...) > > > return err; > > I have a feeling this pattern is actually more common in lldb. I tend to > > use this one, but I am fine with either. > So what now? I had `getCompressedSize` return an error before and changed it > upon request to expected. I can live with both but like the expected thing a > bit better in terms of: this goes in and this comes out mentality. My interpretation of @MaskRay's comment was that he wanted you do write something like ``` if (auto err = uncompressedSize.takeError()) return err; ``` That is orthogonal to what type is returned by `getCompressedSize`, and is indeed a more common pattern in at least some parts of llvm. I am personally fine with both. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66791/new/ https://reviews.llvm.org/D66791 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
