RFC: Namespaces in lldb

Hi All,

In lldb I see that most of the classes are leaving in the lldb or in the lldb_private namespace but we have some class in the global scope. What is the rule we want to use about placing our classes in namespaces? When we want to put a class into lldb, when we want to put it into lldb_private and when we want to leave it in the global scope (do we want to leave anything in the global namespace)?

The current stage is that we have the classes in “lldb/API” and a few other files in the lldb namespace, a lot of thing in the lldb_private namespace and some class in the global namespace. As a first approximation I would suggest to keep the classes in “lldb/API” in the lldb namespace and everything else in lldb_private but it might not be sufficient in all cases.

I think this issue doesn’t cause us any major problem right now, so I don’t plan to move every class to the proper namespace with a few commits but I hope we can make an agreement about what is the preferred location for the different classes and then make progress to move them to the right place (e.g.: when somebody do bigger changes to a class what is not in the correct namespace he/she can also move it to the right place).

Tamas

My take:

Local classes that are in just in .cpp files should go in the anonymous namespace. We didn't start out doing it that way, not all local classes follows this rule. Thats just a bug, feel free to fix that wherever you come across it. Everything else should be in lldb or lldb_private. Anything that might be useful to for the SB API's but is also useful for core lldb should be in the lldb namespace. Everything else should be in lldb_private.

I don't think we need to split it finer than that at present. I don't see any reason to leave any classes in the global namespace, but again I think where we are doing that it is just because we didn't start out putting all our local classes in the anonymous namespace.

Jim

My take:

Local classes that are in just in .cpp files should go in the anonymous namespace. We didn't start out doing it that way, not all local classes follows this rule. Thats just a bug, feel free to fix that wherever you come across it. Everything else should be in lldb or lldb_private. Anything that might be useful to for the SB API's but is also useful for core lldb should be in the lldb namespace. Everything else should be in lldb_private.

Note, BTW, if you fix some local class to put it in the anonymous namespace, be sure to follow the LLVM rules for classes in anonymous namespaces (put only the declarations in the anonymous namespace, and put all method bodies outside the namespace {} construct.)

Jim

My take on namespaces:

- anything in "lldb" namespace will and should be exported for use in the public API
- nothing in the lldb_private namespace should appear in the public API (no mentions in the public API headers)
- all LLDB internal classes should be in the lldb_private namespace
- all plug-ins really should make up a namespace for each individual plug-in, but they currently don't and they use the global namespace
- Any classes defined solely in a .cpp file should be in an anonymous namespace

So the global namespace is probably getting a bit polluted in LLDB, but none of that is exported so it only affects LLDB internally. We should at some point start converting plug-ins over to using a namespace for each plug-in so we can avoid the global namespace, but there is no immediate need unless someone just has free time.

Greg

There's one advantage of doing at least a subset of plugins now
though: we have a growing set of new developers working on LLDB, and I
expect we'll see even more in the future. It would be good to have a
few examples of best practices for plug-ins.

As the core matures and LLDB becomes a viable debugger for a growing
set of targets I suspect we'll see new contributors who primarily
interact with the LLDB project by developing a small set of plug-ins
for their specific platform.

So the global namespace is probably getting a bit polluted in LLDB, but none of that is exported so it only affects LLDB internally. We should at some point start converting plug-ins over to using a namespace for each plug-in so we can avoid the global namespace, but there is no immediate need unless someone just has free time.

There's one advantage of doing at least a subset of plugins now
though: we have a growing set of new developers working on LLDB, and I
expect we'll see even more in the future. It would be good to have a
few examples of best practices for plug-ins.

Agreed.

As the core matures and LLDB becomes a viable debugger for a growing
set of targets I suspect we'll see new contributors who primarily
interact with the LLDB project by developing a small set of plug-ins
for their specific platform.

Yep. I won't be able to get to this anytime soon, but will be happy to quickly approve patches that implement this.

Greg

Jim,
The initial developers of lldb-mi perhaps did not want to use the classes from
lldb_private inside lldb-mi. So in some cases, they sort of duplicated the code
for things like Mutex in lld-mi which are already available in host layer. If there
are no objections on using lldb_private inside lldb-mi then I can remove that
code duplication.

Regards,
Abid

Please just switch to using C++11 for the host layer stuff (std::mutex, std::condition, etc). Don't expose or copy any code. Nothing from lldb_private should be used in any code that links against the LLDB shared library. If you want internal stuff, you should switch over to be a tool that links against the internal lldb_private stuff and not use the lldb::SB API.

And I do prefer that we try and use the lldb::SB API for lldb-mi.

Greg

Thanks for all of the comments. I plan to do the move for Platform/gdb-remote, platform/Android, Platform/Linux, Process/gdb-remote, Process/Linux within a week and leave the rest of the classes for now, but possibly move them in the future if my time permits.

Tamas