-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[lldb] Expose language plugin commands based based on language of current frame #136766
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[lldb] Expose language plugin commands based based on language of current frame #136766
Conversation
@llvm/pr-subscribers-lldb Author: Dave Lee (kastiglione) ChangesFull diff: https://github.com/llvm/llvm-project/pull/136766.diff 2 Files Affected:
diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 724d88d65f6ac..26e0767951e7f 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -730,6 +730,12 @@ class CommandInterpreter : public Broadcaster,
bool EchoCommandNonInteractive(llvm::StringRef line,
const Flags &io_handler_flags) const;
+ /// Return the language specific command object for the current frame.
+ ///
+ /// For example, when stopped on a C++ frame, this returns the command object
+ /// for "language cplusplus" (`CommandObjectMultiwordItaniumABI`).
+ lldb::CommandObjectSP GetFrameLanguageCommand() const;
+
// A very simple state machine which models the command handling transitions
enum class CommandHandlingState {
eIdle,
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index eb4741feb0aa5..2ff02ae5086b4 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1018,6 +1018,26 @@ CommandInterpreter::VerifyUserMultiwordCmdPath(Args &path, bool leaf_is_command,
return cur_as_multi;
}
+CommandObjectSP CommandInterpreter::GetFrameLanguageCommand() const {
+ if (auto frame_sp = GetExecutionContext().GetFrameSP()) {
+ auto frame_language = Language::GetPrimaryLanguage(
+ frame_sp->GuessLanguage().AsLanguageType());
+
+ auto it = m_command_dict.find("language");
+ if (it != m_command_dict.end()) {
+ // The root "language" command.
+ CommandObjectSP language_cmd_sp = it->second;
+
+ if (auto *plugin = Language::FindPlugin(frame_language)) {
+ // "cplusplus", "objc", etc.
+ auto lang_name = plugin->GetPluginName();
+ return language_cmd_sp->GetSubcommandSPExact(lang_name);
+ }
+ }
+ }
+ return {};
+}
+
CommandObjectSP
CommandInterpreter::GetCommandSP(llvm::StringRef cmd_str, bool include_aliases,
bool exact, StringList *matches,
@@ -1050,11 +1070,20 @@ CommandInterpreter::GetCommandSP(llvm::StringRef cmd_str, bool include_aliases,
command_sp = pos->second;
}
+ // The `language` subcommand ("language objc", "language cplusplus", etc).
+ CommandObjectMultiword *lang_subcmd = nullptr;
+ if (!command_sp) {
+ if (auto subcmd_sp = GetFrameLanguageCommand()) {
+ lang_subcmd = subcmd_sp->GetAsMultiwordCommand();
+ command_sp = subcmd_sp->GetSubcommandSPExact(cmd_str);
+ }
+ }
+
if (!exact && !command_sp) {
// We will only get into here if we didn't find any exact matches.
CommandObjectSP user_match_sp, user_mw_match_sp, alias_match_sp,
- real_match_sp;
+ real_match_sp, lang_match_sp;
StringList local_matches;
if (matches == nullptr)
@@ -1064,6 +1093,7 @@ CommandInterpreter::GetCommandSP(llvm::StringRef cmd_str, bool include_aliases,
unsigned int num_alias_matches = 0;
unsigned int num_user_matches = 0;
unsigned int num_user_mw_matches = 0;
+ unsigned int num_lang_matches = 0;
// Look through the command dictionaries one by one, and if we get only one
// match from any of them in toto, then return that, otherwise return an
@@ -1121,11 +1151,28 @@ CommandInterpreter::GetCommandSP(llvm::StringRef cmd_str, bool include_aliases,
user_mw_match_sp = pos->second;
}
+ if (lang_subcmd) {
+ num_lang_matches =
+ AddNamesMatchingPartialString(lang_subcmd->GetSubcommandDictionary(),
+ cmd_str, *matches, descriptions);
+ }
+
+ if (num_lang_matches == 1) {
+ cmd.assign(matches->GetStringAtIndex(num_cmd_matches + num_alias_matches +
+ num_user_matches +
+ num_user_mw_matches));
+
+ auto &lang_dict = lang_subcmd->GetSubcommandDictionary();
+ auto pos = lang_dict.find(cmd);
+ if (pos != lang_dict.end())
+ lang_match_sp = pos->second;
+ }
+
// If we got exactly one match, return that, otherwise return the match
// list.
if (num_user_matches + num_user_mw_matches + num_cmd_matches +
- num_alias_matches ==
+ num_alias_matches + num_lang_matches ==
1) {
if (num_cmd_matches)
return real_match_sp;
@@ -1133,8 +1180,10 @@ CommandInterpreter::GetCommandSP(llvm::StringRef cmd_str, bool include_aliases,
return alias_match_sp;
else if (num_user_mw_matches)
return user_mw_match_sp;
- else
+ else if (num_user_matches)
return user_match_sp;
+ else
+ return lang_match_sp;
}
} else if (matches && command_sp) {
matches->AppendString(cmd_str);
|
Placed in draft state while I write tests. |
This sounds like a nice way to save some keystrokes, but I wonder how it could go wrong and what we should do about it. What if a language plugin adds a command that conflicts with a top-level command? Perhaps somebody has a downstream fork that introduces a command like Similarly, if you're in a frame where we can detect the language, maybe you step into a frame you don't have debug info or symbols for. The commands you were using before now require the full I don't think these are hard blockers, but I think we should think through these scenarios carefully. Many of our users are opinionated about how commands should work ("This breaks my python scripts!"), and even more users may not understand the conditionality of this feature ("I typed this command before, why does it say there's no such command now?") |
@bulbazord the top-level command will be used. This is a fallback, for when a command is entered for which no top-level command exists. If a language plugin adds a subcommand that conflicts with a top-level command, then the only way to run it will be by explicitly running I'm not aware of any conflicts today, and given the relatively slow rate at which language plugins add new commands, I don't see this as a problem in practice. |
@bulbazord one potential solution to this is to use the top-most stack frame for which a language is known. |
@bulbazord I agree. I don't think this should break anything. |
I'll have to check this tomorrow, but the current implementation might have this possible flaw: Let's say there's both a custom python command named |
I agree with Dave & Alex, we should only promote the lang commands to top-level commands if there are no conflicts with the command the user typed. So I also think, since especially if we start making the choice algorithm less obvious (e.g. first frame on the stack for which we "know" the language) we need to somehow tell the user which language we chose for them:
or something like that. I don't think using the status line is appropriate for this because GUI's won't in general show the status line. |
f4f687f
to
4546da0
Compare
1caa020
to
4546da0
Compare
✅ With the latest revision this PR passed the Python code formatter. |
I've updated the implementation to not resolve to a language specific command whenever the entered command prefix matches a top level command. In other words, given a python command Given how few language specific commands there are (currently 6 total), I don't see this as causing problems:
|
I have not implemented the mentioned idea of using the top frame that has a known language. I think that situation would be rare, and makes this more magic. I think it's better to wait for feedback then to add implement something that users may not need/want. |
The code looks fine but this would be super confusing if you don't know what is happening here, so we have to document it somehow. We can release note it but a lot of people don't read release notes. We probably should put something in the Tutorial, which discusses command resolution, though not a lot of folks read that either from what I can tell. We should add something to the If would be really cool if when I'm in a C++ frame and I did: (lldb) help demangle I would get a preface like: Chosen from 'language cplusplus demangle' because the current frame's language is cplusplus. |
7f93043
to
6180b95
Compare
@jimingham I've added 2f91d3f which includes a long help for |
@bulbazord note that knowing the language of a frame does not depend on debug info, a symbol alone can be used to identify its language based on its mangling. |
I plan to do this in a follow up. Note that |
…rent frame (llvm#136766) Use the current frame's language to lookup commands provided by language plugins. This means commands like `language {objc,cplusplus} <command>` can be used directly, without using the `language <lang>` prefix. For example, when stopped on a C++ frame, `demangle _Z1fv` will run `language cplusplus demangle _Z1fv`. rdar://149882520 (cherry picked from commit 7ed185a)
The following buildbots are broken
https://lab.llvm.org/buildbot/#/builders/195/builds/9686
Please fix it ASAP. |
Use the current frame's language to lookup commands provided by language plugins.
This means commands like
language {objc,cplusplus} <command>
can be used directly, without using thelanguage <lang>
prefix.For example, when stopped on a C++ frame,
demangle _Z1fv
will runlanguage cplusplus demangle _Z1fv
.rdar://149882520