[Lldb-commits] [PATCH] D78141: [lldb/Reproducers] Simplify LLDB_RECORD macros

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Apr 15 01:02:53 PDT 2020

labath added a comment.

This looks even better than I hoped. I think this is a worthwhile simplification even without the followup patches. Just a couple of questions inline...

Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:117-135
 #define LLDB_RECORD_CONSTRUCTOR(Class, Signature, ...)                         \
   lldb_private::repro::Recorder _recorder(LLVM_PRETTY_FUNCTION,                \
                                           stringify_args(__VA_ARGS__));        \
   if (lldb_private::repro::InstrumentationData _data =                         \
           LLDB_GET_INSTRUMENTATION_DATA()) {                                   \
     _recorder.Record(_data.GetSerializer(), _data.GetRegistry(),               \
                      &lldb_private::repro::construct<Class Signature>::doit,   \
Could you merge these two in a similar way as well (not with the method macros, just with themselves)? I know that the constructors could be implemented as a function in the other patch, but I have a feeling it will be pretty confusing if two very similar functionalities were implemented in completely different ways...

Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:137
-#define LLDB_RECORD_METHOD(Result, Class, Method, Signature, ...)              \
+#define LLDB_RECORD_(Method, T1, T2, ...)                                      \
   lldb_private::repro::Recorder _recorder(LLVM_PRETTY_FUNCTION,                \
idea: Should we standardize the name of the inner class and drop the `Method` argument?

Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:757-760
+  void Record(Serializer &serializer, Registry &registry, void (*f)(),
+              const EmptyArg &arg) {
+    Record(serializer, registry, f);
+  }
Why the void overload? It looks like the templated version would work just fine for void too...



More information about the lldb-commits mailing list