[PATCH] D70727: [clangd] Keep go-to-definition working at the end of an identifier (fixes #194)

2019-11-30 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge added a comment.

In D70727#1763781 , @hokein wrote:

> In D70727#1760485 , @nridge wrote:
>
> > By the way, may I get permissions to assign issues to myself (and make 
> > other such metadata changes to issues) on github?
>
>
> sure, we just sent you an invitation to the clangd org.


Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70727/new/

https://reviews.llvm.org/D70727



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70727: [clangd] Keep go-to-definition working at the end of an identifier (fixes #194)

2019-11-30 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge abandoned this revision.
nridge marked an inline comment as done.
nridge added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:153
+  // allowing go-to-definition to work at the end of an identifier.
+  if (Result.empty() && Offset > 0) {
+Result = getDeclAtOffset(AST, Offset - 1, Relations);

hokein wrote:
> this looks a hacky workaround, I assume it may introduce other regression 
> issues. 
> 
> I think https://reviews.llvm.org/D70773 and https://reviews.llvm.org/D70807 
> should fix most annoying cases (function call). 
Fair enough! Will abandon this patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70727/new/

https://reviews.llvm.org/D70727



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70727: [clangd] Keep go-to-definition working at the end of an identifier (fixes #194)

2019-11-29 Thread Haojian Wu via Phabricator via cfe-commits 
hokein added a comment.

In D70727#1760485 , @nridge wrote:

> By the way, may I get permissions to assign issues to myself (and make other 
> such metadata changes to issues) on github?


sure, we just sent you an invitation to the clangd org.




Comment at: clang-tools-extra/clangd/XRefs.cpp:153
+  // allowing go-to-definition to work at the end of an identifier.
+  if (Result.empty() && Offset > 0) {
+Result = getDeclAtOffset(AST, Offset - 1, Relations);

this looks a hacky workaround, I assume it may introduce other regression 
issues. 

I think https://reviews.llvm.org/D70773 and https://reviews.llvm.org/D70807 
should fix most annoying cases (function call). 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70727/new/

https://reviews.llvm.org/D70727



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70727: [clangd] Keep go-to-definition working at the end of an identifier (fixes #194)

2019-11-26 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge added a comment.

By the way, may I get permissions to assign issues to myself (and make other 
such metadata changes to issues) on github?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70727/new/

https://reviews.llvm.org/D70727



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70727: [clangd] Keep go-to-definition working at the end of an identifier (fixes #194)

2019-11-26 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge created this revision.
nridge added a reviewer: hokein.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70727

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -450,6 +450,17 @@
   +^+x;
 }
   )cpp",
+
+  R"cpp(// End of identifier (definition)
+void [[func]]^() {}
+  )cpp",
+
+  R"cpp(// End of identifier (reference)
+void [[func]]() {}
+void test() {
+  func^();
+}
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -129,11 +129,8 @@
   return Merged.CanonicalDeclaration;
 }
 
-std::vector getDeclAtPosition(ParsedAST , SourceLocation Pos,
-DeclRelationSet Relations) {
-  FileID FID;
-  unsigned Offset;
-  std::tie(FID, Offset) = AST.getSourceManager().getDecomposedSpellingLoc(Pos);
+std::vector getDeclAtOffset(ParsedAST , unsigned Offset,
+  DeclRelationSet Relations) {
   SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
   std::vector Result;
   if (const SelectionTree::Node *N = Selection.commonAncestor()) {
@@ -143,6 +140,22 @@
   return Result;
 }
 
+std::vector getDeclAtPosition(ParsedAST , SourceLocation Pos,
+DeclRelationSet Relations) {
+  FileID FID;
+  unsigned Offset;
+  std::tie(FID, Offset) = AST.getSourceManager().getDecomposedSpellingLoc(Pos);
+  std::vector Result = getDeclAtOffset(AST, Offset, Relations);
+  // If no declaration was found at this offset, try the previous offset.
+  // This compensates for the fact that SelectionTree interprets an offset
+  // as applying to the character after rather than the character before,
+  // allowing go-to-definition to work at the end of an identifier.
+  if (Result.empty() && Offset > 0) {
+Result = getDeclAtOffset(AST, Offset - 1, Relations);
+  }
+  return Result;
+}
+
 llvm::Optional makeLocation(ASTContext , SourceLocation TokLoc,
   llvm::StringRef TUPath) {
   const SourceManager  = AST.getSourceManager();


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -450,6 +450,17 @@
   +^+x;
 }
   )cpp",
+
+  R"cpp(// End of identifier (definition)
+void [[func]]^() {}
+  )cpp",
+
+  R"cpp(// End of identifier (reference)
+void [[func]]() {}
+void test() {
+  func^();
+}
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -129,11 +129,8 @@
   return Merged.CanonicalDeclaration;
 }
 
-std::vector getDeclAtPosition(ParsedAST , SourceLocation Pos,
-DeclRelationSet Relations) {
-  FileID FID;
-  unsigned Offset;
-  std::tie(FID, Offset) = AST.getSourceManager().getDecomposedSpellingLoc(Pos);
+std::vector getDeclAtOffset(ParsedAST , unsigned Offset,
+  DeclRelationSet Relations) {
   SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
   std::vector Result;
   if (const SelectionTree::Node *N = Selection.commonAncestor()) {
@@ -143,6 +140,22 @@
   return Result;
 }
 
+std::vector getDeclAtPosition(ParsedAST , SourceLocation Pos,
+DeclRelationSet Relations) {
+  FileID FID;
+  unsigned Offset;
+  std::tie(FID, Offset) = AST.getSourceManager().getDecomposedSpellingLoc(Pos);
+  std::vector Result = getDeclAtOffset(AST, Offset, Relations);
+  // If no declaration was found at this offset, try the previous offset.
+  // This compensates for the fact that SelectionTree interprets an offset
+  // as applying to the character after rather than the character before,
+  // allowing go-to-definition to work at the end of an identifier.
+  if (Result.empty() && Offset > 0) {
+Result = getDeclAtOffset(AST, Offset - 1, Relations);
+  }
+  return Result;
+}
+
 llvm::Optional makeLocation(ASTContext , SourceLocation TokLoc,