sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Thanks, LGTM
Sorry about the delay - we got sidetracked into some ideas about whether these
errors should actually be surfaced to the user as diagnostics instead. But that
shouldn't block this patch.
================
Comment at: clang-tools-extra/clangd/ParsedAST.cpp:296
+ elog("Failed to prepare a compiler instance: {0}",
+ ASTDiags.getNumErrors()
+ ? static_cast<DiagBase &>(ASTDiags.take().back()).Message
----------------
there's a bunch of logic that determines which diags we actually end up storing.
I'd suggest always calling take() and testing whether the result is empty,
rather than using getNumErrors() which bypasses all our logic.
Luckily, we do (always?) end up storing the relevant diags here, assuming
they're errors without a source location attached.
However I don't think we should hard-code that assumption here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104056/new/
https://reviews.llvm.org/D104056
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
