adamcz added inline comments.
================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:193
+ } else {
+ elog("Code completion header path manipulation failed {0}",
+ HeaderFile.takeError());
----------------
sammccall wrote:
> I think this is too noisy.
>
> We can hit this in "normal" cases of certain projects.
> For better or worse, we don't currently filter out index results where we
> know the responsible header and can't include it (because it's not under any
> include path entry). In these cases toHeaderFile() returns an error.
>
> Since this can happen in the normal case for *each* completion candidate
> (default 100), and completion requests are very frequent, this could dominate
> log output in affected projects.
>
> It *might* be OK at vlog? I get the desire to not silence errors here. I
> think the question is what are the error cases we're *trying* to call out
> loudly. Maybe we can separate out the "shouldn't happen" vs the "fairly
> expected" cases.
> (Even then there's the prospect that this is either going to not fire or fire
> dozens of times, which is a little sad)
Originally I didn't even log here. We do this expansion at other places and
they may log already, I don't think it's critical to log here. Kadir pointed
out error needs to be handled or it will assert()-fail (that one place returns
Expected rather than Optional, I missed that), which is the motivation for this
change.
I made it vlog, but if you think it's too spammy I can remove it and just
ignore the error (by this time explicitly, so it doesn't crash). I don't think
we'll see these errors often - this only happens when we failed to parse the
URI or recognize the scheme, which basically can only happen with remote index
and some kind of version skew or corrupt data, so I'm fine with vlog() I think.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93220/new/
https://reviews.llvm.org/D93220
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
