[PATCH] D155614: [clangd] Make an include always refer to itself. Background: clang-review expects all referents to have definition, declaration or reference(s).
kadircet added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:1365 // Add the #include line to the references list. ReferencesResult::Reference Result; Result.Loc.range = i guess this patch is not needed anymore, but one comment about this behaviour in clangd as well (no action needed in this patch) why are we providing the reference to the include line itself? in other interactions we do that for the sake of completeness (e.g. if you trigger xrefs on a reference, seeing the declaration/definition location is useful) but usually the result under the cursor is not so interesting, especially in this case. the include line itself is drastically different than the nature of other references, the user can only see the include, if they triggered the xrefs on the include line itself and not when they trigger it on the symbol. also that ref doesn't count if there's no symbols used? FWIW I am not sure if that's a useful interaction. What was the rationale here exactly? It might be useful to always emit this reference, but also append provider to the refs list in the other direction as well (that way the results will be coherent again) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155614/new/ https://reviews.llvm.org/D155614 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D155614: [clangd] Make an include always refer to itself. Background: clang-review expects all referents to have definition, declaration or reference(s).
usaxena95 added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:1361-1362 }); - if (Results.References.empty()) -return std::nullopt; - Sorry for the confusion. This looks intentional and somewhat valuable for unused headers. We could fix this on clang-review's end as discussed on the internal patch. We should also add a comment here explaining the rationale behind returning no refs in such cases. (The check below certainly looks redundant though and could be removed). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155614/new/ https://reviews.llvm.org/D155614 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D155614: [clangd] Make an include always refer to itself. Background: clang-review expects all referents to have definition, declaration or reference(s).
VitaNuo created this revision. Herald added subscribers: kadircet, arphaman. Herald added a project: All. VitaNuo requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D155614 Files: clang-tools-extra/clangd/XRefs.cpp Index: clang-tools-extra/clangd/XRefs.cpp === --- clang-tools-extra/clangd/XRefs.cpp +++ clang-tools-extra/clangd/XRefs.cpp @@ -1358,18 +1358,13 @@ Result.Loc.uri = URIMainFile; Results.References.push_back(std::move(Result)); }); - if (Results.References.empty()) -return std::nullopt; - + // Add the #include line to the references list. ReferencesResult::Reference Result; Result.Loc.range = rangeTillEOL(SM.getBufferData(SM.getMainFileID()), Inc.HashOffset); Result.Loc.uri = URIMainFile; Results.References.push_back(std::move(Result)); - - if (Results.References.empty()) -return std::nullopt; return Results; } } // namespace Index: clang-tools-extra/clangd/XRefs.cpp === --- clang-tools-extra/clangd/XRefs.cpp +++ clang-tools-extra/clangd/XRefs.cpp @@ -1358,18 +1358,13 @@ Result.Loc.uri = URIMainFile; Results.References.push_back(std::move(Result)); }); - if (Results.References.empty()) -return std::nullopt; - + // Add the #include line to the references list. ReferencesResult::Reference Result; Result.Loc.range = rangeTillEOL(SM.getBufferData(SM.getMainFileID()), Inc.HashOffset); Result.Loc.uri = URIMainFile; Results.References.push_back(std::move(Result)); - - if (Results.References.empty()) -return std::nullopt; return Results; } } // namespace ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
