mib added a comment.
Left some comments but overall looks good to me :) Thanks for taking this !
================
Comment at: lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp:37
+ offset_t file_offset, offset_t length) {
+ if (!data_sp) {
+ data_sp = MapFileData(*file, length, file_offset);
----------------
================
Comment at: lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp:47-52
+ if (data_sp->GetByteSize() < length) {
+ data_sp = MapFileData(*file, length, file_offset);
+ if (!data_sp)
+ return nullptr;
+ data_offset = 0;
+ }
----------------
Why do we do this twice ?
================
Comment at: lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp:92-111
+ if (!MagicBytesMatch(data_sp, data_offset, data_sp->GetByteSize()))
+ return 0;
+
+ auto text =
+ llvm::StringRef(reinterpret_cast<const char *>(data_sp->GetBytes()));
+
+ Expected<json::Value> json = json::parse(text);
----------------
I don't see the point of re-parsing the file here, we could just save the
parsed object as a class member.
================
Comment at: lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp:145
+ uint32_t magic = data.GetU8(&offset);
+ return magic == '{';
+}
----------------
Cool 😁
================
Comment at: lldb/source/Plugins/SymbolFile/JSON/SymbolFileJSON.cpp:112
+ symtab.AddSymbol(Symbol(
+ /*symID*/ 0, Mangled(symbol.name), eSymbolTypeCode,
+ /*is_global*/ true, /*is_debug*/ false,
----------------
Should we increment the `symID` here ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145180/new/
https://reviews.llvm.org/D145180
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
