[Lldb-commits] [PATCH] D148062: [lldb] Make ObjectFileJSON loadable as a module

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.


================
Comment at: lldb/source/Core/Section.cpp:683-684
+  llvm::json::ObjectMapper o(value, path);
+  return o && o.map("name", section.name) && o.map("type", section.type) &&
+         o.map("size", section.address) && o.map("size", section.size);
+}
----------------
mib wrote:
> We should use `o.mapOptional` here since the `address`, `size` and `type` are marked optional.
They're not exactly the same: `map` with a `std::optional` lets you deal with whether the value was set outside of the deserialization method, while `mapOptional` requires you to have initialized the variable to the default value previously, or pass it as a third argument. In this particular case it doesn't really matter, as the default value is pretty trivial, but I'd rather preserve that a value wasn't set. That's different from the call to `mapOptional` for the symbol and section vectors, where I don't care whether the vector is empty or was omitted completely. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148062/new/

https://reviews.llvm.org/D148062