[PATCH] D50462: Try building complete AST after a fatal error was emitted if further diagnostics are expected

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Sorry for chiming in, wanted to add my 2 cents to the conversation.

In https://reviews.llvm.org/D50462#1206203, @vsapsai wrote:

> What about having a mode that treats missing header as non-fatal error? Because I believe there are cases where there is no sense to continue after a fatal error. For example, consider hitting the error limit. Showing too many errors isn't useful and also after 50 errors it is a little bit too optimistic to assume that AST is in a good shape. I don't know if there are other fatal-but-we-can-continue errors and if we need a more general solution. So far looks like missing include is the biggest blocker for IDE-like functionality.


Your arguments are definitely valid (too many errors may be confusing and AST may not be prepared to recover on some fatal errors), but I would argue that the IDE use-case is different enough from the command-line tools to make the new behavior a preferred one.

Specifically, a few questions regarding the aforementioned fatal errors:

1. Why should hitting some error limit be considered a fatal error? One of the reasons I see for the command-line tools is not spamming the output buffers with too many errors, so that navigating to the first errors is easier. It looks like a non-issue for IDEs: they can both show all emitted errors in the text editor and make it easy to navigate to the first error (by providing a convenient UI for an error list, shortcuts to go to the first error, etc.). On the contrary, not seeing errors where I type because clang hit the limit before my line looks like a confusing experience for the editors/IDEs.

2. Why should an unresolved include (which is considered a fatal error) give a totally different result from the missing include (which is, obviously, undetectable)? They both require the same amount of work to recover and we obviously want clang to work in absence of some includes.

Besides, errors can also be easily "promoted" to fatal with command-line flags (`-Wfatal-errors`) and editor tools should probably respect those and not override their severity.

W.R.T. to the AST not being good enough: it may not be, but shouldn't we instead invest in improving the recovery on things like unresolved references?
This would give better experience in absence of non-fatal errors too (common case when copy-pasting code, removing too many #include directives from the file, etc.)  and it looks doable.

Overall, I would argue that letting clang recover from fatal errors is the right thing to do for IDEs and editor integrations and the original patch was moving in the right direction.
WDYT?


Repository:
  rC Clang

https://reviews.llvm.org/D50462