[PATCH] D54796: [clangd] C++ API for emitting file status

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 26 08:44:50 PST 2018


hokein added a comment.

> It would be reasonable to be consistent with diagnostics: stop emitting the statuses when ASTWorker was put into shutdown mode.

Thanks, sounds fair.



================
Comment at: clangd/ClangdServer.h:39
 
+// FIXME: find a better name.
 class DiagnosticsConsumer {
----------------
jkorous wrote:
> It would be unfortunate to have this name clashing with `clang::DiagnosticsConsumer` indeed.
> How about something like `FileEventConsumer`?
yeah, I plan to rename it later (when the patch gets stable enough). `FileEventConsumer` is a candidate.


================
Comment at: clangd/TUScheduler.cpp:367
       IdleASTs.take(this);
+      FStatus.messages.push_back("fail to build CompilerInvocation;");
+      Callbacks.onFileUpdated(FStatus);
----------------
jkorous wrote:
> Wouldn't some different failure status like `FileStatusKind::FailedBuild` be appropriate here?
The failure status is grouped into `FileStatus::messages`. I think `messages` provide enough information to users (if there are something wrong when building the file).


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D54796





More information about the cfe-commits mailing list