[Lldb-commits] [PATCH] D59427: [WIP] [lldb] [API] Split SBRegistry into smaller files

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I like the fact that we're moving the register methods into the respective class files. Among other things, this should make it easier for the instrumentation tool to insert register calls as well. However, I am not fond of introducing a public SB API call for something that should really be a private matter. One way to fix that would be to make the `Initialize` function private and make the SBReproducer class a friend, but perhaps it would be even better to not put this function into the public headers at all.

One way to achieve that would be to forward-declare some template function in SBReproducerPrivate.h (or maybe even a completely new header). Something like this ought to do the trick:

  namespace lldb_private { namespace repro {
  template<typename SB> void RegisterMethods(Registry &R);
  }}

Then each cpp file could specialize this function to do what it needs to do:

  namespace lldb_private { namespace repro {
  template<> void RegisterMethods<SBWhatever>(Registry &R) {
  LLDB_REGISTER_FOO(...);
  ...
  }
  }}

The reproducer initialization would become just a collection of `RegisterMethod<SBFoo>()` calls, and no public API would be affected.

BTW: I believe the compiler errors are down to the fact that the REGISTER_FOO macros assume they are called from the context of `Registry` class. If we go down this path, then this will no longer be true, and the macros will need to be adjusted slightly (e.g. to take the registry as an extra argument or something). It technically is possible for SBFoo.cpp to implement a method belonging to the Registry class (which would avoid needing to modify the macro), but this would probably be just confusing.


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

https://reviews.llvm.org/D59427