[Lldb-commits] [PATCH] D73125: [lldb/Plugins] Move entry points out of plugin namespace

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Some of these plugins are very simple (a single class) and so having the namespace there is not that useful. However, for the not-so-simple plugins, it seems strange to have one part of it be in the namespace, and one part outside. E.g., it's unfortunate that that ProcessGDBRemote now has to qualify all references to ThreadGDBRemote, as they are both coupled closely together.

Technically, the ProcessGDBRemote class is not really the "entry point" into "gdb" plugin -- only its "initialize" function is. Nothing else should access the ProcessGDBRemote class directly (except through its base class).

In this sense, it was a mistake to have the "initalize" function be a (static) method. It makes it impossible to forward-declare it, and that means that anything which includes this file gets exposed to whatever ProcessGDBRemote.h includes. IIUC, this was the main reason we created the ScriptInterpreterPythonImpl class (which means that now ScriptInterpreterPython::Initialize references ScriptInterpreterPythonImpl::CreateInstance). Apart from python there are other plugins that could potentially cause problems due to 3rd party dependencies and their prolific uses of macros, and which might benefit from additional compartmentalization.

It seems to me like this would be a good opportunity to solve both of these issues and move the Initialize functions out of their classes (or just create a function that forwards to these). Those functions can be in the lldb_private namespace and have fully predictable names. They are also easily forward-declarable, so we don't need to generate the `#include` directives. And that in turn means we don't have to rename everything to make things super-consistent. And the added benefits are that the plugins are fully self-contained and don't have to do second-level pimpling just to avoid namespace polution.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D73125