This is an archive of the discontinued LLVM Phabricator instance.

[clangd][nfc] Show more information in logs when compiler instance prepare fails
ClosedPublic

Authored by ArcsinX on Jun 10 2021, 1:44 PM.

Details

Summary

Without this patch clangd silently process compiler instance prepare failure and only LSP errors "Invalid AST" could be found in logs.
E.g. the reason of the problem https://github.com/clangd/clangd/issues/734 is impossible to understand without verbose logs or with disabled background index.
This patch adds more information into logs to help understand the reason of such failures.

Logs without this patch:

E[...] Could not build a preamble for file test.cpp version 1

Logs with this patch:

E[...] Could not build a preamble for file test.cpp version 1: CreateTargetInfo() return null
..
E[...] Failed to prepare a compiler instance: unknown target ABI 'lp64'

Diff Detail

Event Timeline

ArcsinX created this revision.Jun 10 2021, 1:44 PM
ArcsinX requested review of this revision.Jun 10 2021, 1:44 PM
sammccall accepted this revision.Jun 29 2021, 8:30 AM

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.

clang-tools-extra/clangd/ParsedAST.cpp
296

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.

This revision is now accepted and ready to land.Jun 29 2021, 8:30 AM
ArcsinX updated this revision to Diff 355389.Jun 29 2021, 3:43 PM

Check ASTDiags.take() for empty instead of ASTDiags.getNumErrors() value check

ArcsinX marked an inline comment as done.Jun 29 2021, 3:45 PM
ArcsinX edited the summary of this revision. (Show Details)Jun 30 2021, 1:35 PM