teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land.
FWIW, we do have a test for this named TestBuiltinFormats.py. It's using the SB API so it's not as a direct test as this one, so I think having this too is good. I think this looks good otherwise so let's ship it. I have a few comments about the comments though, but that can be fixed while landing. ================ Comment at: lldb/unittests/Core/DumpDataExtractorTest.cpp:27 + endian::InlHostByteOrder(), + /*address size*/ 4); + DumpDataExtractor(extractor, &result, 0, format, data_size, item_count, 1, 0, ---------------- Please change that to something like `/*address_size=*/` where `address_size` is the exact argument name. This way clang-tidy can check the name for us. ================ Comment at: lldb/unittests/Core/DumpDataExtractorTest.cpp:74 + // test_format(std::complex<long double>(1.1, 2.2), lldb::eFormatComplex, + // "0x00000000: 1.1 + 2.2i"); + ---------------- I think the explanation is already good enough (and commented out code isn't great). ================ Comment at: lldb/unittests/Core/DumpDataExtractorTest.cpp:79 + test_format<uint16_t>(99, lldb::Format::eFormatDecimal, "0x00000000: 99"); + // Just prints as a signed integer + test_format(-1, lldb::Format::eFormatEnum, "0x00000000: -1"); ---------------- Can you put a period behind that comment and the others? Just being nit-picky here :) ================ Comment at: lldb/unittests/Core/DumpDataExtractorTest.cpp:175 + DataExtractor extractor(dumpbuffer.GetBytes(), dumpbuffer.GetByteSize(), + endian::InlHostByteOrder(), /*address size*/ 4); + ---------------- same as above, please `/*address_size=*/` ================ Comment at: lldb/unittests/Core/DumpDataExtractorTest.cpp:178 + DumpDataExtractor(extractor, &result, 0, lldb::Format::eFormatCharArray, + // item_byte_size + 1, ---------------- Same as above, `/*item_byte_size=*/1` (and the same below). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101453/new/ https://reviews.llvm.org/D101453 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
