teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.
Thanks for the patch! As Jonas pointed out, tests are usually expected for
these kind of patches. But the IOHandler logic has some really quirky way to
write tests, so just went ahead and wrote one that you can just apply on top:
diff --git a/lldb/test/API/iohandler/completion/Makefile
b/lldb/test/API/iohandler/completion/Makefile
new file mode 100644
index 000000000000..10495940055b
--- /dev/null
+++ b/lldb/test/API/iohandler/completion/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git a/lldb/test/API/iohandler/completion/TestIOHandlerCompletion.py
b/lldb/test/API/iohandler/completion/TestIOHandlerCompletion.py
index 2f49b810fb4c..42cc00b4f9e8 100644
--- a/lldb/test/API/iohandler/completion/TestIOHandlerCompletion.py
+++ b/lldb/test/API/iohandler/completion/TestIOHandlerCompletion.py
@@ -20,7 +20,8 @@ class IOHandlerCompletionTest(PExpectTest):
@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr49408')
@skipIf(oslist=["linux"], archs=["arm", "aarch64"])
def test_completion(self):
- self.launch(dimensions=(100,500))
+ self.build()
+ self.launch(dimensions=(100,500),
executable=self.getBuildArtifact("a.out"))
# Start tab completion, go to the next page and then display all
with 'a'.
self.child.send("\t\ta")
@@ -51,6 +52,11 @@ class IOHandlerCompletionTest(PExpectTest):
self.child.send("\n")
self.expect_prompt()
+ # Complete a file path.
+ # FIXME: This should complete to './main.c' and not 'main.c'
+ self.child.send("breakpoint set --file ./main\t")
+ self.child.expect_exact("main.c")
+ self.child.send("\n")
+ self.expect_prompt()
+
# Start tab completion and abort showing more commands with 'n'.
self.child.send("\t")
self.child.expect_exact("More (Y/n/a)")
FWIW, I think this is just fixing the symptom but not the cause of the
underlying LLDB bug. The file completion logic completes `./hello` to `hello.c`
instead of `./hello.c`. It also completes `/hello` to `hello.c` apparently.
This doesn't seem right. I think fixing this crash is fine, but we probably
should also add an assert somewhere that the completion of a token is actually
completing the provided token (and not something else). If people want to
rewrite the current token they should do it explicitly via a different (new)
completion mode. But this seems out-of-scope for this fix. So LGTM with the
test applies on top.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108817/new/
https://reviews.llvm.org/D108817
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
