VitaNuo added a comment.
Thanks for the comments!
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:1348
+ auto Loc = SM.getFileLoc(Ref.RefLocation);
+ for (const auto &H : Providers) {
+ auto MatchingIncludes = ConvertedMainFileIncludes.match(H);
----------------
kadircet wrote:
> we're implementing and testing this logic twice now, once in here and once in
> hover. can we instead have a helper in `IncludeCleaner.h` that looks like:
> ```
> std::optional<include_cleaner::Header> firstSatisfiedProvider(const
> include_cleaner::Includes& Includes, llvm::ArrayRef<include_cleaner::Header>
> Providers);
> // I'd actually return the matching `std::vector<const Include*>` (the
> highest ranked provider that matched some includes in main file), and check
> if the include of interest is part of that set for rest of the operations.
> // Since it represents both the provider and the include in the main file.
> whereas the provider on it's own doesn't say anything about which include in
> main file triggered satisfaction.
> ```
> and turn these call sites into
> ```
> auto Provider = firstSatisfiedProvider(ConvertedMainFileIncludes, Providers);
> if(!Provider || ReferencedInclude.match(Provider).empty())
> return;
> // Include in question provides the symbol, do magic.
> ```
Is the comment under the code in the first snippet a mistake/outdated content?
It confused me.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147044/new/
https://reviews.llvm.org/D147044
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
