[all-commits] [llvm/llvm-project] 22f812: [Tooling/Syntax] Helpers to find spelled tokens to...

[Sam McCall via All-commits]

  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 22f81250889b2e366187ee1465ba0ec71a6e457d
      https://github.com/llvm/llvm-project/commit/22f81250889b2e366187ee1465ba0ec71a6e457d
  Author: Sam McCall <sam.mccall at gmail.com>
  Date:   2019-12-13 (Fri, 13 Dec 2019)

  Changed paths:
    M clang/lib/Tooling/Syntax/Tokens.cpp

  Log Message:
  -----------
  [Tooling/Syntax] Helpers to find spelled tokens touching a location.

Summary: Useful when positions are used to target nodes, with before/after ambiguity.

Reviewers: ilya-biryukov, kbobyrev

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D71356


  Commit: b60896fad926754f715acc5d771555aaaa577e0f
      https://github.com/llvm/llvm-project/commit/b60896fad926754f715acc5d771555aaaa577e0f
  Author: Sam McCall <sam.mccall at gmail.com>
  Date:   2019-12-13 (Fri, 13 Dec 2019)

  Changed paths:
    M clang-tools-extra/clangd/ClangdServer.cpp
    M clang-tools-extra/clangd/Hover.cpp
    M clang-tools-extra/clangd/Selection.cpp
    M clang-tools-extra/clangd/Selection.h
    M clang-tools-extra/clangd/SemanticSelection.cpp
    M clang-tools-extra/clangd/XRefs.cpp
    M clang-tools-extra/clangd/refactor/Rename.cpp
    M clang-tools-extra/clangd/refactor/Tweak.cpp
    M clang-tools-extra/clangd/refactor/Tweak.h
    M clang-tools-extra/clangd/unittests/FindTargetTests.cpp
    M clang-tools-extra/clangd/unittests/HoverTests.cpp
    M clang-tools-extra/clangd/unittests/SelectionTests.cpp
    M clang-tools-extra/clangd/unittests/TweakTesting.cpp
    M clang-tools-extra/clangd/unittests/TweakTests.cpp

  Log Message:
  -----------
  [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

Summary:
The problem:

LSP specifies that Positions are between characters. Therefore when a position
(or an empty range) is used to target elements of the source code, there is an
ambiguity - should we look left or right of the cursor?

Until now, SelectionTree resolved this to the right except in trivial cases
(where there's whitespace, semicolon, or eof on the right).
This meant that it's unable to e.g. out-line `int foo^()` today.

Complicating this, LSP notwithstanding the cursor is *on* a character in many
editors (mostly terminal-based). In these cases there's no ambiguity - we must
"look right" - but there's also no way to tell in LSP.

(Several features currently resolve this by using getBeginningOfIdentifier,
which tries to rewind and supports end-of-identifier. But this relies on
raw lexing and is limited and buggy).

Precedent: well - most other languages aren't so full of densely packed symbols
that we might want to target. Bias-towards-identifier works well enough.
MS C++ for vscode seems to mostly use bias-toward-identifier too.
The problem with this solution is it doesn't provide any way to target some
things such as the constructor call in Foo^(bar());

Presented solution:

When an ambiguous selection is found, we generate *both* possible selection
trees. We try to run the feature on the rightward tree first, and then on the
leftward tree if it fails.

This is basically do-what-I-mean, the main downside is the need to do this on
a feature-by-feature basis (because each feature knows what "fail" means).
The most complicated instance of this is Tweaks, where the preferred selection
may vary tweak-by-tweak.

Wrinkles:

While production behavior is pretty consistent, this introduces some
inconsistency in testing, depending whether the interface we're testing is
inside or outside the "retry" wrapper.

In particular, for many features like Hover, the unit tests will show production
behavior, while for Tweaks the harness would have to run the loop itself if
we want this.

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D71345


Compare: https://github.com/llvm/llvm-project/compare/0eb099273918...b60896fad926