[PATCH] D70312: [clangd] Fix SelectionTree behavior on constructor init-lists.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

Thanks!


For the constructor Foo() : classmember(arg) {}

The AST looks like:

- CXXCtorInitializer  classmember(arg)
  - CXXConstructExpr  classmember(arg)
    - DeclRefExpr:                arg

We want the 'classmember' to be associated with the CXXCtorInitializer, not the
CXXConstructExpr. (CXXConstructExpr is known to have bad ranges).
So just early-claim it.

Thanks @hokein for tracking down/reducing the bug.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70312

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -246,6 +246,17 @@
       // Tricky case: two VarDecls share a specifier.
       {"[[int ^a]], b;", "VarDecl"},
       {"[[int a, ^b]];", "VarDecl"},
+      // Tricky case: CXXConstructExpr wants to claim the whole init range.
+      {
+          R"cpp(
+            class X { X(int); };
+            class Y {
+              X x;
+              Y() : [[^x(4)]] {}
+            };
+          )cpp",
+          "CXXCtorInitializer", // Not the CXXConstructExpr!
+      },
       // Tricky case: anonymous struct is a sibling of the VarDecl.
       {"[[st^ruct {int x;}]] y;", "CXXRecordDecl"},
       {"[[struct {int x;} ^y]];", "VarDecl"},
Index: clang-tools-extra/clangd/Selection.cpp
===================================================================
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -10,6 +10,7 @@
 #include "Logger.h"
 #include "SourceCode.h"
 #include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
@@ -397,6 +398,9 @@
       // int (*[[s]])();
       else if (auto *VD = llvm::dyn_cast<VarDecl>(D))
         return VD->getLocation();
+    } else if (const auto* CCI = N.get<CXXCtorInitializer>()) {
+      // : [[b_]](42)
+      return CCI->getMemberLocation();
     }
     return SourceRange();
   }



Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -246,6 +246,17 @@
       // Tricky case: two VarDecls share a specifier.
       {"[[int ^a]], b;", "VarDecl"},
       {"[[int a, ^b]];", "VarDecl"},
+      // Tricky case: CXXConstructExpr wants to claim the whole init range.
+      {
+          R"cpp(
+            class X { X(int); };
+            class Y {
+              X x;
+              Y() : [[^x(4)]] {}
+            };
+          )cpp",
+          "CXXCtorInitializer", // Not the CXXConstructExpr!
+      },
       // Tricky case: anonymous struct is a sibling of the VarDecl.
       {"[[st^ruct {int x;}]] y;", "CXXRecordDecl"},
       {"[[struct {int x;} ^y]];", "VarDecl"},
Index: clang-tools-extra/clangd/Selection.cpp
===================================================================
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -10,6 +10,7 @@
 #include "Logger.h"
 #include "SourceCode.h"
 #include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
@@ -397,6 +398,9 @@
       // int (*[[s]])();
       else if (auto *VD = llvm::dyn_cast<VarDecl>(D))
         return VD->getLocation();
+    } else if (const auto* CCI = N.get<CXXCtorInitializer>()) {
+      // : [[b_]](42)
+      return CCI->getMemberLocation();
     }
     return SourceRange();
   }

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