This is an archive of the discontinued LLVM Phabricator instance.

Auto-completion bug fix for dot operator
ClosedPublic

Authored by diazhector98 on Jan 27 2020, 1:51 PM.

Details

Summary

There was a bug on LLDB VSCode where there was the following behavior:

//Code

struct foo {
    int bar:
};
...
foo my_foo = {10};

Trying to auto-complete my_foo.b with my_foo.bar resulted instead with my_foo.my_foo.bar

This diff fixes this bug and adds some tests to check correct behavior.

It also fixes the same bug using the arrow operator (->) when user manually requests completions.
TODO: Fix bug where no recommended completions are automatically shown with arrow operator

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptJan 27 2020, 1:51 PM
diazhector98 planned changes to this revision.Jan 27 2020, 1:57 PM
diazhector98 marked 5 inline comments as done.
diazhector98 added inline comments.
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/completions/TestVSCode_completions.py
118

also test this

foo.var1 + var2

foo.var1 + va

var2
var1
lldb/tools/lldb-vscode/lldb-vscode.cpp
957

commit_points

959

int breakpoint_index = -1

960

space before {

967

space before {

  1. Updating D73506: Bug fix #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment. #
  4. If you intended to create a new revision, use:
  5. $ arc diff --create

Removed print statements

As title

  1. Updating D73506: Bug fix #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment. #
  4. If you intended to create a new revision, use:
  5. $ arc diff --create

Syntax fixes and uncommenting @darwin

As title

diazhector98 planned changes to this revision.Jan 27 2020, 2:45 PM
  1. Updating D73506: Bug fix #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment. #
  4. If you intended to create a new revision, use:
  5. $ arc diff --create

Replaced storing breakpoint string with its index
.

diazhector98 updated this revision to Diff 240694.EditedJan 27 2020, 3:13 PM

Brackets syntax

diazhector98 updated this revision to Diff 240700.EditedJan 27 2020, 3:26 PM

Added more tests

diazhector98 updated this revision to Diff 240701.EditedJan 27 2020, 3:27 PM

Uncomment @skipIfDarwin

Harbormaster completed remote builds in B45047: Diff 240701.
wallace added inline comments.Jan 27 2020, 3:43 PM
lldb/tools/lldb-vscode/lldb-vscode.cpp
960

int or size_t is fine

diazhector98 retitled this revision from Bug fix to Auto-completion bug fix for dot operator.Jan 27 2020, 4:03 PM
diazhector98 edited the summary of this revision. (Show Details)
diazhector98 edited the summary of this revision. (Show Details)

int32 to int

diazhector98 marked an inline comment as done.

changed int to size_t to avoid warning

labath added a subscriber: labath.Jan 28 2020, 12:25 AM

Thanks for the patch.

I don't know much about the LSP protocol, but I see opportunities for simplifying the string chopping code. See the inline comment for what /I think/ should be the equivalent code.

It has also occurred to me that this may break for expressions containing quoted dots (foo(".", b<TAB>), but I don't know if there's anything we can do about that...

lldb/tools/lldb-vscode/lldb-vscode.cpp
957–973

If I correctly understand what this is doing, all of this should boil down to something like:

llvm::StringRef match_ref = match;
for (StringRef commit_point: {".", "->"}) {
  if (match_ref.contains(commit_point))
    match_ref = match_ref.rsplit(commit_point).second;
}
EmplaceSafeString(item, "text", match_ref);

I don't really know the vscode plugin code but I pointed out when this was implemented that the whole token replacement mechanism that LLDB's completion API uses is really hard to get right. If you're at it, you might also add some tests for the things I pointed out back then (e.g., foo1. v and foo1 .v and foo1 . v) where the returned tokens might be unexpectedly cut off.

Simplifying code

diazhector98 marked an inline comment as done.

Added tests for spaces between member operators

wallace accepted this revision.Jan 30 2020, 8:43 AM
This revision is now accepted and ready to land.Jan 30 2020, 8:43 AM
This revision was automatically updated to reflect the committed changes.