usaxena95 accepted this revision. usaxena95 added a comment. This revision is now accepted and ready to land.
LG. Thanks! ================ Comment at: clang-tools-extra/clangd/Protocol.cpp:1228 + Result["parents"] = RP.parents; + return std::move(Result); +} ---------------- kadircet wrote: > usaxena95 wrote: > > Nit: Allow RVO. > there's an issue with one of the compilers used by a build-bot which can't > find the relevant constructor if we don't have std::move here (hence we've > the same pattern across others). we should definitely check at some point to > see if that compiler is gone, but i'd rather not do that in the scope of this > patch. Interesting. Makes sense to get to it later. ================ Comment at: clang-tools-extra/clangd/XRefs.cpp:1713 + fillSuperTypes(*ParentDecl, TUPath, *ParentSym, RPSet); + Item.data.parents->emplace_back(ParentSym->data); + Item.parents->emplace_back(std::move(*ParentSym)); ---------------- nit: std::move. ================ Comment at: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp:780 + withResolveParents(HasValue(UnorderedElementsAre( + withResolveID(Result.front().data.symbolID.str()))))))); +} ---------------- kadircet wrote: > usaxena95 wrote: > > It would be clearer if we use `getSymbolID(&findDecl(AST, "Parent1"))` here > > and in `SuperTypes`. > > > > > not sure what you mean by also doing it in `SuperTypes` Nvm. I was referring to `SuperTypes` test but it does not check for the symbol id. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131385/new/ https://reviews.llvm.org/D131385 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
