[PATCH] D72634: [clangd] Improve ObjC property handling in SelectionTree.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGaf071f03f379: [clangd] Improve ObjC property handling in 
SelectionTree. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72634

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.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
@@ -323,12 +323,52 @@
 Foo x = [[^12_ud]];
   )cpp",
   "UserDefinedLiteral"},
+
   {
   R"cpp(
 int a;
 decltype([[^a]] + a) b;
 )cpp",
   "DeclRefExpr"},
+
+  // Objective-C OpaqueValueExpr/PseudoObjectExpr has weird ASTs.
+  // Need to traverse the contents of the OpaqueValueExpr to the POE,
+  // and ensure we traverse only the syntactic form of the 
PseudoObjectExpr.
+  {
+  R"cpp(
+@interface I{}
+@property(retain) I*x;
+@property(retain) I*y;
+@end
+void test(I *f) { [[^f]].x.y = 0; }
+  )cpp",
+  "DeclRefExpr"},
+  {
+  R"cpp(
+@interface I{}
+@property(retain) I*x;
+@property(retain) I*y;
+@end
+void test(I *f) { [[f.^x]].y = 0; }
+  )cpp",
+  "ObjCPropertyRefExpr"},
+  // Examples with implicit properties.
+  {
+  R"cpp(
+@interface I{}
+-(int)foo;
+@end
+int test(I *f) { return 42 + [[^f]].foo; }
+  )cpp",
+  "DeclRefExpr"},
+  {
+  R"cpp(
+@interface I{}
+-(int)foo;
+@end
+int test(I *f) { return 42 + [[f.^foo]]; }
+  )cpp",
+  "ObjCPropertyRefExpr"},
   };
   for (const Case  : Cases) {
 Annotations Test(C.Code);
@@ -339,6 +379,7 @@
 // FIXME: Auto-completion in a template requires disabling delayed template
 // parsing.
 TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+TU.ExtraArgs.push_back("-xobjective-c++");
 
 auto AST = TU.build();
 auto T = makeSelectionTree(C.Code, AST);
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -536,7 +536,8 @@
   [[f.x]].y = 0;
 }
   )cpp";
-  EXPECT_DECLS("OpaqueValueExpr", "@property(atomic, retain, readwrite) I *x");
+  EXPECT_DECLS("ObjCPropertyRefExpr",
+   "@property(atomic, retain, readwrite) I *x");
 
   Code = R"cpp(
 @protocol Foo
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -462,6 +462,14 @@
  TraverseStmt(S->getRangeInit()) && TraverseStmt(S->getBody());
 });
   }
+  // OpaqueValueExpr blocks traversal, we must explicitly traverse it.
+  bool TraverseOpaqueValueExpr(OpaqueValueExpr *E) {
+return traverseNode(E, [&] { return TraverseStmt(E->getSourceExpr()); });
+  }
+  // We only want to traverse the *syntactic form* to understand the selection.
+  bool TraversePseudoObjectExpr(PseudoObjectExpr *E) {
+return traverseNode(E, [&] { return TraverseStmt(E->getSyntacticForm()); 
});
+  }
 
 private:
   using Base = RecursiveASTVisitor;


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -323,12 +323,52 @@
 Foo x = [[^12_ud]];
   )cpp",
   "UserDefinedLiteral"},
+
   {
   R"cpp(
 int a;
 decltype([[^a]] + a) b;
 )cpp",
   "DeclRefExpr"},
+
+  // Objective-C OpaqueValueExpr/PseudoObjectExpr has weird ASTs.
+  // Need to traverse the contents of the OpaqueValueExpr to the POE,
+  // and ensure we traverse only the syntactic form of the PseudoObjectExpr.
+  {
+  R"cpp(
+@interface I{}
+@property(retain) I*x;
+@property(retain) I*y;
+@end
+void test(I *f) { [[^f]].x.y = 0; }
+  )cpp",
+  "DeclRefExpr"},
+  {
+  R"cpp(
+@interface I{}
+@property(retain) I*x;
+@property(retain) I*y;
+@end
+void test(I *f) { [[f.^x]].y = 0; }
+  )cpp",
+  

[PATCH] D72634: [clangd] Improve ObjC property handling in SelectionTree.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62195 tests passed, 0 failed 
and 815 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72634



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

[PATCH] D72634: [clangd] Improve ObjC property handling in SelectionTree.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 240377.
sammccall added a comment.

more tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72634

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.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
@@ -323,12 +323,52 @@
 Foo x = [[^12_ud]];
   )cpp",
   "UserDefinedLiteral"},
+
   {
   R"cpp(
 int a;
 decltype([[^a]] + a) b;
 )cpp",
   "DeclRefExpr"},
+
+  // Objective-C OpaqueValueExpr/PseudoObjectExpr has weird ASTs.
+  // Need to traverse the contents of the OpaqueValueExpr to the POE,
+  // and ensure we traverse only the syntactic form of the 
PseudoObjectExpr.
+  {
+  R"cpp(
+@interface I{}
+@property(retain) I*x;
+@property(retain) I*y;
+@end
+void test(I *f) { [[^f]].x.y = 0; }
+  )cpp",
+  "DeclRefExpr"},
+  {
+  R"cpp(
+@interface I{}
+@property(retain) I*x;
+@property(retain) I*y;
+@end
+void test(I *f) { [[f.^x]].y = 0; }
+  )cpp",
+  "ObjCPropertyRefExpr"},
+  // Examples with implicit properties.
+  {
+  R"cpp(
+@interface I{}
+-(int)foo;
+@end
+int test(I *f) { return 42 + [[^f]].foo; }
+  )cpp",
+  "DeclRefExpr"},
+  {
+  R"cpp(
+@interface I{}
+-(int)foo;
+@end
+int test(I *f) { return 42 + [[f.^foo]]; }
+  )cpp",
+  "ObjCPropertyRefExpr"},
   };
   for (const Case  : Cases) {
 Annotations Test(C.Code);
@@ -339,6 +379,7 @@
 // FIXME: Auto-completion in a template requires disabling delayed template
 // parsing.
 TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+TU.ExtraArgs.push_back("-xobjective-c++");
 
 auto AST = TU.build();
 auto T = makeSelectionTree(C.Code, AST);
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -536,7 +536,8 @@
   [[f.x]].y = 0;
 }
   )cpp";
-  EXPECT_DECLS("OpaqueValueExpr", "@property(atomic, retain, readwrite) I *x");
+  EXPECT_DECLS("ObjCPropertyRefExpr",
+   "@property(atomic, retain, readwrite) I *x");
 
   Code = R"cpp(
 @protocol Foo
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -462,6 +462,14 @@
  TraverseStmt(S->getRangeInit()) && TraverseStmt(S->getBody());
 });
   }
+  // OpaqueValueExpr blocks traversal, we must explicitly traverse it.
+  bool TraverseOpaqueValueExpr(OpaqueValueExpr *E) {
+return traverseNode(E, [&] { return TraverseStmt(E->getSourceExpr()); });
+  }
+  // We only want to traverse the *syntactic form* to understand the selection.
+  bool TraversePseudoObjectExpr(PseudoObjectExpr *E) {
+return traverseNode(E, [&] { return TraverseStmt(E->getSyntacticForm()); 
});
+  }
 
 private:
   using Base = RecursiveASTVisitor;


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -323,12 +323,52 @@
 Foo x = [[^12_ud]];
   )cpp",
   "UserDefinedLiteral"},
+
   {
   R"cpp(
 int a;
 decltype([[^a]] + a) b;
 )cpp",
   "DeclRefExpr"},
+
+  // Objective-C OpaqueValueExpr/PseudoObjectExpr has weird ASTs.
+  // Need to traverse the contents of the OpaqueValueExpr to the POE,
+  // and ensure we traverse only the syntactic form of the PseudoObjectExpr.
+  {
+  R"cpp(
+@interface I{}
+@property(retain) I*x;
+@property(retain) I*y;
+@end
+void test(I *f) { [[^f]].x.y = 0; }
+  )cpp",
+  "DeclRefExpr"},
+  {
+  R"cpp(
+@interface I{}
+@property(retain) I*x;
+@property(retain) I*y;
+@end
+void test(I *f) { [[f.^x]].y = 0; }
+  )cpp",
+  "ObjCPropertyRefExpr"},
+  // Examples with implicit properties.
+  {
+  R"cpp(
+   

[PATCH] D72634: [clangd] Improve ObjC property handling in SelectionTree.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61796 tests passed, 0 failed 
and 781 were skipped.

{icon question-circle color=gray} clang-tidy: unknown.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72634



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

[PATCH] D72634: [clangd] Improve ObjC property handling in SelectionTree.

[David Goldman via Phabricator via cfe-commits]

dgoldman requested changes to this revision.
dgoldman added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:347
+  )cpp",
+  "ObjCPropertyRefExpr"},
+

Please add a test for the case mentioned in 
https://github.com/clangd/clangd/issues/253 as well (for implicit property 
refs) even though it should also be a ObjCPropertyRefExpr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72634



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

[PATCH] D72634: [clangd] Improve ObjC property handling in SelectionTree.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: dgoldman.
Herald added subscribers: cfe-commits, usaxena95, kadircet, jfb, arphaman, 
jkorous, MaskRay, ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72634

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.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
@@ -323,6 +323,29 @@
 Foo x = [[^12_ud]];
   )cpp",
   "UserDefinedLiteral"},
+
+  // Objective-C OpaqueValueExpr/PseudoObjectExpr has weird ASTs.
+  // Need to traverse the contents of the OpaqueValueExpr to the POE,
+  // and ensure we traverse only the syntactic form of the 
PseudoObjectExpr.
+  {
+  R"cpp(
+@interface I{}
+@property(retain) I*x;
+@property(retain) I*y;
+@end
+void test(I *f) { [[^f]].x.y = 0; }
+  )cpp",
+  "DeclRefExpr"},
+  {
+  R"cpp(
+@interface I{}
+@property(retain) I*x;
+@property(retain) I*y;
+@end
+void test(I *f) { [[f.^x]].y = 0; }
+  )cpp",
+  "ObjCPropertyRefExpr"},
+
   };
   for (const Case  : Cases) {
 Annotations Test(C.Code);
@@ -333,6 +356,7 @@
 // FIXME: Auto-completion in a template requires disabling delayed template
 // parsing.
 TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+TU.ExtraArgs.push_back("-xobjective-c++");
 
 auto AST = TU.build();
 auto T = makeSelectionTree(C.Code, AST);
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -511,7 +511,8 @@
   [[f.x]].y = 0;
 }
   )cpp";
-  EXPECT_DECLS("OpaqueValueExpr", "@property(atomic, retain, readwrite) I *x");
+  EXPECT_DECLS("ObjCPropertyRefExpr",
+   "@property(atomic, retain, readwrite) I *x");
 
   Code = R"cpp(
 @protocol Foo
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -462,6 +462,14 @@
  TraverseStmt(S->getRangeInit()) && TraverseStmt(S->getBody());
 });
   }
+  // OpaqueValueExpr blocks traversal, we must explicitly traverse it.
+  bool TraverseOpaqueValueExpr(OpaqueValueExpr *E) {
+return traverseNode(E, [&] { return TraverseStmt(E->getSourceExpr()); });
+  }
+  // We only want to traverse the *syntactic form* to understand the selection.
+  bool TraversePseudoObjectExpr(PseudoObjectExpr *E) {
+return traverseNode(E, [&] { return TraverseStmt(E->getSyntacticForm()); 
});
+  }
 
 private:
   using Base = RecursiveASTVisitor;


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -323,6 +323,29 @@
 Foo x = [[^12_ud]];
   )cpp",
   "UserDefinedLiteral"},
+
+  // Objective-C OpaqueValueExpr/PseudoObjectExpr has weird ASTs.
+  // Need to traverse the contents of the OpaqueValueExpr to the POE,
+  // and ensure we traverse only the syntactic form of the PseudoObjectExpr.
+  {
+  R"cpp(
+@interface I{}
+@property(retain) I*x;
+@property(retain) I*y;
+@end
+void test(I *f) { [[^f]].x.y = 0; }
+  )cpp",
+  "DeclRefExpr"},
+  {
+  R"cpp(
+@interface I{}
+@property(retain) I*x;
+@property(retain) I*y;
+@end
+void test(I *f) { [[f.^x]].y = 0; }
+  )cpp",
+  "ObjCPropertyRefExpr"},
+
   };
   for (const Case  : Cases) {
 Annotations Test(C.Code);
@@ -333,6 +356,7 @@
 // FIXME: Auto-completion in a template requires disabling delayed template
 // parsing.
 TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+TU.ExtraArgs.push_back("-xobjective-c++");
 
 auto AST = TU.build();
 auto T = makeSelectionTree(C.Code, AST);
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -511,7 +511,8 @@
   [[f.x]].y = 0;
 }
   )cpp";
-  EXPECT_DECLS("OpaqueValueExpr",