[PATCH] D89670: [clangd] Store the containing symbol for refs

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3bec07f91fb1: [clangd] Store the containing symbol for refs (authored by nridge). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D89670: [clangd] Store the containing symbol for refs

nridge updated this revision to Diff 302759. nridge marked 4 inline comments as done. nridge added a comment. Address final review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89670/new/ https://reviews.llvm.org/D89670 Files:

[PATCH] D89670: [clangd] Store the containing symbol for refs

kadircet accepted this revision. kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:770 +EXPECT_TRUE(bool(Ref)); +if (!AllowNull) + EXPECT_FALSE(Ref->Container.isNull()); nit: instead of making

[PATCH] D89670: [clangd] Store the containing symbol for refs

nridge added a comment. (I will wait for a response about the containers for top-level decls before committing.) Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:803 + + AssertSameContainer("toplevel1", "toplevel2"); +

[PATCH] D89670: [clangd] Store the containing symbol for refs

nridge updated this revision to Diff 302471. nridge added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89670/new/ https://reviews.llvm.org/D89670 Files: clang-tools-extra/clangd/index/Ref.h

[PATCH] D89670: [clangd] Store the containing symbol for refs

nridge marked an inline comment as done. nridge added inline comments. Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:782 + +EXPECT_EQ(Ref1->Container, Ref2->Container); + }; kadircet wrote: > can you also assert containers here are

[PATCH] D89670: [clangd] Store the containing symbol for refs

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, LGTM! Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:782 + +EXPECT_EQ(Ref1->Container, Ref2->Container); + }; can

[PATCH] D89670: [clangd] Store the containing symbol for refs

nridge updated this revision to Diff 302121. nridge marked an inline comment as done. nridge added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89670/new/ https://reviews.llvm.org/D89670 Files:

[PATCH] D89670: [clangd] Store the containing symbol for refs

nridge added inline comments. Comment at: clang-tools-extra/clangd/index/Ref.h:91 RefKind Kind = RefKind::Unknown; + SymbolID Container; }; kadircet wrote: > i am afraid we are going to have an indeterminate value here if for whatever > reason `Container`

[PATCH] D89670: [clangd] Store the containing symbol for refs

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/Ref.h:91 RefKind Kind = RefKind::Unknown; + SymbolID Container; }; i am afraid we are going to have an indeterminate value here if for whatever reason `Container` detection fails.

[PATCH] D89670: [clangd] Store the containing symbol for refs

nridge marked an inline comment as done. nridge added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:349 +SymbolRef{SM.getFileLoc(Loc), Roles, + dyn_cast_or_null(ASTNode.Parent)}); // Don't continue indexing if this is

[PATCH] D89670: [clangd] Store the containing symbol for refs

nridge updated this revision to Diff 300518. nridge added a comment. Use Decl rather than NamedDecl, and add a test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89670/new/ https://reviews.llvm.org/D89670 Files:

[PATCH] D89670: [clangd] Store the containing symbol for refs

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:349 +SymbolRef{SM.getFileLoc(Loc), Roles, + dyn_cast_or_null(ASTNode.Parent)}); // Don't continue indexing if this is a mere reference.

[PATCH] D89670: [clangd] Store the containing symbol for refs

nridge created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman. Herald added a project: clang. nridge requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. This is needed to implement call hierarchy. Repository: rG LLVM Github