[Lldb-commits] [PATCH] D77602: [lldb/Reproducers] Support new replay mode: passive replay

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think we're getting closer, though there is still some duplication that I'd like to eradicate.



================
Comment at: lldb/include/lldb/Utility/Reproducer.h:312
 
+  void SetPassiveReplay(bool b) { m_passive_replay = b; }
+
----------------
Now it looks like nothing is calling this function...


================
Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:305
     if (is_trivially_serializable<T>::value)
-      return;
+      return const_cast<T &>(t);
     // We need to make a copy as the original object might go out of scope.
----------------
What's up with the `const_cast` ? Should this maybe take a `T &t` argument and let `T` be deduced as `const U` when needed?


================
Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:645-651
+      : m_serializer(nullptr), m_deserializer(nullptr), m_registry(nullptr){};
   InstrumentationData(Serializer &serializer, Registry &registry)
-      : m_serializer(&serializer), m_registry(&registry){};
-
-  Serializer &GetSerializer() { return *m_serializer; }
+      : m_serializer(&serializer), m_deserializer(nullptr),
+        m_registry(&registry){};
+  InstrumentationData(Deserializer &deserializer, Registry &registry)
+      : m_serializer(nullptr), m_deserializer(&deserializer),
+        m_registry(&registry){};
----------------
superfluous semicolons


================
Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:891-898
+      unsigned actual_id = registry.GetID(uintptr_t(&record));
+      unsigned id = deserializer.Deserialize<unsigned>();
+      registry.CheckID(id, actual_id);
+      return recorder.ReplayResult<Result>(
+          static_cast<DefaultReplayer<Result(Class *, Args...)> *>(
+              registry.GetReplayer(id))
+              ->Replay(deserializer),
----------------
All of these replay functions look very similar. Can they be factored into some method on the `Recorder` class or something. It looks like the only thing that really changes is `uintptr_t` ID of the method being replayed. That could be passed in as an extra argument.


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

https://reviews.llvm.org/D77602