[lldb] Improve completion tests

[DavidSpickett]

• 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.

[DavidSpickett]

This builds on #65945, so the first commit is from that PR. If you review using the per commit tab instead that should make it clearer, I hope.

This is me trying to imitate a stacked PR and maybe failing, let's see how it goes.

[llvmbot]

@llvm/pr-subscribers-lldb

Changes

• 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.

Full diff: https://github.com/llvm/llvm-project/pull/65973.diff

7 Files Affected:

• (modified) lldb/packages/Python/lldbsuite/test/lldbtest.py (+12-18)
• (modified) lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp (+1-1)
• (modified) lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp (+1-1)
• (modified) lldb/source/Target/Process.cpp (+2-2)
• (modified) lldb/test/API/commands/expression/completion/TestExprCompletion.py (+46-46)
• (modified) lldb/test/API/functionalities/completion/TestCompletion.py (+4-10)
• (modified) lldb/test/API/functionalities/load_unload/TestLoadUnload.py (+9)

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/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/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])
 
     # 
     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"])
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

[jimingham]

Other than that tiny quibble, this looks fine to me.