labath added a comment. I didn't get around to address the style issues yet, but I wanted to respond to one high-level comment today:
> So we are currently only using the platform for the user ID resolving, but it > might be nice to keep the platform as the argument in case we need it for > something else in the future? Doing this would defeat the purpose of this patch, as the `ProcessInstanceInfo` class would no longer be standalone. I don't think it's very likely that we'll need other platform capabilities to decode this class, but in case we do, I see two options: - hide the details of this new thing behind an interface. This could either be a new interface, or an extension of the user id resolver interface, depending on what the thing is. - Move the Dumping code out of the ProcessInstanceInfo class. If interpreting the ProcessInstanceInfo data is so complicated that we need access to full platform class, then I don't think the ProcessInstanceInfo is the right place for that code. In this case, we could move the code to the platform class (`platform->Dump(stream, info)`), or a completely separate class altogether (like we did with the DumpDataExtractor classes). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58167/new/ https://reviews.llvm.org/D58167 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
