teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land.
LGTM. I still kinda like the unique_ptr deleter but let's not block bot-fixes with refactoring requests. I'll open a review for the unique_ptr as a follow up. In D100800#2699984 <https://reviews.llvm.org/D100800#2699984>, @MaskRay wrote: > In D100800#2699940 <https://reviews.llvm.org/D100800#2699940>, @teemperor > wrote: > >> Thanks for fixing this, I guess we really need a leak sanitizer/valgrind bot >> for LLDB... >> >> I just have some minor comments but otherwise this LGTM. > > Agree.. The 45+ `check-lldb` failures need to be fixed first.. Do you have a list of test failures around? Otherwise I can run the test suite myself when I'm back in the (home) office. ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1249 attrs.is_inline); + free(buf); ---------------- MaskRay wrote: > teemperor wrote: > > `std::free` ? > `std::` for C library functions is uncommon. > > For some common functions (free,strcpy,memset,memcpy,...), the unqualified > version is more common. I can find some `::foo` as well but `std::foo` is > really rare. I believe we're using that more often in newer code (including the demangler), but that was more of nit-pick. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100800/new/ https://reviews.llvm.org/D100800 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
