[Lldb-commits] [lldb] [lldb] Treat user aliases the same as built-ins when tab completing (PR #65974)
David Spickett via lldb-commits
lldb-commits at lists.llvm.org
Mon Sep 11 09:10:15 PDT 2023
https://github.com/DavidSpickett created https://github.com/llvm/llvm-project/pull/65974:
Previously we would check all built-ins first for suggestions,
then check built-ins and aliases. This meant that if you had
an alias brkpt -> breakpoint, "br" would complete to "breakpoint".
Instead of giving you the choice of "brkpt" or "breakpoint".
>From f1f7de482046fd9a1347d040827005841c4d863c Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Mon, 11 Sep 2023 10:46:53 +0000
Subject: [PATCH 1/3] [lldb] Correctly invalidate unloaded image tokens
Some functions in Process were using LLDB_INVALID_ADDRESS
instead of LLDB_INVALID_TOKEN.
The only visible effect of this appears to be that
"process unload <tab>" would complete to 0 even after the
image was unloaded. Since the command is checking for
LLDB_INVALID_TOKEN.
Everything else worked somehow. I've added a check to the
existing load unload tests anyway.
The tab completion cannot be checked as is, but when I make
them more strict in a later patch it will be tested.
---
lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp | 2 +-
lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp | 2 +-
lldb/source/Target/Process.cpp | 4 ++--
.../API/functionalities/load_unload/TestLoadUnload.py | 9 +++++++++
4 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
index 1e91f2ccd198259..b4f1b76c39dbebf 100644
--- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -949,7 +949,7 @@ uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process *process,
Status PlatformPOSIX::UnloadImage(lldb_private::Process *process,
uint32_t image_token) {
const addr_t image_addr = process->GetImagePtrFromToken(image_token);
- if (image_addr == LLDB_INVALID_ADDRESS)
+ if (image_addr == LLDB_INVALID_IMAGE_TOKEN)
return Status("Invalid image token");
StreamString expr;
diff --git a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
index cbac14e2ccf7a92..88d543289a8470e 100644
--- a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
+++ b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
@@ -415,7 +415,7 @@ uint32_t PlatformWindows::DoLoadImage(Process *process,
Status PlatformWindows::UnloadImage(Process *process, uint32_t image_token) {
const addr_t address = process->GetImagePtrFromToken(image_token);
- if (address == LLDB_INVALID_ADDRESS)
+ if (address == LLDB_INVALID_IMAGE_TOKEN)
return Status("invalid image token");
StreamString expression;
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 2b0774588138881..f82ab05362fbee9 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -5911,12 +5911,12 @@ size_t Process::AddImageToken(lldb::addr_t image_ptr) {
lldb::addr_t Process::GetImagePtrFromToken(size_t token) const {
if (token < m_image_tokens.size())
return m_image_tokens[token];
- return LLDB_INVALID_ADDRESS;
+ return LLDB_INVALID_IMAGE_TOKEN;
}
void Process::ResetImageToken(size_t token) {
if (token < m_image_tokens.size())
- m_image_tokens[token] = LLDB_INVALID_ADDRESS;
+ m_image_tokens[token] = LLDB_INVALID_IMAGE_TOKEN;
}
Address
diff --git a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
index 7e8acfa3021acfc..2208e520f1d5612 100644
--- a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
+++ b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
@@ -307,6 +307,15 @@ def run_lldb_process_load_and_unload_commands(self):
patterns=["Unloading .* with index %s.*ok" % index],
)
+ # Confirm that we unloaded properly.
+ self.expect(
+ "image lookup -n a_function",
+ "a_function should not exist after unload",
+ error=True,
+ matching=False,
+ patterns=["1 match found"],
+ )
+
self.runCmd("process continue")
@expectedFailureAll(oslist=["windows"]) # breakpoint not hit
>From f7082c3bf9ee5ef68713998352566a08d92869ab Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Mon, 11 Sep 2023 09:09:31 +0000
Subject: [PATCH 2/3] [lldb] Improve completion tests
* Assert no completions for tests that should not find completions.
* Remove regex mode from complete_from_to, which was unused.
This exposed bugs in 2 of the tests, target stop-hook and
process unload. These were fixed in previous commits but
couldn't be tested properly until this patch.
---
.../Python/lldbsuite/test/lldbtest.py | 30 +++---
.../completion/TestExprCompletion.py | 92 +++++++++----------
.../completion/TestCompletion.py | 14 +--
3 files changed, 62 insertions(+), 74 deletions(-)
diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 49355d61911837f..4f8bdcd7263dc35 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2223,10 +2223,7 @@ def check_completion_with_desc(
)
self.assertFalse(got_failure, error_msg)
- def complete_exactly(self, str_input, patterns):
- self.complete_from_to(str_input, patterns, True)
-
- def complete_from_to(self, str_input, patterns, turn_off_re_match=False):
+ def complete_from_to(self, str_input, patterns):
"""Test that the completion mechanism completes str_input to patterns,
where patterns could be a pattern-string or a list of pattern-strings"""
# Patterns should not be None in order to proceed.
@@ -2254,21 +2251,18 @@ def complete_from_to(self, str_input, patterns, turn_off_re_match=False):
for idx in range(1, num_matches + 1):
compare_string += match_strings.GetStringAtIndex(idx) + "\n"
+ # If the singular pattern is the same as the input, assume that we
+ # shouldn't have any completions.
+ if len(patterns) == 1 and str_input == patterns[0] and num_matches:
+ self.fail("Expected no completions but got:\n" + compare_string)
+
for p in patterns:
- if turn_off_re_match:
- self.expect(
- compare_string,
- msg=COMPLETION_MSG(str_input, p, match_strings),
- exe=False,
- substrs=[p],
- )
- else:
- self.expect(
- compare_string,
- msg=COMPLETION_MSG(str_input, p, match_strings),
- exe=False,
- patterns=[p],
- )
+ self.expect(
+ compare_string,
+ msg=COMPLETION_MSG(str_input, p, match_strings),
+ exe=False,
+ substrs=[p],
+ )
def completions_match(self, command, completions):
"""Checks that the completions for the given command are equal to the
diff --git a/lldb/test/API/commands/expression/completion/TestExprCompletion.py b/lldb/test/API/commands/expression/completion/TestExprCompletion.py
index 3c354a3bce1a9b5..ada818989c789a1 100644
--- a/lldb/test/API/commands/expression/completion/TestExprCompletion.py
+++ b/lldb/test/API/commands/expression/completion/TestExprCompletion.py
@@ -29,34 +29,34 @@ def test_expr_completion(self):
)
# Completing member functions
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr.FooNoArgs", "expr some_expr.FooNoArgsBar()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr.FooWithArgs", "expr some_expr.FooWithArgsBar("
)
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr.FooWithMultipleArgs",
"expr some_expr.FooWithMultipleArgsBar(",
)
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr.FooUnderscore", "expr some_expr.FooUnderscoreBar_()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr.FooNumbers", "expr some_expr.FooNumbersBar1()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr.StaticMemberMethod",
"expr some_expr.StaticMemberMethodBar()",
)
# Completing static functions
- self.complete_exactly(
+ self.complete_from_to(
"expr Expr::StaticMemberMethod", "expr Expr::StaticMemberMethodBar()"
)
# Completing member variables
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr.MemberVariab", "expr some_expr.MemberVariableBar"
)
@@ -94,43 +94,43 @@ def test_expr_completion(self):
self.completions_contain("expr 1+", ["1+some_expr", "1+static_cast"])
# Test with spaces
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr .FooNoArgs", "expr some_expr .FooNoArgsBar()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr .FooNoArgs", "expr some_expr .FooNoArgsBar()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr .FooNoArgs", "expr some_expr .FooNoArgsBar()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr. FooNoArgs", "expr some_expr. FooNoArgsBar()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr . FooNoArgs", "expr some_expr . FooNoArgsBar()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr Expr :: StaticMemberMethod", "expr Expr :: StaticMemberMethodBar()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr Expr ::StaticMemberMethod", "expr Expr ::StaticMemberMethodBar()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr Expr:: StaticMemberMethod", "expr Expr:: StaticMemberMethodBar()"
)
# Test that string literals don't break our parsing logic.
- self.complete_exactly(
+ self.complete_from_to(
'expr const char *cstr = "some_e"; char c = *cst',
'expr const char *cstr = "some_e"; char c = *cstr',
)
- self.complete_exactly(
+ self.complete_from_to(
'expr const char *cstr = "some_e" ; char c = *cst',
'expr const char *cstr = "some_e" ; char c = *cstr',
)
# Requesting completions inside an incomplete string doesn't provide any
# completions.
- self.complete_exactly(
+ self.complete_from_to(
'expr const char *cstr = "some_e', 'expr const char *cstr = "some_e'
)
@@ -139,110 +139,110 @@ def test_expr_completion(self):
self.assume_no_completions("expr -i0 -- some_expr.", 11)
# Test with expr arguments
- self.complete_exactly(
+ self.complete_from_to(
"expr -i0 -- some_expr .FooNoArgs", "expr -i0 -- some_expr .FooNoArgsBar()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr -i0 -- some_expr .FooNoArgs",
"expr -i0 -- some_expr .FooNoArgsBar()",
)
# Addrof and deref
- self.complete_exactly(
+ self.complete_from_to(
"expr (*(&some_expr)).FooNoArgs", "expr (*(&some_expr)).FooNoArgsBar()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr (*(&some_expr)) .FooNoArgs", "expr (*(&some_expr)) .FooNoArgsBar()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr (* (&some_expr)) .FooNoArgs", "expr (* (&some_expr)) .FooNoArgsBar()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr (* (& some_expr)) .FooNoArgs",
"expr (* (& some_expr)) .FooNoArgsBar()",
)
# Addrof and deref (part 2)
- self.complete_exactly(
+ self.complete_from_to(
"expr (&some_expr)->FooNoArgs", "expr (&some_expr)->FooNoArgsBar()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr (&some_expr) ->FooNoArgs", "expr (&some_expr) ->FooNoArgsBar()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr (&some_expr) -> FooNoArgs", "expr (&some_expr) -> FooNoArgsBar()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr (&some_expr)-> FooNoArgs", "expr (&some_expr)-> FooNoArgsBar()"
)
# Builtin arg
- self.complete_exactly("expr static_ca", "expr static_cast")
+ self.complete_from_to("expr static_ca", "expr static_cast")
# From other files
- self.complete_exactly(
+ self.complete_from_to(
"expr fwd_decl_ptr->Hidden", "expr fwd_decl_ptr->HiddenMember"
)
# Types
- self.complete_exactly("expr LongClassNa", "expr LongClassName")
- self.complete_exactly(
+ self.complete_from_to("expr LongClassNa", "expr LongClassName")
+ self.complete_from_to(
"expr LongNamespaceName::NestedCla", "expr LongNamespaceName::NestedClass"
)
# Namespaces
- self.complete_exactly("expr LongNamespaceNa", "expr LongNamespaceName::")
+ self.complete_from_to("expr LongNamespaceNa", "expr LongNamespaceName::")
# Multiple arguments
- self.complete_exactly(
+ self.complete_from_to(
"expr &some_expr + &some_e", "expr &some_expr + &some_expr"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr SomeLongVarNameWithCapitals + SomeLongVarName",
"expr SomeLongVarNameWithCapitals + SomeLongVarNameWithCapitals",
)
- self.complete_exactly(
+ self.complete_from_to(
"expr SomeIntVar + SomeIntV", "expr SomeIntVar + SomeIntVar"
)
# Multiple statements
- self.complete_exactly(
+ self.complete_from_to(
"expr long LocalVariable = 0; LocalVaria",
"expr long LocalVariable = 0; LocalVariable",
)
# Custom Decls
- self.complete_exactly(
+ self.complete_from_to(
"expr auto l = [](int LeftHandSide, int bx){ return LeftHandS",
"expr auto l = [](int LeftHandSide, int bx){ return LeftHandSide",
)
- self.complete_exactly(
+ self.complete_from_to(
"expr struct LocalStruct { long MemberName; } ; LocalStruct S; S.Mem",
"expr struct LocalStruct { long MemberName; } ; LocalStruct S; S.MemberName",
)
# Completing function call arguments
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr.FooWithArgsBar(some_exp",
"expr some_expr.FooWithArgsBar(some_expr",
)
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr.FooWithArgsBar(SomeIntV",
"expr some_expr.FooWithArgsBar(SomeIntVar",
)
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr.FooWithMultipleArgsBar(SomeIntVar, SomeIntVa",
"expr some_expr.FooWithMultipleArgsBar(SomeIntVar, SomeIntVar",
)
# Function return values
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr.Self().FooNoArgs", "expr some_expr.Self().FooNoArgsBar()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr.Self() .FooNoArgs", "expr some_expr.Self() .FooNoArgsBar()"
)
- self.complete_exactly(
+ self.complete_from_to(
"expr some_expr.Self(). FooNoArgs", "expr some_expr.Self(). FooNoArgsBar()"
)
diff --git a/lldb/test/API/functionalities/completion/TestCompletion.py b/lldb/test/API/functionalities/completion/TestCompletion.py
index cc2a20dcd0dca76..8898bf2e2b5da9e 100644
--- a/lldb/test/API/functionalities/completion/TestCompletion.py
+++ b/lldb/test/API/functionalities/completion/TestCompletion.py
@@ -237,16 +237,13 @@ def test_log_dir(self):
src_dir = os.path.dirname(os.path.realpath(__file__))
self.complete_from_to(
"log enable lldb expr -f " + src_dir,
- [src_dir + os.sep],
- turn_off_re_match=True,
- )
+ [src_dir + os.sep])
# <rdar://problem/11052829>
def test_infinite_loop_while_completing(self):
"""Test that 'process print hello\' completes to itself and does not infinite loop."""
self.complete_from_to(
- "process print hello\\", "process print hello\\", turn_off_re_match=True
- )
+ "process print hello\\", "process print hello\\")
def test_watchpoint_co(self):
"""Test that 'watchpoint co' completes to 'watchpoint command '."""
@@ -727,9 +724,7 @@ def test_symbol_name(self):
self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
self.complete_from_to(
"breakpoint set -n Fo",
- "breakpoint set -n Foo::Bar(int,\\ int)",
- turn_off_re_match=True,
- )
+ "breakpoint set -n Foo::Bar(int,\\ int)")
# No completion for Qu because the candidate is
# (anonymous namespace)::Quux().
self.complete_from_to("breakpoint set -n Qu", "")
@@ -822,8 +817,7 @@ def test_common_completion_target_stophook_ids(self):
for subcommand in subcommands:
self.complete_from_to(
"target stop-hook " + subcommand + " 1 ",
- "target stop-hook " + subcommand + " 1 ",
- )
+ "target stop-hook " + subcommand + " 1 ")
def test_common_completion_type_language(self):
self.complete_from_to("type category -l ", ["c"])
>From 085393af7b13ffd9eac87ac28788663f22dfe7cf Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Mon, 11 Sep 2023 11:42:45 +0000
Subject: [PATCH 3/3] [lldb] Treat user aliases the same as built-ins when tab
completing
Previously we would check all built-ins first for suggestions,
then check built-ins and aliases. This meant that if you had
an alias brkpt -> breakpoint, "br" would complete to "breakpoint".
Instead of giving you the choice of "brkpt" or "breakpoint".
---
.../source/Interpreter/CommandInterpreter.cpp | 35 +++----------------
.../abbreviation/TestAbbreviations.py | 2 +-
.../completion/TestCompletion.py | 18 ++++------
3 files changed, 13 insertions(+), 42 deletions(-)
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index 6d1ad799f2d10fb..fc94bf6fbfe117c 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1227,36 +1227,11 @@ CommandObject *
CommandInterpreter::GetCommandObject(llvm::StringRef cmd_str,
StringList *matches,
StringList *descriptions) const {
- CommandObject *command_obj =
- GetCommandSP(cmd_str, false, true, matches, descriptions).get();
-
- // If we didn't find an exact match to the command string in the commands,
- // look in the aliases.
-
- if (command_obj)
- return command_obj;
-
- command_obj = GetCommandSP(cmd_str, true, true, matches, descriptions).get();
-
- if (command_obj)
- return command_obj;
-
- // If there wasn't an exact match then look for an inexact one in just the
- // commands
- command_obj = GetCommandSP(cmd_str, false, false, nullptr).get();
-
- // Finally, if there wasn't an inexact match among the commands, look for an
- // inexact match in both the commands and aliases.
-
- if (command_obj) {
- if (matches)
- matches->AppendString(command_obj->GetCommandName());
- if (descriptions)
- descriptions->AppendString(command_obj->GetHelp());
- return command_obj;
- }
-
- return GetCommandSP(cmd_str, true, false, matches, descriptions).get();
+ // Try to find a match among commands and aliases. Allowing inexact matches,
+ // but perferring exact matches.
+ return GetCommandSP(cmd_str, /*include_aliases=*/true, /*exact=*/false,
+ matches, descriptions)
+ .get();
}
CommandObject *CommandInterpreter::GetUserCommandObject(
diff --git a/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py b/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py
index 10431e41dc81a2e..cade8d87e7f76f5 100644
--- a/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py
+++ b/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py
@@ -20,7 +20,7 @@ def test_command_abbreviations_and_aliases(self):
self.assertTrue(result.Succeeded())
self.assertEqual("apropos script", result.GetOutput())
- command_interpreter.ResolveCommand("h", result)
+ command_interpreter.ResolveCommand("he", result)
self.assertTrue(result.Succeeded())
self.assertEqual("help", result.GetOutput())
diff --git a/lldb/test/API/functionalities/completion/TestCompletion.py b/lldb/test/API/functionalities/completion/TestCompletion.py
index 8898bf2e2b5da9e..fd1db08b9995c88 100644
--- a/lldb/test/API/functionalities/completion/TestCompletion.py
+++ b/lldb/test/API/functionalities/completion/TestCompletion.py
@@ -621,19 +621,15 @@ def test_command_unalias(self):
def test_command_aliases(self):
self.runCmd("command alias brkpt breakpoint")
- # If there is an unambiguous completion from the built-in commands,
- # we choose that.
- self.complete_from_to("br", "breakpoint")
- # Only if there is not, do we then look for an unambiguous completion
- # from the user defined aliases.
+ # Exact matches are chosen if possible, even if there are longer
+ # completions we could use.
+ self.complete_from_to("b", "b ")
+ # Aliases are included in possible completions.
+ self.complete_from_to("br", ["breakpoint", "brkpt"])
+ # An alias can be the chosen completion.
self.complete_from_to("brk", "brkpt")
- # Aliases are included when there's no exact match.
- self.runCmd("command alias play breakpoint")
- self.complete_from_to("pl", ["plugin", "platform", "play"])
-
- # That list can also contain only aliases if there's no built-ins to
- # match.
+ # The list can contain only aliases if there's no built-ins to match.
self.runCmd("command alias test_1 breakpoint")
self.runCmd("command alias test_2 breakpoint")
self.complete_from_to("test_", ["test_1", "test_2"])
More information about the lldb-commits
mailing list