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

Reply via email to