labath added a comment.
I don't have time to do a full review of this today, but this seems pretty good
at a first glance...
================
Comment at: lldb/include/lldb/Target/Process.h:684
+ /// A status object indicating if the operation was sucessful or not.
+ virtual Status LoadModules() { return Status("Not implemented"); }
----------------
Since you're already changing the signature, let's make this return an
llvm::Error.
================
Comment at:
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp:184-206
+bool DYLDRendezvous::StateIsTakeSnapshot() {
+ return (
+ // When the previous and current states are consistent this is the first
+ // time we have been asked to update. Just take a snapshot of the
+ // currently loaded modules.
+ (m_previous.state == eConsistent && m_current.state == eConsistent) ||
+ // If we are about to add or remove a shared object clear out the current
----------------
I like that you've factored out the decoding of the states, but I think it
would be better to make this a separate function returning an enum (`GetAction`
?). This will make it more obvious which combinations of flags are covered.
Also, `StateIsTakeSnapshot` reads weirdly...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64013/new/
https://reviews.llvm.org/D64013
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
