This is an archive of the discontinued LLVM Phabricator instance.

Add language command with ability to load plugin command objects.
ClosedPublic

Authored by domipheus on Apr 30 2015, 9:29 AM.

Details

Summary

Follows on from http://reviews.llvm.org/D9002.

Added language command with load, as to initially load language runtime commands and allow the ability to use existing CommandObject infrastructure. Previous discussion resulted in not having a load, but HandleCommand(char*) which simply dispatched to a LanguageRuntime. However that meant losing help functionality. Having an explicit load allows using existing CommandObjects and better overall user experience.

Added the language list output to 'help language' as to reflect current behaviour.

Diff Detail

Repository
rL LLVM

Event Timeline

domipheus updated this revision to Diff 24729.Apr 30 2015, 9:29 AM
domipheus retitled this revision from to Add language command with ability to load plugin command objects..
domipheus updated this object.
domipheus edited the test plan for this revision. (Show Details)
domipheus added reviewers: clayborg, jingham.
domipheus set the repository for this revision to rL LLVM.
domipheus added a subscriber: Unknown Object (MLST).
clayborg requested changes to this revision.Apr 30 2015, 10:17 AM
clayborg edited edge metadata.

Why do we need a "language load" command? Shouldn't we just be able to do:

(lldb) language c++ ...
(lldb) language objc ...
(lldb) language renderscript ...

Each one would try to get the LanguageRuntime from the current process and fail if they aren't available.

include/lldb/Target/LanguageRuntime.h
139 ↗(On Diff #24729)

return lldb::CommandObjectSP() here.

This revision now requires changes to proceed.Apr 30 2015, 10:17 AM

Maybe I'm missing something here. There isn't a CommandObjectProxy style object that can resolve on the fly. If there was no load command we'd need to re-execute the command if we wanted built in help.

We can use the Handle method suggested before, but without the ability to help language <languagenane> I have a feeling we'd be changing this later on anyway, for use sake.

The idea is that we have a language command whose second argument is the language name. We can probably make it so that we can get a CommandObjectSP from the LanguageRuntime static plugin callbacks (like we do with the CreateInstance static functions) so that we have the "language" CommandObject multi-word command populate its subcommands during CommandObjectLanguage::Initialize() (a static init function that would be run after all plug-ins have loaded.

Thanks for the explanation - I'll have a go at implementing it in that style and update the patch tomorrow.

Great, I look forward to seeing this new patch.

I re-read the comment and there are sill areas of confusion. Can you define 'after all plug-ins have loaded' for this? The root cause of this problem is there is no defined load point for LanguageRuntimes, before they are requested for grabbing values. That was what I was trying to solve with the explicit load command.

So my idea is to modify the static plug-in manager code that currently looks like:

bool
PluginManager::RegisterPlugin
(
    const ConstString &name,
    const char *description,
    LanguageRuntimeCreateInstance create_callback
)

to look like:

bool
PluginManager::RegisterPlugin
(
    const ConstString &name,
    const char *description,
    LanguageType language,
    LanguageRuntimeCreateInstance create_callback
    LanguageRuntimeCreateCommandObject command_callback
)

Then the LanguageRuntimeInstance structure in PluginManager.cpp would need to add fields to store "LanguageType language" and "LanguageRuntimeCreateCommandObject command_callback".

This decouples the need for an actual LanguageRuntime instance since it requires a valid process in order to exist. Then each command_callback can be NULL if the language doesn't vend a command object. Then after all of the ::Initialize() calls in SystemInitializerCommon::Initialize() and SystemInitializerFull::Initialize(), we can call the LanguageRuntime::Initialize() function so it can register any command objects with the "language" multi-word command object.

Each CommandObject in DoExecite() will need to try and grab the current language runtime from the process, if it needs the current language runtime in a process to do its job, and error out if it doesn't get it. Each language command object might be able to just look through the target images and report some information without needed an actual "LanguageRuntime*" instance. Like "c++" might be able to find where the RTTI info for a type "MyType" is by looking for mangled names in the symbol table.

Does that make more sense?

domipheus updated this revision to Diff 24889.May 4 2015, 9:29 AM
domipheus edited edge metadata.

Sorry for the delay in reply - and thanks for the further explanation. How does this look? I've tested it by modifying the renderscript language runtime to use this new method and it works well.

The xcode project will need updating to add the new command source.

clayborg accepted this revision.May 4 2015, 11:13 AM
clayborg edited edge metadata.

Very nice.

This revision is now accepted and ready to land.May 4 2015, 11:13 AM
This revision was automatically updated to reflect the committed changes.