[clangd] Support pseudo-obj expr, opaque values, and property references in findExplicitReferences()

[sammccall]

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

596

For consistency with the targetDecl() tests in this file, and because I had an error in the testcase that threw me for a loop. I'll try to find a better solution project-wide though.

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: diff.json, console-log.txt

[dgoldman]

clang-tools-extra/clangd/FindTarget.cpp

668

Worth mentioning this is handled in add()'s Visitor's VisitObjCPropertyRefExpr?

791

Worth noting that these two functions are used currently for ObjC?

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

1077

Would also be good to have a test for implicit property refs (e.g - (int)z; obj.z;) Or maybe this should go in the other diff?

[sammccall]

more tests

clang-tools-extra/clangd/FindTarget.cpp

668

I don't think we should echo the code structure here, it could change.

791

These functions are used for ObjC because these node types are used for objC - documenting that belongs on the AST class.
(I guess it's not currently done because someone currently/previously had an ambition to use these for other things, thus the generic names)

[merge_guards_bot]

Unit tests: fail. 62194 tests passed, 1 failed and 815 were skipped.

failed: libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_mutex_requirements_mutex/thread_mutex_recursive/try_lock.pass.cpp

clang-tidy: pass.

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.

[ilya-biryukov]

clang-tools-extra/clangd/FindTarget.cpp

794

Should this be done by RecursiveASTVisitor instead (when shouldVisitImplicitCode() is false)?

If yes, maybe add a FIXME to this patch and try fixing in RAV in a follow-up?

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

701

Why is this necessary? Making tests more complicated to avoid warnings does not look right.

712

See the comment about making tests more complicated to avoid warnings. Same question here and in other instances.

[sammccall]

Address comments

[merge_guards_bot]

Unit tests: pass. 62195 tests passed, 0 failed and 815 were skipped.

clang-tidy: pass.

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.

clang-tools-extra/clangd/FindTarget.cpp

llvm::SmallVector<ReferenceLoc, 2> refInDecl(const Decl *D) {
 
   Visitor V;
   V.Visit(D);
   return V.Refs;
 }
 
 llvm::SmallVector<ReferenceLoc, 2> refInExpr(const Expr *E) {
   struct Visitor : ConstStmtVisitor<Visitor> {
-    // FIXME: handle more complicated cases, e.g. ObjC, designated initializers.
+    // FIXME: handle more complicated cases: more ObjC, designated initializers.
     llvm::SmallVector<ReferenceLoc, 2> Refs;
 
     void VisitConceptSpecializationExpr(const ConceptSpecializationExpr *E) {
       Refs.push_back(ReferenceLoc{E->getNestedNameSpecifierLoc(),
                                   E->getConceptNameLoc(),
                                   /*IsDecl=*/false,
                                   {E->getNamedConcept()}});
     }
struct Visitor : ConstStmtVisitor<Visitor> {
     }
 
     void VisitSizeOfPackExpr(const SizeOfPackExpr *E) {
       Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
                                   E->getPackLoc(),
                                   /*IsDecl=*/false,
                                   {E->getPack()}});
     }
+
+    void VisitObjCPropertyRefExpr(const ObjCPropertyRefExpr *E) {
+      Refs.push_back(ReferenceLoc{
+          NestedNameSpecifierLoc(), E->getLocation(),
+          /*IsDecl=*/false,
+          // Select the getter, setter, or @property depending on the call.
+          explicitReferenceTargets(DynTypedNode::create(*E), {})});
+    }
   };
 
   Visitor V;
   V.Visit(E);
   return V.Refs;
 }
 
 llvm::SmallVector<ReferenceLoc, 2> refInTypeLoc(TypeLoc L) {
bool TraverseElaboratedTypeLoc(ElaboratedTypeLoc L) {
     return RecursiveASTVisitor::TraverseElaboratedTypeLoc(L);
   }
 
   bool VisitExpr(Expr *E) {
     visitNode(DynTypedNode::create(*E));
     return true;
   }
 
+  bool TraverseOpaqueValueExpr(OpaqueValueExpr *OVE) {
+    visitNode(DynTypedNode::create(*OVE));
+    // Not clear why the source expression is skipped by default...
+    // FIXME: can we just make RecursiveASTVisitor do this?
+    return RecursiveASTVisitor::TraverseStmt(OVE->getSourceExpr());
+  }
+
+  bool TraversePseudoObjectExpr(PseudoObjectExpr *POE) {
+    visitNode(DynTypedNode::create(*POE));
+    // Traverse only the syntactic form to find the *written* references.
+    // (The semantic form also contains lots of duplication)
+    return RecursiveASTVisitor::TraverseStmt(POE->getSyntacticForm());
+  }
+
   // We re-define Traverse*, since there's no corresponding Visit*.
   // TemplateArgumentLoc is the only way to get locations for references to
   // template template parameters.
   bool TraverseTemplateArgumentLoc(TemplateArgumentLoc A) {
     switch (A.getArgument().getKind()) {
     case TemplateArgument::Template:
     case TemplateArgument::TemplateExpansion:
       reportReference(ReferenceLoc{A.getTemplateQualifierLoc(),

clang-tools-extra/clangd/unittests/FindTargetTests.cpp

protected:
   AllRefs annotateReferencesInFoo(llvm::StringRef Code) {
     TestTU TU;
     TU.Code = Code;
 
     // FIXME: Auto-completion in a template requires disabling delayed template
     // parsing.
     TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
     TU.ExtraArgs.push_back("-std=c++2a");
+    TU.ExtraArgs.push_back("-xobjective-c++");
 
     auto AST = TU.build();
     auto *TestDecl = &findDecl(AST, "foo");
     if (auto *T = llvm::dyn_cast<FunctionTemplateDecl>(TestDecl))
       TestDecl = T->getTemplatedDecl();
 
     std::vector<ReferenceLoc> Refs;
     if (const auto *Func = llvm::dyn_cast<FunctionDecl>(TestDecl))
       findExplicitReferences(Func->getBody(), [&Refs](ReferenceLoc R) {
         Refs.push_back(std::move(R));
       });
std::pair</*Code*/ llvm::StringRef, /*References*/ llvm::StringRef> Cases[] =
         "1: targets = {ns::global}, qualifier = 'ns::'\n"},
        // Simple types.
        {R"cpp(
          struct Struct { int a; };
          using Typedef = int;
          void foo() {
            $0^Struct $1^x;
            $2^Typedef $3^y;
            static_cast<$4^Struct*>(0);
          }
        )cpp",
         "0: targets = {Struct}\n"
         "1: targets = {x}, decl\n"
         "2: targets = {Typedef}\n"
         "3: targets = {y}, decl\n"
         "4: targets = {Struct}\n"},
        // Name qualifiers.
        {R"cpp(
          namespace a { namespace b { struct S { typedef int type; }; } }
          void foo() {
            $0^a::$1^b::$2^S $3^x;
            using namespace $4^a::$5^b;
            $6^S::$7^type $8^y;
          }
         )cpp",
         "0: targets = {a}\n"
         "1: targets = {a::b}, qualifier = 'a::'\n"
         "2: targets = {a::b::S}, qualifier = 'a::b::'\n"
std::pair</*Code*/ llvm::StringRef, /*References*/ llvm::StringRef> Cases[] =
        // Templates
        {R"cpp(
             namespace foo {
               template <typename $0^T>
               class $1^Bar {};
             }
           )cpp",
         "0: targets = {foo::Bar::T}, decl\n"
         "1: targets = {foo::Bar}, decl\n"},
        // Templates
        {R"cpp(
             namespace foo {
               template <typename $0^T>
               void $1^func();
             }
           )cpp",
         "0: targets = {T}, decl\n"
std::pair</*Code*/ llvm::StringRef, /*References*/ llvm::StringRef> Cases[] =
               }
           )cpp",
            "0: targets = {T}, decl\n"
            "1: targets = {Drawable}\n"
            "2: targets = {T}\n"
            "3: targets = {foo::bar}, decl\n"
            "4: targets = {T}\n"
            "5: targets = {t}, decl\n"
-           "6: targets = {t}\n"}};
+           "6: targets = {t}\n"},
+       // Objective-C: properties
+       {
+           R"cpp(
+            @interface I {}
+            @property(retain) I* x;
+            @property(retain) I* y;
+            @end
+            I *f;
+            void foo() {
+              $0^f.$1^x.$2^y = 0;
+            }
+          )cpp",
+           "0: targets = {f}\n"
+           "1: targets = {I::x}\n"
+           "2: targets = {I::y}\n"},
+       // Objective-C: implicit properties
+       {
+           R"cpp(
+            @interface I {}
+            -(I*)x;
+            -(void)setY:(I*)y;
+            @end
+            I *f;
+            void foo() {
+              $0^f.$1^x.$2^y = 0;
+            }
+          )cpp",
+           "0: targets = {f}\n"
+           "1: targets = {I::x}\n"
+           "2: targets = {I::setY:}\n"}};
 
   for (const auto &C : Cases) {
     llvm::StringRef ExpectedCode = C.first;
     llvm::StringRef ExpectedRefs = C.second;
 
     auto Actual =
         annotateReferencesInFoo(llvm::Annotations(ExpectedCode).code());
     EXPECT_EQ(ExpectedCode, Actual.AnnotatedCode);
     EXPECT_EQ(ExpectedRefs, Actual.DumpedReferences) << ExpectedCode;
   }
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang