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 cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits