Page MenuHomePhabricator

[LanguageRuntime] Move CPPLanguageRuntime into a plugin
ClosedPublic

Authored by xiaobai on Thu, Jul 11, 2:15 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

xiaobai created this revision.Thu, Jul 11, 2:15 PM
JDevlieghere added inline comments.Thu, Jul 11, 2:22 PM
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
55 ↗(On Diff #209331)

What's the benefit of making this a separate plugin, as compared to making it part of Plugins/Language/CPlusPlus?

jingham requested changes to this revision.Thu, Jul 11, 2:25 PM

My model for this was that there was a CPPLanguageRuntime.cpp that contains everything you can implement about the CPP runtime that is independent of any particular implementation of the CPP language runtime, and then a plugin instance (in this case the ItaniumABILanguageRuntime) that contains all the bits that are specific to a particular implementation. Then putting the CPPLanguageRuntime.cpp in Target lets you know that this has to only contain the generic parts of the runtime behavior. That still seems to me a useful distinction.

This revision now requires changes to proceed.Thu, Jul 11, 2:25 PM
xiaobai marked an inline comment as done.Thu, Jul 11, 2:26 PM
xiaobai added inline comments.
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
55 ↗(On Diff #209331)

I view LanguageRuntimes as distinct from Languages and thus I think they should go into their own plugins. However, I'm not against moving this to Plugins/Language/CPlusPlus if you think it would make more sense to do so for another reason (e.g. less plugins overall?)

What is the indented relationship between CPPLanguage and CPPLanguageRuntime plugins (and generally between any Language and its LanguageRuntime)? Right now you're having the CPPLanguage depend on the CPPLanguageRuntime plugin. There is no reverse dependency, so this may be fine, if that's how we intend things to be. However, it's not clear to me whether that's the right way to organize things. Intuitively, I'd expect the LanguageRuntime to depend on Language, and not the other way around...

compnerd added inline comments.Thu, Jul 11, 2:28 PM
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
55 ↗(On Diff #209331)

We do need the abstraction since there are multiple C++ runtimes: c++, stdc++, MSVCPRT, stlport, etc. Each one is slightly different. Furthermore, libstdc++ supported the GNU and Solaris ABIs, libc++ only does itanium, MSVCPRT only does MSVC ABI. So, we need to have some layer to differentiate between the various ABIs and just general C++ language support.

My model for this was that there was a CPPLanguageRuntime.cpp that contains everything you can implement about the CPP runtime that is independent of any particular implementation of the CPP language runtime, and then a plugin instance (in this case the ItaniumABILanguageRuntime) that contains all the bits that are specific to a particular implementation. Then putting the CPPLanguageRuntime.cpp in Target lets you know that this has to only contain the generic parts of the runtime behavior. That still seems to me a useful distinction.

I think that's fine, but it does not conflict the idea of the generic parts of a cpp language runtime being a plugin also. This way the generic parts of lldb would be truly language agnostic. Then if anything wants to work with a c++ runtime, but it does not care which one, it can use CPPLanguageRuntime. And if you need a very specific runtime, you go for the Itanium thingy.

labath added inline comments.Thu, Jul 11, 2:37 PM
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
55 ↗(On Diff #209331)

That is true. However, I'm not sure whether the current boundary actually makes sense. E.g. the c++ language plugin implements pretty printers for both libc++ and libstdc++.

What is the indented relationship between CPPLanguage and CPPLanguageRuntime plugins (and generally between any Language and its LanguageRuntime)? Right now you're having the CPPLanguage depend on the CPPLanguageRuntime plugin. There is no reverse dependency, so this may be fine, if that's how we intend things to be. However, it's not clear to me whether that's the right way to organize things. Intuitively, I'd expect the LanguageRuntime to depend on Language, and not the other way around...

The nominal distinction is that the Language files contain what you can know about or need to do with a given language without having a process and thus a live runtime to query. The LanguageRuntimes are supposed to be about what you can gather from the actual runtime, or what you need to do to handle things like "stepping into steps across ObjC method dispatch functions" or "stepping into std::function steps in to the target function". Given this distinction you'd actually expect the two to be independent of one another, but then it turns out that in both ObjC and Swift, you can't actually know the sizes of objects, or the offsets of ivar members until you have a running process. So things you'd think the Language would know actually require the LanguageRuntime...

xiaobai marked an inline comment as done.Thu, Jul 11, 3:13 PM

My model for this was that there was a CPPLanguageRuntime.cpp that contains everything you can implement about the CPP runtime that is independent of any particular implementation of the CPP language runtime, and then a plugin instance (in this case the ItaniumABILanguageRuntime) that contains all the bits that are specific to a particular implementation. Then putting the CPPLanguageRuntime.cpp in Target lets you know that this has to only contain the generic parts of the runtime behavior. That still seems to me a useful distinction.

I think that's fine, but it does not conflict the idea of the generic parts of a cpp language runtime being a plugin also. This way the generic parts of lldb would be truly language agnostic. Then if anything wants to work with a c++ runtime, but it does not care which one, it can use CPPLanguageRuntime. And if you need a very specific runtime, you go for the Itanium thingy.

+1

What is the indented relationship between CPPLanguage and CPPLanguageRuntime plugins (and generally between any Language and its LanguageRuntime)? Right now you're having the CPPLanguage depend on the CPPLanguageRuntime plugin. There is no reverse dependency, so this may be fine, if that's how we intend things to be. However, it's not clear to me whether that's the right way to organize things. Intuitively, I'd expect the LanguageRuntime to depend on Language, and not the other way around...

The nominal distinction is that the Language files contain what you can know about or need to do with a given language without having a process and thus a live runtime to query. The LanguageRuntimes are supposed to be about what you can gather from the actual runtime, or what you need to do to handle things like "stepping into steps across ObjC method dispatch functions" or "stepping into std::function steps in to the target function". Given this distinction you'd actually expect the two to be independent of one another, but then it turns out that in both ObjC and Swift, you can't actually know the sizes of objects, or the offsets of ivar members until you have a running process. So things you'd think the Language would know actually require the LanguageRuntime...

I also think that Language depending on LanguageRuntime makes sense. It seems to me that in order for the Language to be able to answer questions about implementation-defined features, it must ask the language runtime for help.

source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
55 ↗(On Diff #209331)

Given that those are formatters specific to the libc++ and libstdc++ implementations, I think it would make sense that those are a part of the C++ language runtime plugin(s).

My model for this was that there was a CPPLanguageRuntime.cpp that contains everything you can implement about the CPP runtime that is independent of any particular implementation of the CPP language runtime, and then a plugin instance (in this case the ItaniumABILanguageRuntime) that contains all the bits that are specific to a particular implementation. Then putting the CPPLanguageRuntime.cpp in Target lets you know that this has to only contain the generic parts of the runtime behavior. That still seems to me a useful distinction.

I think that's fine, but it does not conflict the idea of the generic parts of a cpp language runtime being a plugin also. This way the generic parts of lldb would be truly language agnostic. Then if anything wants to work with a c++ runtime, but it does not care which one, it can use CPPLanguageRuntime. And if you need a very specific runtime, you go for the Itanium thingy.

I see where you are going...

Im my head, the plugin hierarchy for runtimes goes LanguageRuntime -> CPPLanguageRuntime -> ItaniumABILanguageRuntime. I've been calling all those "parts of the plugin for language runtimes". LanguageRuntime is the generic plugin interface, and currently CPPLanguageRuntime (and ObjCLanguageRuntime...) are also being treated as "generic parts of the LanguageRuntime plugin". Then ItaniumABILanguageRuntime.h is an instance of the plugin. Then the generic plugin interface files live out of the Plugin hierarchy and are free game, and the instances are in Plugins and should not be included from generic code.

Anyway, if you can make all the generic parts of lldb not depend on the language specific - but still abstract - part of the plugin, that would be fine. Then just LanguageRuntime.h would live in Target, and CPPLanguageRuntime would live in Plugins/Language

CPPLanguageRuntime is really only used in other plugins, so it's easy. ObjCLanguageRuntime has a few more uses in generic code, and the SwiftLanguageRuntime has some more uses in generic code (included in 16 generic files). That corresponds to the fact that ObjC and the Swift are systems that are increasingly more dependent on their runtimes in order for the debugger to comprehend them. Presumably we could fix this by adding some more abstract methods to LanguageRuntime, though I'd have to do some more investigation to see how awkward that would be, particularly for swift...

It doesn't seem to me useful to move CPPLanguageRuntime to the instance-specific part of LanguageRuntime, if we can't also do that for ObjC and Swift. That's just confusing. But if as a proof of concept you can get ObjCLanguageRuntime.h out of generic code, then presumably we can also do that for Swift, and the project seems worthwhile.

jingham added inline comments.Thu, Jul 11, 3:32 PM
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
55 ↗(On Diff #209331)

So the problem with that is that formatters don't actually require a live process to be useful. They can be used to view static data in a not-running process. But LanguageRuntimes currently only come from a process. So to move the formatters to the LanguageRuntime - which does make some sense - you're going to have to change the LanguageRuntime to do some things for a target with no process and the more things when the Process gets a target. That's not a bad change, BTW.

Anyway, that's why the formatters are a little out-of-place.

Anyway, if you can make all the generic parts of lldb not depend on the language specific - but still abstract - part of the plugin, that would be fine. Then just LanguageRuntime.h would live in Target, and CPPLanguageRuntime would live in Plugins/Language

This is exactly where I'm trying to take things and it's why I've been shuffling things around and making things more generic.

CPPLanguageRuntime is really only used in other plugins, so it's easy. ObjCLanguageRuntime has a few more uses in generic code, and the SwiftLanguageRuntime has some more uses in generic code (included in 16 generic files). That corresponds to the fact that ObjC and the Swift are systems that are increasingly more dependent on their runtimes in order for the debugger to comprehend them. Presumably we could fix this by adding some more abstract methods to LanguageRuntime, though I'd have to do some more investigation to see how awkward that would be, particularly for swift...

ObjCLanguageRuntime has 3 more uses in "base lldb" (non-plugin libraries) that I'm trying to get rid of.

  • Core/ValueObject.cpp: D64159
  • Expression/IRDynamicChecks.cpp: D64591
  • Symbol/ClangASTContext.cpp: I don't plan on removing this one. Instead, I want to move ClangASTContext out of Symbol into a plugin.

After the first 2 are moved out, I intend on moving ObjCLanguageRuntime out as well. I'm also working on SwiftLanguageRuntime in the swift-lldb repository, although a little more slowly as I wanted to get C++ and ObjC done first.

Also, as an aside, I intend on trying to get swift debugging support into llvm.org at some point. I don't think llvm.org is in a place where that can be done cleanly, and I think swift-lldb's swift-specific bits need to be pulled into plugins before it becomes a reasonable endeavor. But that's the direction I want to head in. I feel similarly about Rust support. :)

It doesn't seem to me useful to move CPPLanguageRuntime to the instance-specific part of LanguageRuntime, if we can't also do that for ObjC and Swift. That's just confusing. But if as a proof of concept you can get ObjCLanguageRuntime.h out of generic code, then presumably we can also do that for Swift, and the project seems worthwhile.

I uploaded this first to get some feedback and introduce the idea of moving CPPLanguageRuntime, ObjCLanguageRuntime, and SwiftLanguageRuntime. I figured I'd get any issues out of the way with the easiest one.

source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
55 ↗(On Diff #209331)

Right this makes sense to me. I think we could figure something out here, as I've dealt with needing access to LanguageRuntimes without a process. I think something can be done, but that can be left for another time I think.

jingham accepted this revision.Thu, Jul 11, 4:00 PM

Okay... Provided we can come up with not too torturous ways to get the ObjC and Swift support out of generic-code, it seems okay to do this as a first step. I just don't want to end up in the state where we have ObjCLanguageRuntime as generic but CPPLanguageRuntime is not.

This revision is now accepted and ready to land.Thu, Jul 11, 4:00 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFri, Jul 12, 1:10 PM