hokein added inline comments.
================
Comment at: clang-tools-extra/clangd/HeaderSourceSwitch.cpp:86
+ auto AwardTarget = [&](const char *TargetURI) {
+ if (auto TargetPath = URI::resolve(TargetURI, OriginalFile)) {
+ if (*TargetPath != OriginalFile) // exclude the original file.
----------------
kadircet wrote:
> should we have a cache for these uri resolutions?
> We will most likely have a few URIs that are being resolved over and over
> again.
> We will most likely have a few URIs that are being resolved over and over
> again.
This is true, but having a cache seems to be a premature optimization at the
moment. I think the number for the Decl in the main file is relatively small in
practice. We could add it when it turns out a performance problem.
================
Comment at: clang-tools-extra/clangd/HeaderSourceSwitch.cpp:114
+ for (auto It = Candidates.begin(); It != Candidates.end(); ++It) {
+ if (It->second > Best->second)
+ Best = It;
----------------
kadircet wrote:
> what if there are ties ?
>
> we don't need to do anything clever yet, but we should at least be
> deterministic. Currently it depends on stringmap ordering.
> Could you pick the candidate that comes first in the lexical order in such
> cases?
> and add some test cases?
good point, I thought the StringMap is sorted by the key value, but it uses
hash value.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67907/new/
https://reviews.llvm.org/D67907
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
