[PATCH] D66759: [clangd] Surface errors from command-line parsing

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D66759#1646509 <https://reviews.llvm.org/D66759#1646509>, @kadircet wrote:

> Thanks! This looks really useful when we can't build an AST due to unknown compiler commands, but I am not sure about how useful it is to surface command-line parsing errors once we are able to build an AST.
>  Because people most likely won't care about these errors once clangd is working for them, and even get annoyed since most of the developers has little control over how their build system generates their compile
>  commands, therefore it will be really hard to get rid of those errors showing up in all the files they want to edit.


How common is this? Do we have any particular examples?

> So I suggest only surfacing these if we are not able to create a compiler invocation or build an ast at all. WDYT?

My worry comes from the fact that I don't think we can guarantee that clangd is working for the users after we encountered those errors.
In particular, after D66731 <https://reviews.llvm.org/D66731>, we will be getting a compiler invocation and building the AST in many more cases. However, the AST might be completely bogus, e.g. because we did not recognize actually important flags.
Surfacing this error to the user is useful, because that would explain why clangd is rusty, gives weird results, etc.

In turn, the users will report bugs and we'll be able to collect data on whether this is noisy or everyone is doing compile_commands.json wrong and we should do something about it or whether it's actually very rare in practice and turns out to be a useful indicator for the users.
Either result seems plausible.
I'm all in for hiding annoying errors, but I don't know what the failure modes are in that particular case and I'd be tempted to first see whether surfacing the errors would **actually** end up producing those annoying errors and what the errors are.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66759/new/

https://reviews.llvm.org/D66759