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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: kadircet.
Herald added subscribers: arphaman, jkorous, MaskRay, javed.absar.
Herald added a project: clang.
ilya-biryukov added a parent revision: D66731: [Driver] Add an option for createInvocationFromCommandLine to recover on errors.

Those errors are exposed at the first character of a file,
for a lack of a better place.

Previously, all errors were stored inside the AST and report
accordingly. However, errors in command-line argument parsing could
result in failure to produce the AST, so we need an alternative ways to
report those errors.

We take the following approach in this patch:

- buildCompilerInvocation() now requires an explicit DiagnosticConsumer.
- TUScheduler and TestTU now collect the diagnostics produced when parsing command line arguments. If pasing of the AST failed, diagnostics are reported via a new ParsingCallbacks::onFailedAST method. If parsing of the AST succeeded, any errors produced during command-line parsing are stored alongside the AST inside the ParsedAST instance and reported as previously by calling the ParsingCallbacks::onMainAST method;
- The client code that uses ClangdServer's DiagnosticConsumer does not need to change, it will receive new diagnostics in the onDiagnosticsReady() callback

Errors produced when parsing command-line arguments are collected using
the same StoreDiags class that is used to collect all other errors. They
are recognized by their location being invalid. IIUC, the location is
invalid as there is no source manager at this point, it is created at a
later stage.

Although technically we might also get diagnostics that mention the
command-line arguments FileID with after the source manager was created
(and they have valid source locations), we choose to not handle those
and they are dropped as not coming from the main file. AFAICT, those
diagnostics should always be notes, therefore it's safe to drop them
without loosing too much information.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66759

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdUnit.cpp
  clang-tools-extra/clangd/ClangdUnit.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/HeadersTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D66759.217202.patch
Type: text/x-patch
Size: 24336 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190826/35d2e112/attachment-0001.bin>