This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE333885: [clangd] Avoid indexing decls associated with
friend decls. (authored by ioeric, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D47623?vs=149723&id=149724#toc
Repository:
ioeric updated this revision to Diff 149723.
ioeric added a comment.
- Revert unintended changes.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
Files:
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
unittests/clangd/SymbolCollectorTests.cpp
Index:
ioeric updated this revision to Diff 149721.
ioeric marked an inline comment as done.
ioeric added a comment.
Herald added a subscriber: mgorny.
- Addressed review comment.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
Files:
clangd/CMakeLists.txt
clangd/Headers.h
ioeric added a comment.
In https://reviews.llvm.org/D47623#1120547, @ilya-biryukov wrote:
> In https://reviews.llvm.org/D47623#1119426, @ioeric wrote:
>
> > Sorry for the late response Ilya. I was trying to test these cases. So,
> > with the current change, if a real "canonical" declaration come
sammccall accepted this revision.
sammccall added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:302
+ // A declaration created for a friend declaration should not be used as the
+ // canonical declaration in the index. If D is a defintion and is not OrigD,
+ //
ilya-biryukov added a comment.
In https://reviews.llvm.org/D47623#1119426, @ioeric wrote:
> Sorry for the late response Ilya. I was trying to test these cases. So, with
> the current change, if a real "canonical" declaration comes before the friend
> decl, then the reference will still be recor
ioeric added a comment.
In https://reviews.llvm.org/D47623#1118951, @ilya-biryukov wrote:
> In https://reviews.llvm.org/D47623#1118810, @sammccall wrote:
>
> > - friend decls that are not definitions should be ignored for indexing
> > purposes
>
>
> This is not generally true IIUC. A friend decl
ioeric updated this revision to Diff 149528.
ioeric added a comment.
- Remove debug message.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
Files:
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
unittests/clangd/SymbolCollectorTests.cpp
Index: unit
ioeric updated this revision to Diff 149526.
ioeric added a comment.
- Make canonical decls determinstic.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
Files:
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
unittests/clangd/SymbolCollectorTests.cpp
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
This looks OK, the asymmetry still seems a little odd to me and could be
reduced a little.
(Please also resolve Ilya's question about references with him, I don't have a
strong opinion)
ioeric added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:297
+// If OrigD is an object of a friend declaration, skip it.
+if (ASTNode.OrigD->getFriendObjectKind() !=
+Decl::FriendObjectKind::FOK_None)
sammccall wrote:
> ioeric wr
ioeric updated this revision to Diff 149465.
ioeric added a comment.
- Clarify.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
Files:
clangd/index/SymbolCollector.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
ioeric added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:293
assert(CompletionAllocator && CompletionTUInfo);
+ // A declaration created for a friend declaration should not be used as the
+ // canonical declaration in the index.
ilya-biryuk
sammccall added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:297
+// If OrigD is an object of a friend declaration, skip it.
+if (ASTNode.OrigD->getFriendObjectKind() !=
+Decl::FriendObjectKind::FOK_None)
ioeric wrote:
> sammccall
ilya-biryukov added a comment.
In https://reviews.llvm.org/D47623#1118810, @sammccall wrote:
> - friend decls that are not definitions should be ignored for indexing
> purposes
This is not generally true IIUC. A friend declaration can be a reference, a
declaration or a definition.
int foo(
ioeric added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:297
+// If OrigD is an object of a friend declaration, skip it.
+if (ASTNode.OrigD->getFriendObjectKind() !=
+Decl::FriendObjectKind::FOK_None)
sammccall wrote:
> this seem
ioeric updated this revision to Diff 149446.
ioeric marked an inline comment as done.
ioeric edited the summary of this revision.
ioeric added a comment.
- Addressed review comment.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47623
Files:
clangd/index/SymbolCollector.cpp
ioeric added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:293
assert(CompletionAllocator && CompletionTUInfo);
+ // A declaration created for a friend declaration should not be used as the
+ // canonical declaration in the index.
ilya-biryuk
sammccall added a comment.
Do I understand the intent of this change correctly?
- friend decls that are not definitions should be ignored for indexing purposes
- this means they should never be selected as canonical decl
- if the friend decl is the only decl, then the symbol should not be indexed
ilya-biryukov added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:293
assert(CompletionAllocator && CompletionTUInfo);
+ // A declaration created for a friend declaration should not be used as the
+ // canonical declaration in the index.
Mayb
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov, klimek.
These decls are sometime used as the canonical declarations (e.g. for
go-to-def),
which seems to be bad.
Repository:
rCTE Clang Tools Extra
https:
21 matches
Mail list logo
