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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to