kadircet added inline comments.
================ Comment at: clang-tools-extra/clangd/FindTarget.cpp:715 + + void VisitObjCImplementationDecl(const ObjCImplementationDecl *OIMD) { + if (const auto *CI = OIMD->getClassInterface()) ---------------- can you also add tests for these into `FindTargetTests`? ================ Comment at: clang-tools-extra/clangd/FindTarget.cpp:716 + void VisitObjCImplementationDecl(const ObjCImplementationDecl *OIMD) { + if (const auto *CI = OIMD->getClassInterface()) + Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(), ---------------- we don't have the null check in other places, what's the significance here? ================ Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:131 return HighlightingKind::Interface; - if (isa<ObjCCategoryDecl>(D)) + if (isa<ObjCCategoryDecl, ObjCCategoryImplDecl>(D)) return HighlightingKind::Namespace; ---------------- let's do this in a separate change, with some tests ================ Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:171-177 + if (const auto *C = dyn_cast<ObjCCategoryDecl>(D)) { + if (C->getLocation() == TokenStartLoc) + D = C->getClassInterface(); + else if (const auto *I = C->getImplementation()) + if (I->getLocation() == TokenStartLoc) + D = C->getClassInterface(); + } ---------------- sorry i don't follow what's the logic doing here and we're likely doing these in the wrong layer. we should either: - Fix selection tree to pick the correct ASTNode, if it's picking the wrong one due to not having special cases for these locations here - Fix the `targetDecl` logic to make sure it emits all the declarations that might be referenced by this ast node, if it's missing ClassInterface. - Fix the canonicalRenameDecl, if we should always prefer `ClassInterface` in these cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152720/new/ https://reviews.llvm.org/D152720 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits